[PATCH] ARM: ftrace: Don't assume stack frames are contiguous in memory

[Linus Walleij]

On Mon, May 27, 2024 at 6:12 PM Ard Biesheuvel <ardb+git at google.com> wrote:

> From: Ard Biesheuvel <ardb at kernel.org>
>
> 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.  The ftrace code
> assumes that this activation record is pushed first, and that any stack
> space for locals is allocated below this. This would imply that the
> caller's value of SP can be obtained by adding 4 to FP (which points to
> PC in the calling 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 due to corruption of data on
> the stack.
>
> The plain uaccess API is ftrace instrumented itself, so to avoid
> unbounded recursion, use the __get_kernel_nofault() primitive instead.
>
> Closes: https://lore.kernel.org/all/alp44tukzo6mvcwl4ke4ehhmojrqnv6xfcdeuliybxfjfvgd3e@gpjvwj33cc76
> Reported-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
> Closes: https://lore.kernel.org/all/d870c149-4363-43de-b0ea-7125dec5608e@broadcom.com/
> Reported-by: Justin Chen <justin.chen at broadcom.com>
> Cc: Thorsten Scherer <T.Scherer at eckelmann.de>
> Cc: Florian Fainelli <florian.fainelli at broadcom.com>
> Cc: Doug Berger <doug.berger at broadcom.com>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>

Good catch and nice patch.
Reviewed-by: Linus Walleij <linus.walleij at linaro.org>

Yours,
Linus Walleij