[PATCH v4 53/57] x86/mm: move arch_within_stack_frames() to usercopy.c

From: Josh Poimboeuf
Date: Thu Aug 18 2016 - 09:10:40 EST


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@xxxxxxxxxx>
---
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