From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1947153AbcHRNIr (ORCPT ); Thu, 18 Aug 2016 09:08:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33774 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1946379AbcHRNIK (ORCPT ); Thu, 18 Aug 2016 09:08:10 -0400 From: Josh Poimboeuf To: Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Andy Lutomirski , Linus Torvalds , Steven Rostedt , Brian Gerst , Kees Cook , Peter Zijlstra , Frederic Weisbecker , Byungchul Park , Nilay Vaish Subject: [PATCH v4 55/57] x86/mm: simplify starting frame logic for hardened usercopy Date: Thu, 18 Aug 2016 08:06:35 -0500 Message-Id: In-Reply-To: References: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 18 Aug 2016 13:08:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Currently, arch_within_stack_frames() has to manually find the stack frame of its great-grandparent (i.e., it's caller's caller's caller). This is somewhat fragile because it relies on the current call path and inlining decisions. Get the starting frame address closer to the source, in check_stack_object(), and pass it to arch_within_stack_frames(). Signed-off-by: Josh Poimboeuf --- arch/x86/include/asm/thread_info.h | 2 ++ arch/x86/lib/usercopy.c | 12 ++++-------- include/linux/thread_info.h | 1 + mm/usercopy.c | 15 ++++++++++----- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index fd849e6..0f27e04 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -180,10 +180,12 @@ static inline unsigned long current_stack_pointer(void) #ifdef CONFIG_FRAME_POINTER int arch_within_stack_frames(const void * const stack, const void * const stackend, + void *first_frame, const void *obj, unsigned long len); #else static inline int arch_within_stack_frames(const void * const stack, const void * const stackend, + void *first_frame const void *obj, unsigned long len) { return 0; diff --git a/arch/x86/lib/usercopy.c b/arch/x86/lib/usercopy.c index 8fe0a9c..fe0b233 100644 --- a/arch/x86/lib/usercopy.c +++ b/arch/x86/lib/usercopy.c @@ -48,20 +48,16 @@ EXPORT_SYMBOL_GPL(copy_from_user_nmi); */ int arch_within_stack_frames(const void * const stack, const void * const stackend, + void *first_frame, const void *obj, unsigned long len) { struct unwind_state state; const void *frame, *frame_end; - /* - * Start at the end of our grandparent's frame (beginning of - * great-grandparent's frame). - */ - unwind_start(&state, current, NULL, NULL); - if (WARN_ON_ONCE(!unwind_next_frame(&state) || - !unwind_next_frame(&state))) - return 0; + unwind_start(&state, current, NULL, first_frame); frame = unwind_get_stack_ptr(&state); + if (WARN_ON_ONCE(frame != first_frame)) + return 0; /* * low ----------------------------------------------> high diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h index cbd8990..aa58813 100644 --- a/include/linux/thread_info.h +++ b/include/linux/thread_info.h @@ -108,6 +108,7 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag) #ifndef CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES static inline int arch_within_stack_frames(const void * const stack, const void * const stackend, + void *first_frame, const void *obj, unsigned long len) { return 0; diff --git a/mm/usercopy.c b/mm/usercopy.c index 8ebae91..359249c 100644 --- a/mm/usercopy.c +++ b/mm/usercopy.c @@ -26,8 +26,8 @@ enum { }; /* - * Checks if a given pointer and length is contained by the current - * stack frame (if possible). + * Checks if a given pointer and length is contained by a frame in + * the current stack (if possible). * * Returns: * NOT_STACK: not at all on the stack @@ -35,7 +35,8 @@ enum { * GOOD_STACK: fully on the stack (when can't do frame-checking) * BAD_STACK: error condition (invalid stack position or bad stack frame) */ -static noinline int check_stack_object(const void *obj, unsigned long len) +static __always_inline int check_stack_object(const void *obj, + unsigned long len) { const void * const stack = task_stack_page(current); const void * const stackend = stack + THREAD_SIZE; @@ -53,8 +54,12 @@ static noinline int check_stack_object(const void *obj, unsigned long len) if (obj < stack || stackend < obj + len) return BAD_STACK; - /* Check if object is safely within a valid frame. */ - ret = arch_within_stack_frames(stack, stackend, obj, len); + /* + * Starting with the caller's stack frame, check if the object + * is safely within a valid frame. + */ + ret = arch_within_stack_frames(stack, stackend, + __builtin_frame_address(0), obj, len); if (ret) return ret; -- 2.7.4