arm64: unwind: reference pt_regs via embedded stack frame

As it turns out, the unwind code is slightly broken, and probably has
been for a while. The problem is in the dumping of the exception stack,
which is intended to dump the contents of the pt_regs struct at each
level in the call stack where an exception was taken and routed to a
routine marked as __exception (which means its stack frame is right
below the pt_regs struct on the stack).

'Right below the pt_regs struct' is ill defined, though: the unwind
code assigns 'frame pointer + 0x10' to the .sp member of the stackframe
struct at each level, and dump_backtrace() happily dereferences that as
the pt_regs pointer when encountering an __exception routine. However,
the actual size of the stack frame created by this routine (which could
be one of many __exception routines we have in the kernel) is not known,
and so frame.sp is pretty useless to figure out where struct pt_regs
really is.

So it seems the only way to ensure that we can find our struct pt_regs
when walking the stack frames is to put it at a known fixed offset of
the stack frame pointer that is passed to such __exception routines.
The simplest way to do that is to put it inside pt_regs itself, which is
the main change implemented by this patch. As a bonus, doing this allows
us to get rid of a fair amount of cruft related to walking from one stack
to the other, which is especially nice since we intend to introduce yet
another stack for overflow handling once we add support for vmapped
stacks. It also fixes an inconsistency where we only add a stack frame
pointing to ELR_EL1 if we are executing from the IRQ stack but not when
we are executing from the task stack.

To consistly identify exceptions regs even in the presence of exceptions
taken from entry code, we must check whether the next frame was created
by entry text, rather than whether the current frame was crated by
exception text.

To avoid backtracing using PCs that fall in the idmap, or are controlled
by userspace, we must explcitly zero the FP and LR in startup paths, and
must ensure that the frame embedded in pt_regs is zeroed upon entry from
EL0. To avoid these NULL entries showin in the backtrace, unwind_frame()
is updated to avoid them.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
[Mark: compare current frame against .entry.text, avoid bogus PCs]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>

diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index 8155e486ce4899d41c6efbc036cf24d5c07bed20..8ba89c4ca1830889169f49110f3f01365c10518a 100644 (file)
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -16,31 +16,6 @@ struct pt_regs;
 
 DECLARE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack);
 
-/*
- * The highest address on the stack, and the first to be used. Used to
- * find the dummy-stack frame put down by el?_irq() in entry.S, which
- * is structured as follows:
- *
- *       ------------
- *       |          |  <- irq_stack_ptr
- *   top ------------
- *       |   x19    | <- irq_stack_ptr - 0x08
- *       ------------
- *       |   x29    | <- irq_stack_ptr - 0x10
- *       ------------
- *
- * where x19 holds a copy of the task stack pointer where the struct pt_regs
- * from kernel_entry can be found.
- *
- */
-#define IRQ_STACK_PTR() ((unsigned long)raw_cpu_ptr(irq_stack) + IRQ_STACK_START_SP)
-
-/*
- * The offset from irq_stack_ptr where entry.S will store the original
- * stack pointer. Used by unwind_frame() and dump_backtrace().
- */
-#define IRQ_STACK_TO_TASK_STACK(ptr) (*((unsigned long *)((ptr) - 0x08)))
-
 extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
 
 static inline int nr_legacy_irqs(void)

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 11403fdd0a50593bad84714d17fea5510126db5e..ee72aa9790789b7db6afc14bc31a6226aa046535 100644 (file)
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -119,6 +119,7 @@ struct pt_regs {
        u64 syscallno;
        u64 orig_addr_limit;
        u64 unused;     // maintain 16 byte alignment
+       u64 stackframe[2];
 };
 
 #define MAX_REG_OFFSET offsetof(struct pt_regs, pstate)

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 02e9035b0685823faca22e5b6943ba39b7d27ce2..41361684580d7d47fc7d87ee4d735d6d24655999 100644 (file)
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -60,4 +60,9 @@ static inline int in_exception_text(unsigned long ptr)
        return in ? : __in_irqentry_text(ptr);
 }
 
+static inline int in_entry_text(unsigned long ptr)
+{
+       return ptr >= (unsigned long)&__entry_text_start &&
+              ptr < (unsigned long)&__entry_text_end;
+}
 #endif

diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index b3bb7ef97bc85c2c50580d10179f3fbbbb33e8ef..71bf088f1e4b2e63062fd1e75320b69c0951b050 100644 (file)
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -75,6 +75,7 @@ int main(void)
   DEFINE(S_ORIG_X0,            offsetof(struct pt_regs, orig_x0));
   DEFINE(S_SYSCALLNO,          offsetof(struct pt_regs, syscallno));
   DEFINE(S_ORIG_ADDR_LIMIT,    offsetof(struct pt_regs, orig_addr_limit));
+  DEFINE(S_STACKFRAME,         offsetof(struct pt_regs, stackframe));
   DEFINE(S_FRAME_SIZE,         sizeof(struct pt_regs));
   BLANK();
   DEFINE(MM_CONTEXT_ID,                offsetof(struct mm_struct, context.id.counter));

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 9e126d3d8b531ee5fb439a3d3b8ecba175bf572d..612a077ba109d533bb9f1406e7ea54ad947cd01d 100644 (file)
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -111,6 +111,18 @@
        mrs     x23, spsr_el1
        stp     lr, x21, [sp, #S_LR]
 
+       /*
+        * In order to be able to dump the contents of struct pt_regs at the
+        * time the exception was taken (in case we attempt to walk the call
+        * stack later), chain it together with the stack frames.
+        */
+       .if \el == 0
+       stp     xzr, xzr, [sp, #S_STACKFRAME]
+       .else
+       stp     x29, x22, [sp, #S_STACKFRAME]
+       .endif
+       add     x29, sp, #S_STACKFRAME
+
 #ifdef CONFIG_ARM64_SW_TTBR0_PAN
        /*
         * Set the TTBR0 PAN bit in SPSR. When the exception is taken from
@@ -265,14 +277,6 @@ alternative_else_nop_endif
 
        /* switch to the irq stack */
        mov     sp, x26
-
-       /*
-        * Add a dummy stack frame, this non-standard format is fixed up
-        * by unwind_frame()
-        */
-       stp     x29, x19, [sp, #-16]!
-       mov     x29, sp
-
 9998:
        .endm
 

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 973df7de7bf8d3e4fcde6dc2ba1590fa051964a2..f9e4aacf4f423246e858951bd7a652502b542b9e 100644 (file)
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -362,6 +362,9 @@ __primary_switched:
        ret                                     // to __primary_switch()
 0:
 #endif
+       add     sp, sp, #16
+       mov     x29, #0
+       mov     x30, #0
        b       start_kernel
 ENDPROC(__primary_switched)
 
@@ -617,6 +620,7 @@ __secondary_switched:
        ldr     x2, [x0, #CPU_BOOT_TASK]
        msr     sp_el0, x2
        mov     x29, #0
+       mov     x30, #0
        b       secondary_start_kernel
 ENDPROC(__secondary_switched)
 

diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index beaf51fb3088b6549be07dfe3b3cec784073dcf9..81d9262acaf0b1409b70eb658e96f06cd376296c 100644 (file)
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -76,34 +76,13 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
 
        /*
-        * Check whether we are going to walk through from interrupt stack
-        * to task stack.
-        * If we reach the end of the stack - and its an interrupt stack,
-        * unpack the dummy frame to find the original elr.
-        *
-        * Check the frame->fp we read from the bottom of the irq_stack,
-        * and the original task stack pointer are both in current->stack.
+        * Frames created upon entry from EL0 have NULL FP and PC values, so
+        * don't bother reporting these. Frames created by __noreturn functions
+        * might have a valid FP even if PC is bogus, so only terminate where
+        * both are NULL.
         */
-       if (frame->sp == IRQ_STACK_PTR()) {
-               struct pt_regs *irq_args;
-               unsigned long orig_sp = IRQ_STACK_TO_TASK_STACK(frame->sp);
-
-               if (object_is_on_stack((void *)orig_sp) &&
-                  object_is_on_stack((void *)frame->fp)) {
-                       frame->sp = orig_sp;
-
-                       /* orig_sp is the saved pt_regs, find the elr */
-                       irq_args = (struct pt_regs *)orig_sp;
-                       frame->pc = irq_args->pc;
-               } else {
-                       /*
-                        * This frame has a non-standard format, and we
-                        * didn't fix it, because the data looked wrong.
-                        * Refuse to output this frame.
-                        */
-                       return -EINVAL;
-               }
-       }
+       if (!frame->fp && !frame->pc)
+               return -EINVAL;
 
        return 0;
 }

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 5797f5037ec9fede24c9cc20f59c546d4413da28..075c29a24345324a04ea96609aa4e0b80882d08d 100644 (file)
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -143,7 +143,6 @@ static void dump_instr(const char *lvl, struct pt_regs *regs)
 void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 {
        struct stackframe frame;
-       unsigned long irq_stack_ptr;
        int skip;
 
        pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
@@ -154,15 +153,6 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
        if (!try_get_task_stack(tsk))
                return;
 
-       /*
-        * Switching between stacks is valid when tracing current and in
-        * non-preemptible context.
-        */
-       if (tsk == current && !preemptible())
-               irq_stack_ptr = IRQ_STACK_PTR();
-       else
-               irq_stack_ptr = 0;
-
        if (tsk == current) {
                frame.fp = (unsigned long)__builtin_frame_address(0);
                frame.sp = current_stack_pointer;
@@ -182,13 +172,12 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
        skip = !!regs;
        printk("Call trace:\n");
        while (1) {
-               unsigned long where = frame.pc;
                unsigned long stack;
                int ret;
 
                /* skip until specified stack frame */
                if (!skip) {
-                       dump_backtrace_entry(where);
+                       dump_backtrace_entry(frame.pc);
                } else if (frame.fp == regs->regs[29]) {
                        skip = 0;
                        /*
@@ -203,20 +192,13 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
                ret = unwind_frame(tsk, &frame);
                if (ret < 0)
                        break;
-               stack = frame.sp;
-               if (in_exception_text(where)) {
-                       /*
-                        * If we switched to the irq_stack before calling this
-                        * exception handler, then the pt_regs will be on the
-                        * task stack. The easiest way to tell is if the large
-                        * pt_regs would overlap with the end of the irq_stack.
-                        */
-                       if (stack < irq_stack_ptr &&
-                           (stack + sizeof(struct pt_regs)) > irq_stack_ptr)
-                               stack = IRQ_STACK_TO_TASK_STACK(irq_stack_ptr);
+               if (in_entry_text(frame.pc)) {
+                       stack = frame.fp - offsetof(struct pt_regs, stackframe);
 
-                       dump_mem("", "Exception stack", stack,
-                                stack + sizeof(struct pt_regs));
+                       if (on_task_stack(tsk, stack) ||
+                           (tsk == current && !preemptible() && on_irq_stack(stack)))
+                               dump_mem("", "Exception stack", stack,
+                                        stack + sizeof(struct pt_regs));
                }
        }