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

[Ard Biesheuvel]

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>
---
 arch/arm/kernel/ftrace.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index a0b6d1e3812f..e61591f33a6c 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -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,
-- 
2.45.1.288.g0e0cd299f1-goog