Skip to content

Commit

Permalink
ARM: 9405/1: ftrace: Don't assume stack frames are contiguous in memory
Browse files Browse the repository at this point in the history
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, khadas#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/d870c149-4363-43de-b0ea-7125dec5608e@broadcom.com/

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reported-by: Justin Chen <justin.chen@broadcom.com>
Tested-by: Thorsten Scherer <t.scherer@eckelmann.de>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
  • Loading branch information
ardbiesheuvel authored and Russell King (Oracle) committed Jun 10, 2024
1 parent 1613e60 commit e3cf20e
Showing 1 changed file with 15 additions and 2 deletions.
17 changes: 15 additions & 2 deletions arch/arm/kernel/ftrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,24 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
unsigned long old;

if (unlikely(atomic_read(&current->tracing_graph_pause)))
err_out:
return;

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

0 comments on commit e3cf20e

Please sign in to comment.