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);
 }
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to