svn commit: r368454 - in head/sys: cddl/dev/dtrace/riscv cddl/dev/fbt riscv/include riscv/riscv

[John Baldwin]

Author: jhb
Date: Tue Dec  8 17:57:18 2020
New Revision: 368454
URL: https://svnweb.freebsd.org/changeset/base/368454

Log:
  Stack unwinding robustness fixes for RISC-V.
  
  - Push the kstack_contains check down into unwind_frame() so that it
    is honored by DDB and DTrace.
  
  - Check that the trapframe for an exception frame is contained in the
    traced thread's kernel stack for DDB traces.
  
  Reviewed by:	markj
  Obtained from:	CheriBSD
  Sponsored by:	DARPA
  Differential Revision:	https://reviews.freebsd.org/D27357

Modified:
  head/sys/cddl/dev/dtrace/riscv/dtrace_isa.c
  head/sys/cddl/dev/fbt/fbt.c
  head/sys/riscv/include/stack.h
  head/sys/riscv/riscv/db_trace.c
  head/sys/riscv/riscv/stack_machdep.c
  head/sys/riscv/riscv/unwind.c

Modified: head/sys/cddl/dev/dtrace/riscv/dtrace_isa.c
==============================================================================
--- head/sys/cddl/dev/dtrace/riscv/dtrace_isa.c	Tue Dec  8 17:44:34 2020	(r368453)
+++ head/sys/cddl/dev/dtrace/riscv/dtrace_isa.c	Tue Dec  8 17:57:18 2020	(r368454)
@@ -90,7 +90,7 @@ dtrace_getpcstack(pc_t *pcstack, int pcstack_limit, in
 	state.pc = (uintptr_t)dtrace_getpcstack;
 
 	while (depth < pcstack_limit) {
-		if (unwind_frame(&state))
+		if (!unwind_frame(curthread, &state))
 			break;
 
 		if (!INKERNEL(state.pc) || !INKERNEL(state.fp))
@@ -259,10 +259,10 @@ dtrace_getstackdepth(int aframes)
 	int scp_offset;
 	register_t sp;
 	int depth;
-	int done;
+	bool done;
 
 	depth = 1;
-	done = 0;
+	done = false;
 
 	__asm __volatile("mv %0, sp" : "=&r" (sp));
 
@@ -271,7 +271,7 @@ dtrace_getstackdepth(int aframes)
 	state.pc = (uintptr_t)dtrace_getstackdepth;
 
 	do {
-		done = unwind_frame(&state);
+		done = !unwind_frame(curthread, &state);
 		if (!INKERNEL(state.pc) || !INKERNEL(state.fp))
 			break;
 		depth++;

Modified: head/sys/cddl/dev/fbt/fbt.c
==============================================================================
--- head/sys/cddl/dev/fbt/fbt.c	Tue Dec  8 17:44:34 2020	(r368453)
+++ head/sys/cddl/dev/fbt/fbt.c	Tue Dec  8 17:57:18 2020	(r368454)
@@ -137,6 +137,15 @@ fbt_excluded(const char *name)
 		return (1);
 
 	/*
+	 * Stack unwinders may be called from probe context on some
+	 * platforms.
+	 */
+#if defined(__riscv)
+	if (strcmp(name, "unwind_frame") == 0)
+		return (1);
+#endif
+
+	/*
 	 * When DTrace is built into the kernel we need to exclude
 	 * the FBT functions from instrumentation.
 	 */

Modified: head/sys/riscv/include/stack.h
==============================================================================
--- head/sys/riscv/include/stack.h	Tue Dec  8 17:44:34 2020	(r368453)
+++ head/sys/riscv/include/stack.h	Tue Dec  8 17:57:18 2020	(r368454)
@@ -46,6 +46,6 @@ struct unwind_state {
 	uintptr_t pc;
 };
 
-int unwind_frame(struct unwind_state *);
+bool unwind_frame(struct thread *, struct unwind_state *);
 
 #endif /* !_MACHINE_STACK_H_ */

Modified: head/sys/riscv/riscv/db_trace.c
==============================================================================
--- head/sys/riscv/riscv/db_trace.c	Tue Dec  8 17:44:34 2020	(r368453)
+++ head/sys/riscv/riscv/db_trace.c	Tue Dec  8 17:57:18 2020	(r368454)
@@ -73,7 +73,7 @@ db_md_set_watchpoint(db_expr_t addr, db_expr_t size)
 }
 
 static void
-db_stack_trace_cmd(struct unwind_state *frame)
+db_stack_trace_cmd(struct thread *td, struct unwind_state *frame)
 {
 	const char *name;
 	db_expr_t offset;
@@ -100,6 +100,11 @@ db_stack_trace_cmd(struct unwind_state *frame)
 			struct trapframe *tf;
 
 			tf = (struct trapframe *)(uintptr_t)frame->sp;
+			if (!kstack_contains(td, (vm_offset_t)tf,
+			    sizeof(*tf))) {
+				db_printf("--- invalid trapframe %p\n", tf);
+				break;
+			}
 
 			if ((tf->tf_scause & SCAUSE_INTR) != 0)
 				db_printf("--- interrupt %ld\n",
@@ -119,7 +124,7 @@ db_stack_trace_cmd(struct unwind_state *frame)
 		if (strcmp(name, "fork_trampoline") == 0)
 			break;
 
-		if (unwind_frame(frame) < 0)
+		if (!unwind_frame(td, frame))
 			break;
 	}
 }
@@ -135,7 +140,7 @@ db_trace_thread(struct thread *thr, int count)
 	frame.sp = ctx->pcb_sp;
 	frame.fp = ctx->pcb_s[0];
 	frame.pc = ctx->pcb_ra;
-	db_stack_trace_cmd(&frame);
+	db_stack_trace_cmd(thr, &frame);
 	return (0);
 }
 
@@ -150,5 +155,5 @@ db_trace_self(void)
 	frame.sp = sp;
 	frame.fp = (uintptr_t)__builtin_frame_address(0);
 	frame.pc = (uintptr_t)db_trace_self;
-	db_stack_trace_cmd(&frame);
+	db_stack_trace_cmd(curthread, &frame);
 }

Modified: head/sys/riscv/riscv/stack_machdep.c
==============================================================================
--- head/sys/riscv/riscv/stack_machdep.c	Tue Dec  8 17:44:34 2020	(r368453)
+++ head/sys/riscv/riscv/stack_machdep.c	Tue Dec  8 17:57:18 2020	(r368454)
@@ -53,10 +53,8 @@ stack_capture(struct thread *td, struct stack *st, str
 	stack_zero(st);
 
 	while (1) {
-		if (!kstack_contains(td, (vm_offset_t)frame->fp -
-		    (sizeof(uintptr_t) * 2), sizeof(uintptr_t) * 2))
+		if (!unwind_frame(td, frame))
 			break;
-		unwind_frame(frame);
 		if (!INKERNEL((vm_offset_t)frame->pc))
 			break;
 		if (stack_put(st, frame->pc) == -1)

Modified: head/sys/riscv/riscv/unwind.c
==============================================================================
--- head/sys/riscv/riscv/unwind.c	Tue Dec  8 17:44:34 2020	(r368453)
+++ head/sys/riscv/riscv/unwind.c	Tue Dec  8 17:57:18 2020	(r368454)
@@ -35,23 +35,24 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD$");
 #include <sys/param.h>
+#include <sys/proc.h>
 
 #include <machine/stack.h>
 #include <machine/vmparam.h>
 
-int
-unwind_frame(struct unwind_state *frame)
+bool
+unwind_frame(struct thread *td, struct unwind_state *frame)
 {
 	uintptr_t fp;
 
 	fp = frame->fp;
 
-	if (!INKERNEL(fp))
-		return (-1);
+	if (!kstack_contains(td, fp - sizeof(fp) * 2, sizeof(fp) * 2))
+		return (false);
 
 	frame->sp = fp;
 	frame->fp = ((uintptr_t *)fp)[-2];
 	frame->pc = ((uintptr_t *)fp)[-1] - 4;
 
-	return (0);
+	return (true);
 }