lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <a47ef86a990a60ab17bd1363270ad4eeed9d22f2.1471525031.git.jpoimboe@redhat.com>
Date:	Thu, 18 Aug 2016 08:06:33 -0500
From:	Josh Poimboeuf <jpoimboe@...hat.com>
To:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>,
	"H . Peter Anvin" <hpa@...or.com>
Cc:	x86@...nel.org, linux-kernel@...r.kernel.org,
	Andy Lutomirski <luto@...capital.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Brian Gerst <brgerst@...il.com>,
	Kees Cook <keescook@...omium.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Byungchul Park <byungchul.park@....com>,
	Nilay Vaish <nilayvaish@...il.com>
Subject: [PATCH v4 53/57] x86/mm: move arch_within_stack_frames() to usercopy.c

When I tried to port arch_within_stack_frames() to use the new unwinder,
I got a nightmare include file "header soup" scenario when unwind.h was
included from thread_info.h.  And anyway, I think thread_info.h isn't
really an appropriate place for this function.  So move it to usercopy.c
instead.

Since it relies on its parent's stack pointer, and the function is no
longer inlined, the arguments to the __builtin_frame_address() calls
have been incremented.

Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
---
 arch/Kconfig                       |  4 ++--
 arch/x86/include/asm/thread_info.h | 46 ++++++++------------------------------
 arch/x86/lib/usercopy.c            | 43 +++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index e9c9334..1513043 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -467,8 +467,8 @@ config HAVE_ARCH_WITHIN_STACK_FRAMES
 	  An architecture should select this if it can walk the kernel stack
 	  frames to determine if an object is part of either the arguments
 	  or local variables (i.e. that it excludes saved return addresses,
-	  and similar) by implementing an inline arch_within_stack_frames(),
-	  which is used by CONFIG_HARDENED_USERCOPY.
+	  and similar) by implementing arch_within_stack_frames(), which is
+	  used by CONFIG_HARDENED_USERCOPY.
 
 config HAVE_CONTEXT_TRACKING
 	bool
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 8b7c8d8e..fd849e6 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -176,49 +176,21 @@ static inline unsigned long current_stack_pointer(void)
 	return sp;
 }
 
-/*
- * Walks up the stack frames to make sure that the specified object is
- * entirely contained by a single stack frame.
- *
- * Returns:
- *		 1 if within a frame
- *		-1 if placed across a frame boundary (or outside stack)
- *		 0 unable to determine (no frame pointers, etc)
- */
+#ifdef CONFIG_HARDENED_USERCOPY
+#ifdef CONFIG_FRAME_POINTER
+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)
 {
-#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 *) ? 1 : -1;
-		oldframe = frame;
-		frame = *(const void * const *)frame;
-	}
-	return -1;
-#else
 	return 0;
-#endif
 }
+#endif /* CONFIG_FRAME_POINTER */
+#endif /* CONFIG_HARDENED_USERCOPY */
+
 
 #else /* !__ASSEMBLY__ */
 
diff --git a/arch/x86/lib/usercopy.c b/arch/x86/lib/usercopy.c
index b490878..2492fa7 100644
--- a/arch/x86/lib/usercopy.c
+++ b/arch/x86/lib/usercopy.c
@@ -9,6 +9,7 @@
 
 #include <asm/word-at-a-time.h>
 #include <linux/sched.h>
+#include <asm/unwind.h>
 
 /*
  * We rely on the nested NMI work to allow atomic faults from the NMI path; the
@@ -34,3 +35,45 @@ copy_from_user_nmi(void *to, const void __user *from, unsigned long n)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(copy_from_user_nmi);
+
+#if defined(CONFIG_HARDENED_USERCOPY) && defined(CONFIG_FRAME_POINTER)
+/*
+ * Walks up the stack frames to make sure that the specified object is
+ * entirely contained by a single stack frame.
+ *
+ * Returns:
+ *		 1 if within a frame
+ *		-1 if placed across a frame boundary (or outside stack)
+ *		 0 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)
+{
+	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 *) ? 1 : -1;
+		oldframe = frame;
+		frame = *(const void * const *)frame;
+	}
+	return -1;
+}
+#endif /* CONFIG_HARDENED_USERCOPY && CONFIG_FRAME_POINTER */
-- 
2.7.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ