ARM: enable dumping stacks for CONFIG_SMP

This topic has come up before, see
http://comments.gmane.org/gmane.linux.ports.arm.kernel/102458
for the previous discussion.

SMP is now the norm for new ARM systems, and the limitation that CONFIG_STACKTRACE doesn't work for tasks besides 'current' causes problems. The particular case I'm dealing with is automated debugging information collected from /proc/<pid>/stack when userspace detects a process is not responding.

Dumping stacktraces is currently disabled due to the worry that the task may be running on another CPU and that the unwinder may be unstable when presented with a stack that is being modified. I have attempted to harden the frame pointer based unwinder and the unwind table based unwinder against invalid stacks. I separated the two into individual patches, as I expect the patch to the table unwinder to be more controversial than the frame pointer unwinder.

Even without the hardening, unwinding a stack for a running process is not completely untested. When CONFIG_ARM_UNWIND is enabled, sysrq-t calls unwind_backtrace for all tasks including running ones. In addition, any callers to unwind_frame with preemption enabled, including proc_pid_stack, could see a modified stack even on a UP system (pointed out by Rabin Vincent the last time this topic came up).

v2 of the patchset adds additional checks to avoid starting the stacktrace in an unmapped region or looping forever when unwinding a bad stack.

v2:
add depth parameter to unwind_frame verify that sp changes for non-leaf frames verify that initial sp value is in mapped lowmem verify that stack offsets are in the range
[sizeof(struct thread_info), THREAD_START_SP)

v3:
fix calling sp_addr_valid before page structs are set up

 arch/arm/include/asm/stacktrace.h |    6 ++-
 arch/arm/kernel/process.c         |    2 +-
 arch/arm/kernel/stacktrace.c      |  122 +++++++++++++++++++++++++++++++------
 arch/arm/kernel/time.c            |    3 +-
 arch/arm/kernel/unwind.c          |   46 ++++++++++----
 5 files changed, 145 insertions(+), 34 deletions(-)

Dumping stacktraces is currently disabled in ARM SMP for all tasks except the current task due to the worry that the task may be running on another CPU and that the unwinder may be unstable when presented with a stack that is being modified.

Unwinding with CONFIG_FRAME_POINTER is fairly simple compared to when CONFIG_ARM_UNWIND is set. The next frame's FP and SP registers are read from the stack and can be validated against the current values to ensure that they do not leave the stack and make progress towards the upper end of the stack. This guarantees that accesses do not fault and that execution is bounded.

Add additional validations to the version of unwind_frame used when CONFIG_FRAME_POINTER=y:

Verify that the stack is in a mapped region of kernel memory. Fixes crashes seen in unwind_frame on real systems, although stack corruption caused by memory instability is likely the cause of the invalid sp and fp values.

Fix address comparison to catch fp >= 0xfffffffc correctly. Fixes crash reported by Todd Poynor.

Ensure the stack pointer moves to a higher address between each frame to make sure repeated calls to unwind_frame can't loop forever.

Includes ideas from Dave Martin.

---
v2:
verify that initial sp value is in mapped lowmem verify that stack offsets are in the range
[sizeof(struct thread_info), THREAD_START_SP)
export stack validation functions for use by unwind

v3:
avoid using page structs to convert vaddr to pfn

 arch/arm/include/asm/stacktrace.h |   4 ++
 arch/arm/kernel/stacktrace.c      | 105 +++++++++++++++++++++++++++++++++++---
 2 files changed, 103 insertions(+), 6 deletions(-)

index 4d0a164..6909056 100644
@@ -12,4 +12,8 @@ struct stackframe {
 extern void walk_stackframe(struct stackframe *frame,
 			    int (*fn)(struct stackframe *, void *), void *data);
 
+bool sp_addr_valid(unsigned long sp);
+bool addr_in_stack(unsigned long orig_sp, unsigned long vsp);
+bool sp_in_stack(unsigned long orig_sp, unsigned long vsp);
+
 #endif	/* __ASM_STACKTRACE_H */

1.8.1

Unwinding with CONFIG_ARM_UNWIND is much more complicated than unwinding with CONFIG_FRAME_POINTER, but there are only a few points that require validation in order to avoid faults or infinite loops. Avoiding faults is easy by adding checks to verify that the stack bounds are in lowmem, and that all accesses relative to the stack pointer remain inside the stack.

When CONFIG_FRAME_POINTER is not set it is possible for a leaf frame to have the same SP as its caller, but otherwise the SP of the caller should always be higher than the SP of the callee. Add a depth parameter to unwind_frame to allow it to distinguish leaf frames from non-leaf frames, and return an error if a caller has the same sp as a non-leaf frame callee.

---
v2:
add depth parameter to unwind_frame verify that sp changes for non-leaf frames verify that initial sp value is in mapped lowmem verify that stack offsets are in the range
[sizeof(struct thread_info), THREAD_START_SP)

v3:
no changes

arch/arm/include/asm/stacktrace.h | 2 +-

 arch/arm/kernel/process.c         |  2 +-
 arch/arm/kernel/stacktrace.c      |  6 +++--
 arch/arm/kernel/time.c            |  3 ++-
 arch/arm/kernel/unwind.c          | 46 +++++++++++++++++++++++++++++----------
 5 files changed, 42 insertions(+), 17 deletions(-)

index 6909056..cc48971 100644
@@ -8,7 +8,7 @@ struct stackframe {
 	unsigned long pc;
 };
 
-extern int unwind_frame(struct stackframe *frame);
+extern int unwind_frame(struct stackframe *frame, int depth);
 extern void walk_stackframe(struct stackframe *frame,
 			    int (*fn)(struct stackframe *, void *), void *data);
 

index 41aad9e..10ea483 100644
@@ -633,7 +633,7 @@ unsigned long get_wchan(struct task_struct *p)
 	frame.lr = 0;			/* recovered from the stack */
 	frame.pc = thread_saved_pc(p);
 	do {
-		int ret = unwind_frame(&frame);
+		int ret = unwind_frame(&frame, count);
 		if (ret < 0)
 			return 0;
 		if (!in_sched_functions(frame.pc))

index aba76d5..d897f4c 100644
@@ -101,7 +101,7 @@ bool sp_in_stack(unsigned long orig_sp, unsigned long sp)
  * Note that with framepointer enabled, even the leaf functions have the same
  * prologue and epilogue, therefore we can ignore the LR value in this case.
  */
-int notrace unwind_frame(struct stackframe *frame)
+int notrace unwind_frame(struct stackframe *frame, int depth)
 {
 	unsigned long fp = frame->fp;
 	unsigned long sp = frame->sp;

@@ -139,12 +139,14 @@ int notrace unwind_frame(struct stackframe *frame)
 void notrace walk_stackframe(struct stackframe *frame,
 		     int (*fn)(struct stackframe *, void *), void *data)
 {
+	int depth = 0;
+
 	while (1) {
 		int ret;
 
 		if (fn(frame, data))
 			break;
-		ret = unwind_frame(frame);
+		ret = unwind_frame(frame, depth++);
 		if (ret < 0)
 			break;
 	}

index fe31b22..5f0bf17 100644
@@ -51,6 +51,7 @@
 unsigned long profile_pc(struct pt_regs *regs)
 {
 	struct stackframe frame;
+	int depth = 0;
 
 	if (!in_lock_functions(regs->ARM_pc))
 		return regs->ARM_pc;

@@ -60,7 +61,7 @@ unsigned long profile_pc(struct pt_regs *regs)
 	frame.lr = regs->ARM_lr;
 	frame.pc = regs->ARM_pc;
 	do {
-		int ret = unwind_frame(&frame);
+		int ret = unwind_frame(&frame, depth++);
 		if (ret < 0)
 			return 0;
 	} while (in_lock_functions(frame.pc));

1.8.1

Now that unwind_frame has been hardened against invalid stacks for CONFIG_ARM_UNWIND or CONFIG_FRAME_POINTER, enable dumping the stack on non-current threads for CONFIG_SMP.

---
 arch/arm/kernel/stacktrace.c | 11 -----------
 1 file changed, 11 deletions(-)

index d897f4c..c6e6477 100644
@@ -187,22 +187,11 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 	data.skip = trace->skip;
 
 	if (tsk != current) {
-#ifdef CONFIG_SMP
-		/*
-		 * What guarantees do we have here that 'tsk' is not
-		 * running on another CPU?  For now, ignore it as we
-		 * can't guarantee we won't explode.
-		 */
-		if (trace->nr_entries < trace->max_entries)
-			trace->entries[trace->nr_entries++] = ULONG_MAX;
-		return;
-#else
 		data.no_sched_functions = 1;
 		frame.fp = thread_saved_fp(tsk);
 		frame.sp = thread_saved_sp(tsk);
 		frame.lr = 0;		/* recovered from the stack */
 		frame.pc = thread_saved_pc(tsk);
-#endif
 	} else {
 		register unsigned long current_sp asm ("sp");
 

1.8.1