From: Sahara <[email protected]>
This series of patches introduce the arm64 arch_within_stack_frames
implementation using stacktrace functions. Also the base code is
moved from thread_info.h to stacktrace.h. x86 code is reimplemented
to use frame pointer unwinder functions.
Note: The code is still missing in case of using x86 ORC unwinder and
guess unwinder.
v2 changes:
- Remove 'arm64: usercopy: consider dynamic array stack variable'
- minor fix in x86 arch_within_stack_frames code.
v3 changes:
- Fix build problem caused by circular inclusion of header in x86 UP config
James Morse (1):
arm64: usercopy: implement arch_within_stack_frames
Sahara (2):
stacktrace: move arch_within_stack_frames from thread_info.h
x86: usercopy: reimplement arch_within_stack_frames with unwinder
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/stacktrace.c | 76 +++++++++++++++++++++++++++++++++
arch/x86/include/asm/thread_info.h | 51 +---------------------
arch/x86/include/asm/unwind.h | 5 +++
arch/x86/kernel/Makefile | 2 +-
arch/x86/kernel/stacktrace.c | 87 ++++++++++++++++++++++++++++++++++++++
arch/x86/kernel/unwind_frame.c | 4 +-
include/linux/page_ext.h | 1 -
include/linux/stacktrace.h | 24 +++++++++++
include/linux/thread_info.h | 21 ---------
mm/usercopy.c | 2 +-
11 files changed, 197 insertions(+), 77 deletions(-)
--
2.7.4
From: James Morse <[email protected]>
This implements arch_within_stack_frames() for arm64 that should
validate if a given object is contained by a kernel stack frame.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Sahara <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/stacktrace.c | 76 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 177be0d..72d0747 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -128,6 +128,7 @@ config ARM64
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_KPROBES
select HAVE_KRETPROBES
+ select HAVE_ARCH_WITHIN_STACK_FRAMES
select IOMMU_DMA if IOMMU_SUPPORT
select IRQ_DOMAIN
select IRQ_FORCED_THREADING
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index d5718a0..5eb3784 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -27,6 +27,11 @@
#include <asm/stack_pointer.h>
#include <asm/stacktrace.h>
+#define FAKE_FRAME(frame, my_func) do { \
+ frame.fp = (unsigned long)__builtin_frame_address(0); \
+ frame.pc = (unsigned long)my_func; \
+} while (0)
+
/*
* AArch64 PCS assigns the frame pointer to x29.
*
@@ -100,6 +105,77 @@ void notrace walk_stackframe(struct task_struct *tsk, struct stackframe *frame,
}
}
+struct check_frame_arg {
+ unsigned long obj_start;
+ unsigned long obj_end;
+ unsigned long frame_start;
+ int discard_frames;
+ int err;
+};
+
+static int check_frame(struct stackframe *frame, void *d)
+{
+ struct check_frame_arg *arg = d;
+ unsigned long frame_end = frame->fp;
+
+ /* object overlaps multiple frames */
+ if (arg->obj_start < frame->fp && frame->fp < arg->obj_end) {
+ arg->err = BAD_STACK;
+ return 1;
+ }
+
+ /*
+ * Discard frames and check object is in a frame written early
+ * enough.
+ */
+ if (arg->discard_frames)
+ arg->discard_frames--;
+ else if ((arg->frame_start <= arg->obj_start &&
+ arg->obj_start < frame_end) &&
+ (arg->frame_start < arg->obj_end && arg->obj_end <= frame_end))
+ return 1;
+
+ /* object exists in a previous frame */
+ if (arg->obj_end < arg->frame_start) {
+ arg->err = BAD_STACK;
+ return 1;
+ }
+
+ arg->frame_start = frame_end + 0x10;
+
+ return 0;
+}
+
+/* Check obj doesn't overlap a stack frame record */
+int arch_within_stack_frames(const void *stack,
+ const void *stack_end,
+ const void *obj, unsigned long obj_len)
+{
+ struct stackframe frame;
+ struct check_frame_arg arg;
+
+ if (!IS_ENABLED(CONFIG_FRAME_POINTER))
+ return NOT_STACK;
+
+ arg.err = GOOD_FRAME;
+ arg.obj_start = (unsigned long)obj;
+ arg.obj_end = arg.obj_start + obj_len;
+
+ FAKE_FRAME(frame, arch_within_stack_frames);
+ arg.frame_start = frame.fp;
+
+ /*
+ * Skip 4 non-inlined frames: <fake frame>,
+ * arch_within_stack_frames(), check_stack_object() and
+ * __check_object_size().
+ */
+ arg.discard_frames = 4;
+
+ walk_stackframe(current, &frame, check_frame, &arg);
+
+ return arg.err;
+}
+
#ifdef CONFIG_STACKTRACE
struct stack_trace_data {
struct stack_trace *trace;
--
2.7.4
From: Sahara <[email protected]>
The old arch_within_stack_frames which used the frame pointer is
now reimplemented to use frame pointer unwinder apis. So the main
functionality is same as before.
Signed-off-by: Sahara <[email protected]>
---
arch/x86/include/asm/unwind.h | 5 ++++
arch/x86/kernel/stacktrace.c | 64 +++++++++++++++++++++++++++++++++---------
arch/x86/kernel/unwind_frame.c | 4 +--
3 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 1f86e1b..6f04906f 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -87,6 +87,11 @@ void unwind_init(void);
void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size,
void *orc, size_t orc_size);
#else
+#ifdef CONFIG_UNWINDER_FRAME_POINTER
+#define FRAME_HEADER_SIZE (sizeof(long) * 2)
+size_t regs_size(struct pt_regs *regs);
+#endif
+
static inline void unwind_init(void) {}
static inline
void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size,
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index ff178a0..3de1105 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -13,6 +13,33 @@
#include <asm/unwind.h>
+static inline void *get_cur_frame(struct unwind_state *state)
+{
+ void *frame = NULL;
+
+#if defined(CONFIG_UNWINDER_FRAME_POINTER)
+ if (state->regs)
+ frame = (void *)state->regs;
+ else
+ frame = (void *)state->bp;
+#endif
+ return frame;
+}
+
+static inline void *get_frame_end(struct unwind_state *state)
+{
+ void *frame_end = NULL;
+
+#if defined(CONFIG_UNWINDER_FRAME_POINTER)
+ if (state->regs) {
+ frame_end = (void *)state->regs + regs_size(state->regs);
+ } else {
+ frame_end = (void *)state->bp + FRAME_HEADER_SIZE;
+ }
+#endif
+ return frame_end;
+}
+
/*
* Walks up the stack frames to make sure that the specified object is
* entirely contained by a single stack frame.
@@ -26,31 +53,42 @@ 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);
+#if defined(CONFIG_UNWINDER_FRAME_POINTER)
+ struct unwind_state state;
+ void *prev_frame_end = NULL;
/*
* low ----------------------------------------------> high
* [saved bp][saved ip][args][local vars][saved bp][saved ip]
* ^----------------^
* allow copies only within here
+ *
+ * Skip 3 non-inlined frames: arch_within_stack_frames(),
+ * check_stack_object() and __check_object_size().
+ *
*/
- while (stack <= frame && frame < stackend) {
+ unsigned int discard_frames = 3;
+
+ for (unwind_start(&state, current, NULL, NULL); !unwind_done(&state);
+ unwind_next_frame(&state)) {
/*
* 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;
+ if (discard_frames) {
+ discard_frames--;
+ } else {
+ void *frame = get_cur_frame(&state);
+
+ if (!frame || !prev_frame_end)
+ return NOT_STACK;
+ if (obj + len <= frame)
+ return obj >= prev_frame_end ?
+ GOOD_FRAME : BAD_STACK;
+ }
+ /* save current frame end before move to next frame */
+ prev_frame_end = get_frame_end(&state);
}
return BAD_STACK;
#else
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 3dc26f9..c8bfa5c 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -8,8 +8,6 @@
#include <asm/stacktrace.h>
#include <asm/unwind.h>
-#define FRAME_HEADER_SIZE (sizeof(long) * 2)
-
unsigned long unwind_get_return_address(struct unwind_state *state)
{
if (unwind_done(state))
@@ -69,7 +67,7 @@ static void unwind_dump(struct unwind_state *state)
}
}
-static size_t regs_size(struct pt_regs *regs)
+size_t regs_size(struct pt_regs *regs)
{
/* x86_32 regs from kernel mode are two words shorter: */
if (IS_ENABLED(CONFIG_X86_32) && !user_mode(regs))
--
2.7.4
From: Sahara <[email protected]>
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 <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
arch/x86/include/asm/thread_info.h | 51 +-------------------------------------
arch/x86/kernel/Makefile | 2 +-
arch/x86/kernel/stacktrace.c | 49 ++++++++++++++++++++++++++++++++++++
include/linux/page_ext.h | 1 -
include/linux/stacktrace.h | 24 ++++++++++++++++++
include/linux/thread_info.h | 21 ----------------
mm/usercopy.c | 2 +-
7 files changed, 76 insertions(+), 74 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 <asm/stacktrace.h>
#include <asm/unwind.h>
+
+/*
+ * 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/page_ext.h b/include/linux/page_ext.h
index ca5461e..f3b7dd9 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -3,7 +3,6 @@
#define __LINUX_PAGE_EXT_H
#include <linux/types.h>
-#include <linux/stacktrace.h>
#include <linux/stackdepot.h>
struct pglist_data;
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 <linux/bitops.h>
-
-/*
- * 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 <asm/thread_info.h>
#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 <linux/sched.h>
#include <linux/sched/task.h>
#include <linux/sched/task_stack.h>
-#include <linux/thread_info.h>
+#include <linux/stacktrace.h>
#include <asm/sections.h>
/*
--
2.7.4
From: [email protected]
> Sent: 09 April 2018 12:59
>
> The old arch_within_stack_frames which used the frame pointer is
> now reimplemented to use frame pointer unwinder apis. So the main
> functionality is same as before.
How much slower does this make the code?
Following stack frames using %bp is reasonably quick.
I can't imagine some of the other unwinder APIs being any where
near that fast.
While fine for fault tracebacks, using them during usercopy
is likely to have measurable performance impact.
David
Hi Sahara,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16 next-20180409]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/kpark3469-gmail-com/usercopy-reimplement-arch_within_stack_frames/20180409-221953
config: x86_64-randconfig-s0-04100256 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/gpu//drm/drm_mm.c: In function 'save_stack':
drivers/gpu//drm/drm_mm.c:109:9: error: variable 'trace' has initializer but incomplete type
struct stack_trace trace = {
^~~~~~~~~~~
>> drivers/gpu//drm/drm_mm.c:110:3: error: unknown field 'entries' specified in initializer
.entries = entries,
^
drivers/gpu//drm/drm_mm.c:110:14: warning: excess elements in struct initializer
.entries = entries,
^~~~~~~
drivers/gpu//drm/drm_mm.c:110:14: note: (near initialization for 'trace')
>> drivers/gpu//drm/drm_mm.c:111:3: error: unknown field 'max_entries' specified in initializer
.max_entries = STACKDEPTH,
^
drivers/gpu//drm/drm_mm.c:103:20: warning: excess elements in struct initializer
#define STACKDEPTH 32
^
drivers/gpu//drm/drm_mm.c:111:18: note: in expansion of macro 'STACKDEPTH'
.max_entries = STACKDEPTH,
^~~~~~~~~~
drivers/gpu//drm/drm_mm.c:103:20: note: (near initialization for 'trace')
#define STACKDEPTH 32
^
drivers/gpu//drm/drm_mm.c:111:18: note: in expansion of macro 'STACKDEPTH'
.max_entries = STACKDEPTH,
^~~~~~~~~~
>> drivers/gpu//drm/drm_mm.c:112:3: error: unknown field 'skip' specified in initializer
.skip = 1
^
drivers/gpu//drm/drm_mm.c:112:11: warning: excess elements in struct initializer
.skip = 1
^
drivers/gpu//drm/drm_mm.c:112:11: note: (near initialization for 'trace')
drivers/gpu//drm/drm_mm.c:109:21: error: storage size of 'trace' isn't known
struct stack_trace trace = {
^~~~~
>> drivers/gpu//drm/drm_mm.c:115:2: error: implicit declaration of function 'save_stack_trace' [-Werror=implicit-function-declaration]
save_stack_trace(&trace);
^~~~~~~~~~~~~~~~
drivers/gpu//drm/drm_mm.c:109:21: warning: unused variable 'trace' [-Wunused-variable]
struct stack_trace trace = {
^~~~~
drivers/gpu//drm/drm_mm.c: In function 'show_leaks':
drivers/gpu//drm/drm_mm.c:135:10: error: variable 'trace' has initializer but incomplete type
struct stack_trace trace = {
^~~~~~~~~~~
drivers/gpu//drm/drm_mm.c:136:4: error: unknown field 'entries' specified in initializer
.entries = entries,
^
drivers/gpu//drm/drm_mm.c:136:15: warning: excess elements in struct initializer
.entries = entries,
^~~~~~~
drivers/gpu//drm/drm_mm.c:136:15: note: (near initialization for 'trace')
drivers/gpu//drm/drm_mm.c:137:4: error: unknown field 'max_entries' specified in initializer
.max_entries = STACKDEPTH
^
drivers/gpu//drm/drm_mm.c:103:20: warning: excess elements in struct initializer
#define STACKDEPTH 32
^
drivers/gpu//drm/drm_mm.c:137:19: note: in expansion of macro 'STACKDEPTH'
.max_entries = STACKDEPTH
^~~~~~~~~~
drivers/gpu//drm/drm_mm.c:103:20: note: (near initialization for 'trace')
#define STACKDEPTH 32
^
drivers/gpu//drm/drm_mm.c:137:19: note: in expansion of macro 'STACKDEPTH'
.max_entries = STACKDEPTH
^~~~~~~~~~
drivers/gpu//drm/drm_mm.c:135:22: error: storage size of 'trace' isn't known
struct stack_trace trace = {
^~~~~
drivers/gpu//drm/drm_mm.c:147:3: error: implicit declaration of function 'snprint_stack_trace' [-Werror=implicit-function-declaration]
snprint_stack_trace(buf, BUFSZ, &trace, 0);
^~~~~~~~~~~~~~~~~~~
drivers/gpu//drm/drm_mm.c:135:22: warning: unused variable 'trace' [-Wunused-variable]
struct stack_trace trace = {
^~~~~
cc1: some warnings being treated as errors
vim +/max_entries +111 drivers/gpu//drm/drm_mm.c
5705670d Chris Wilson 2016-10-31 105
5705670d Chris Wilson 2016-10-31 106 static noinline void save_stack(struct drm_mm_node *node)
5705670d Chris Wilson 2016-10-31 107 {
5705670d Chris Wilson 2016-10-31 108 unsigned long entries[STACKDEPTH];
5705670d Chris Wilson 2016-10-31 @109 struct stack_trace trace = {
5705670d Chris Wilson 2016-10-31 @110 .entries = entries,
5705670d Chris Wilson 2016-10-31 @111 .max_entries = STACKDEPTH,
5705670d Chris Wilson 2016-10-31 @112 .skip = 1
5705670d Chris Wilson 2016-10-31 113 };
5705670d Chris Wilson 2016-10-31 114
5705670d Chris Wilson 2016-10-31 @115 save_stack_trace(&trace);
5705670d Chris Wilson 2016-10-31 116 if (trace.nr_entries != 0 &&
5705670d Chris Wilson 2016-10-31 117 trace.entries[trace.nr_entries-1] == ULONG_MAX)
5705670d Chris Wilson 2016-10-31 118 trace.nr_entries--;
5705670d Chris Wilson 2016-10-31 119
5705670d Chris Wilson 2016-10-31 120 /* May be called under spinlock, so avoid sleeping */
5705670d Chris Wilson 2016-10-31 121 node->stack = depot_save_stack(&trace, GFP_NOWAIT);
5705670d Chris Wilson 2016-10-31 122 }
5705670d Chris Wilson 2016-10-31 123
:::::: The code at line 111 was first introduced by commit
:::::: 5705670d0463423337c82d00877989e7df8b169d drm: Track drm_mm allocators and show leaks on shutdown
:::::: TO: Chris Wilson <[email protected]>
:::::: CC: Daniel Vetter <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Apr 09, 2018 at 01:14:58PM +0000, David Laight wrote:
> From: [email protected]
> > Sent: 09 April 2018 12:59
> >
> > The old arch_within_stack_frames which used the frame pointer is
> > now reimplemented to use frame pointer unwinder apis. So the main
> > functionality is same as before.
>
> How much slower does this make the code?
> Following stack frames using %bp is reasonably quick.
> I can't imagine some of the other unwinder APIs being any where
> near that fast.
> While fine for fault tracebacks, using them during usercopy
> is likely to have measurable performance impact.
Agreed, using the unwind interface is going to be quite a bit slower
than the current manual approach. So this patch has only drawbacks and
no benefits.
The only benefit would be if you used the unwind API in a generic
manner, such that it also worked for the ORC unwinder. But even then I
think we'd need to see performance numbers, with both FP and ORC, to see
how bad the impact is on usercopy.
--
Josh