Commit e3cf20e5 authored by Ard Biesheuvel's avatar Ard Biesheuvel Committed by Russell King (Oracle)

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/d870c149-4363-43de-b0ea-7125dec5608e@broadcom.com/Reported-by: default avatarUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reported-by: default avatarJustin Chen <justin.chen@broadcom.com>
Tested-by: default avatarThorsten Scherer <t.scherer@eckelmann.de>
Reviewed-by: default avatarLinus Walleij <linus.walleij@linaro.org>
Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
parent 1613e604
......@@ -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,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment