From patchwork Tue Apr 10 07:30:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: kpark3469@gmail.com X-Patchwork-Id: 10332425 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8973660365 for ; Tue, 10 Apr 2018 07:31:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 75AFF28B4F for ; Tue, 10 Apr 2018 07:31:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6A64528B51; Tue, 10 Apr 2018 07:31:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 2AB0128B4F for ; Tue, 10 Apr 2018 07:31:36 +0000 (UTC) Received: (qmail 19569 invoked by uid 550); 10 Apr 2018 07:31:30 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 18308 invoked from network); 10 Apr 2018 07:31:26 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=QVdUNFQEP2PBzgiizGMptMiMFI80vvYZP7iNFdF67Sg=; b=beBpv6g8j9l3/sTpCc57fNqfwEPDj7f2QT/UJtkNre4Ae0xNIvqyZoef3dyjji6Kw8 E55IzufsOkBOf1eV1rZr0oYSh1JVh2AsbWbKaddD5/K6bY42ctKUTfsKZBj+dO09bOBj Uqds8NgOH/Rq5DXlf44zWE7j+jIHjrGDTNLpi2wK7hEjawS/iOpDlSPD9JA/hJEXHukj sqOeh83lAWKxLNqiTUkEm2QWrKa6wg4KnKsbrxrtmDvMiElxosHRpirStnuI+92W7LK7 OFBhcwp0fzA3sGDs/0jnf0wbyxKceNOXX/y1HilRJINJC5f8/n6oMG1JnbBLxi1Fgkp9 c9Tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=QVdUNFQEP2PBzgiizGMptMiMFI80vvYZP7iNFdF67Sg=; b=aKyZU1jT6hDfe0HzVC6Qt/dvEV8cVtZeci1+skY8EVSw0mJu+AmmqZQcb/SdXmgn+J HFLtGgN8Fcw1PNgqxZW2mhjGSIwkQyp0r9WwBmQyHvQR2I2H+UY52vYdqA8J4PuZCafJ OlZBkOM6zVQbdKDAjo3O2stZHm5rGVXmjhwbH8CVxIyZNy0wFqhQ2ncSZJv8eS0bHX2P DwAy6zw3xzVHJHtrS6qIfyH0N/3y0haiswWwpbgnGkGEpZNhPfhlunAPZTdYxFTFDP8b HPW7lm38VIZmW1OR3V3iI5vTQWO1GvZCJP6s6H3I3zKOAT4oFkxtgUtEhFrilmf1eqhf go9Q== X-Gm-Message-State: ALQs6tBwvxCq07E2AvzKuu1vL0N7I4mkq4TV+S8r5FWZJSduuuvfUiQx G2FvMLCueAt1OsVprQ7kb3jr8Q== X-Google-Smtp-Source: AIpwx482grO+mPOJ0F88H1gyuEWU38puBOZxurRC7sdWwgRxiIlWXXb8acLkMQGQH3eY1qSHLJmZGw== X-Received: by 10.28.7.133 with SMTP id 127mr692743wmh.74.1523345475084; Tue, 10 Apr 2018 00:31:15 -0700 (PDT) From: kpark3469@gmail.com To: kernel-hardening@lists.openwall.com Cc: catalin.marinas@arm.com, keescook@chromium.org, will.deacon@arm.com, mark.rutland@arm.com, james.morse@arm.com, panand@redhat.com, keun-o.park@darkmatter.ae, psodagud@codeaurora.org, jpoimboe@redhat.com, mingo@kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 1/3] stacktrace: move arch_within_stack_frames from thread_info.h Date: Tue, 10 Apr 2018 11:30:45 +0400 Message-Id: <1523345447-10725-2-git-send-email-kpark3469@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1523345447-10725-1-git-send-email-kpark3469@gmail.com> References: <1523345447-10725-1-git-send-email-kpark3469@gmail.com> X-Virus-Scanned: ClamAV using ClamSMTP From: Sahara Since the inlined arch_within_stack_frames function was placed within asm/thread_info.h, using stacktrace functions to unwind stack was restricted. Now in order to have this function use more abundant apis, it is moved to architecture's stacktrace.c. And, it is changed from inline to noinline function to clearly have three depth calls like: do_usercopy_stack() copy_[to|from]_user() : inline check_copy_size() : inline __check_object_size() check_stack_object() arch_within_stack_frames() With this change, the x86's implementation was slightly changed also. Signed-off-by: Sahara Reviewed-by: Kees Cook --- arch/x86/include/asm/thread_info.h | 51 +------------------------------------- arch/x86/kernel/Makefile | 2 +- arch/x86/kernel/stacktrace.c | 49 ++++++++++++++++++++++++++++++++++++ include/linux/stacktrace.h | 24 ++++++++++++++++++ include/linux/thread_info.h | 21 ---------------- mm/usercopy.c | 2 +- 6 files changed, 76 insertions(+), 73 deletions(-) diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index a5d9521..e25d70a 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -156,59 +156,10 @@ struct thread_info { * * preempt_count needs to be 1 initially, until the scheduler is functional. */ -#ifndef __ASSEMBLY__ - -/* - * Walks up the stack frames to make sure that the specified object is - * entirely contained by a single stack frame. - * - * Returns: - * GOOD_FRAME if within a frame - * BAD_STACK if placed across a frame boundary (or outside stack) - * NOT_STACK unable to determine (no frame pointers, etc) - */ -static inline int arch_within_stack_frames(const void * const stack, - const void * const stackend, - const void *obj, unsigned long len) -{ -#if defined(CONFIG_FRAME_POINTER) - const void *frame = NULL; - const void *oldframe; - - oldframe = __builtin_frame_address(1); - if (oldframe) - frame = __builtin_frame_address(2); - /* - * low ----------------------------------------------> high - * [saved bp][saved ip][args][local vars][saved bp][saved ip] - * ^----------------^ - * allow copies only within here - */ - while (stack <= frame && frame < stackend) { - /* - * If obj + len extends past the last frame, this - * check won't pass and the next frame will be 0, - * causing us to bail out and correctly report - * the copy as invalid. - */ - if (obj + len <= frame) - return obj >= oldframe + 2 * sizeof(void *) ? - GOOD_FRAME : BAD_STACK; - oldframe = frame; - frame = *(const void * const *)frame; - } - return BAD_STACK; -#else - return NOT_STACK; -#endif -} - -#else /* !__ASSEMBLY__ */ - +#ifdef __ASSEMBLY__ #ifdef CONFIG_X86_64 # define cpu_current_top_of_stack (cpu_tss_rw + TSS_sp1) #endif - #endif #ifdef CONFIG_COMPAT diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 02d6f5c..3b8afd5 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -70,7 +70,7 @@ obj-$(CONFIG_IA32_EMULATION) += tls.o obj-y += step.o obj-$(CONFIG_INTEL_TXT) += tboot.o obj-$(CONFIG_ISA_DMA_API) += i8237.o -obj-$(CONFIG_STACKTRACE) += stacktrace.o +obj-y += stacktrace.o obj-y += cpu/ obj-y += acpi/ obj-y += reboot.o diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index 093f2ea..ff178a0 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -12,6 +12,54 @@ #include #include + +/* + * Walks up the stack frames to make sure that the specified object is + * entirely contained by a single stack frame. + * + * Returns: + * GOOD_FRAME if within a frame + * BAD_STACK if placed across a frame boundary (or outside stack) + * NOT_STACK unable to determine (no frame pointers, etc) + */ +int arch_within_stack_frames(const void * const stack, + const void * const stackend, + const void *obj, unsigned long len) +{ +#if defined(CONFIG_FRAME_POINTER) + const void *frame = NULL; + const void *oldframe; + + oldframe = __builtin_frame_address(2); + if (oldframe) + frame = __builtin_frame_address(3); + /* + * low ----------------------------------------------> high + * [saved bp][saved ip][args][local vars][saved bp][saved ip] + * ^----------------^ + * allow copies only within here + */ + while (stack <= frame && frame < stackend) { + /* + * If obj + len extends past the last frame, this + * check won't pass and the next frame will be 0, + * causing us to bail out and correctly report + * the copy as invalid. + */ + if (obj + len <= frame) + return obj >= oldframe + 2 * sizeof(void *) ? + GOOD_FRAME : BAD_STACK; + oldframe = frame; + frame = *(const void * const *)frame; + } + return BAD_STACK; +#else + return NOT_STACK; +#endif +} + +#ifdef CONFIG_STACKTRACE + static int save_stack_address(struct stack_trace *trace, unsigned long addr, bool nosched) { @@ -241,3 +289,4 @@ void save_stack_trace_user(struct stack_trace *trace) if (trace->nr_entries < trace->max_entries) trace->entries[trace->nr_entries++] = ULONG_MAX; } +#endif /* CONFIG_STACKTRACE */ diff --git a/include/linux/stacktrace.h b/include/linux/stacktrace.h index ba29a06..60bb988 100644 --- a/include/linux/stacktrace.h +++ b/include/linux/stacktrace.h @@ -7,6 +7,30 @@ struct task_struct; struct pt_regs; +/* + * For per-arch arch_within_stack_frames() implementations, defined in + * kernel/stacktrace.c. + */ +enum { + BAD_STACK = -1, + NOT_STACK = 0, + GOOD_FRAME, + GOOD_STACK, +}; + +#ifdef CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES +extern int arch_within_stack_frames(const void * const stack, + const void * const stackend, + const void *obj, unsigned long len); +#else +static inline int arch_within_stack_frames(const void * const stack, + const void * const stackend, + const void *obj, unsigned long len) +{ + return NOT_STACK; +} +#endif + #ifdef CONFIG_STACKTRACE struct stack_trace { unsigned int nr_entries, max_entries; diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h index 34f053a..5403851 100644 --- a/include/linux/thread_info.h +++ b/include/linux/thread_info.h @@ -23,18 +23,6 @@ #endif #include - -/* - * For per-arch arch_within_stack_frames() implementations, defined in - * asm/thread_info.h. - */ -enum { - BAD_STACK = -1, - NOT_STACK = 0, - GOOD_FRAME, - GOOD_STACK, -}; - #include #ifdef __KERNEL__ @@ -92,15 +80,6 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag) #define tif_need_resched() test_thread_flag(TIF_NEED_RESCHED) -#ifndef CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES -static inline int arch_within_stack_frames(const void * const stack, - const void * const stackend, - const void *obj, unsigned long len) -{ - return 0; -} -#endif - #ifdef CONFIG_HARDENED_USERCOPY extern void __check_object_size(const void *ptr, unsigned long n, bool to_user); diff --git a/mm/usercopy.c b/mm/usercopy.c index e9e9325..6a74776 100644 --- a/mm/usercopy.c +++ b/mm/usercopy.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include /*