Skip to content

Commit e3cf20e

Browse files
ardbiesheuvelRussell King (Oracle)
authored andcommitted
ARM: 9405/1: ftrace: Don't assume stack frames are contiguous in memory
The frame pointer unwinder relies on a standard layout of the stack frame, consisting of (in downward order) Calling frame: PC <---------+ LR | SP | FP | .. locals .. | Callee frame: | PC | LR | SP | FP ----------+ where after storing its previous value on the stack, FP is made to point at the location of PC in the callee stack frame, using the canonical prologue: mov ip, sp stmdb sp!, {fp, ip, lr, pc} sub fp, ip, #4 The ftrace code assumes that this activation record is pushed first, and that any stack space for locals is allocated below this. Strict adherence to this would imply that the caller's value of SP at the time of the function call can always be obtained by adding 4 to FP (which points to PC in the callee frame). However, recent versions of GCC appear to deviate from this rule, and so the only reliable way to obtain the caller's value of SP is to read it from the activation record. Since this involves a read from memory rather than simple arithmetic, we need to use the uaccess API here which protects against inadvertent data aborts resulting from attempts to dereference bogus FP values. The plain uaccess API is ftrace instrumented itself, so to avoid unbounded recursion, use the __get_kernel_nofault() primitive directly. Closes: https://lore.kernel.org/all/alp44tukzo6mvcwl4ke4ehhmojrqnv6xfcdeuliybxfjfvgd3e@gpjvwj33cc76 Closes: https://lore.kernel.org/all/[email protected]/ Reported-by: Uwe Kleine-König <[email protected]> Reported-by: Justin Chen <[email protected]> Tested-by: Thorsten Scherer <[email protected]> Reviewed-by: Linus Walleij <[email protected]> Signed-off-by: Ard Biesheuvel <[email protected]> Signed-off-by: Russell King (Oracle) <[email protected]>
1 parent 1613e60 commit e3cf20e

File tree

1 file changed

+15
-2
lines changed

1 file changed

+15
-2
lines changed

arch/arm/kernel/ftrace.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,11 +232,24 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
232232
unsigned long old;
233233

234234
if (unlikely(atomic_read(&current->tracing_graph_pause)))
235+
err_out:
235236
return;
236237

237238
if (IS_ENABLED(CONFIG_UNWINDER_FRAME_POINTER)) {
238-
/* FP points one word below parent's top of stack */
239-
frame_pointer += 4;
239+
/*
240+
* Usually, the stack frames are contiguous in memory but cases
241+
* have been observed where the next stack frame does not live
242+
* at 'frame_pointer + 4' as this code used to assume.
243+
*
244+
* Instead, dereference the field in the stack frame that
245+
* stores the SP of the calling frame: to avoid unbounded
246+
* recursion, this cannot involve any ftrace instrumented
247+
* functions, so use the __get_kernel_nofault() primitive
248+
* directly.
249+
*/
250+
__get_kernel_nofault(&frame_pointer,
251+
(unsigned long *)(frame_pointer - 8),
252+
unsigned long, err_out);
240253
} else {
241254
struct stackframe frame = {
242255
.fp = frame_pointer,

0 commit comments

Comments
 (0)