Author: jhb
Date: Tue Dec  8 18:00:58 2020
New Revision: 368455
URL: https://svnweb.freebsd.org/changeset/base/368455

Log:
  Check that the frame pointer is within the current stack.
  
  This same check is used on other architectures.  Previously this would
  permit a stack frame to unwind into any arbitrary kernel address
  (including unmapped addresses).
  
  Reviewed by:  andrew, markj
  Obtained from:        CheriBSD
  Sponsored by: DARPA
  Differential Revision:        https://reviews.freebsd.org/D27362

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

Modified: head/sys/arm64/arm64/db_trace.c
==============================================================================
--- head/sys/arm64/arm64/db_trace.c     Tue Dec  8 17:57:18 2020        
(r368454)
+++ head/sys/arm64/arm64/db_trace.c     Tue Dec  8 18:00:58 2020        
(r368455)
@@ -65,7 +65,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)
 {
        c_db_sym_t sym;
        const char *name;
@@ -74,10 +74,8 @@ db_stack_trace_cmd(struct unwind_state *frame)
 
        while (1) {
                uintptr_t pc = frame->pc;
-               int ret;
 
-               ret = unwind_frame(frame);
-               if (ret < 0)
+               if (!unwind_frame(td, frame))
                        break;
 
                sym = db_search_symbol(pc, DB_STGY_ANY, &offset);
@@ -112,7 +110,7 @@ db_trace_thread(struct thread *thr, int count)
                frame.sp = (uintptr_t)ctx->pcb_sp;
                frame.fp = (uintptr_t)ctx->pcb_x[29];
                frame.pc = (uintptr_t)ctx->pcb_x[30];
-               db_stack_trace_cmd(&frame);
+               db_stack_trace_cmd(thr, &frame);
        } else
                db_trace_self();
        return (0);
@@ -129,5 +127,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/arm64/arm64/stack_machdep.c
==============================================================================
--- head/sys/arm64/arm64/stack_machdep.c        Tue Dec  8 17:57:18 2020        
(r368454)
+++ head/sys/arm64/arm64/stack_machdep.c        Tue Dec  8 18:00:58 2020        
(r368455)
@@ -43,15 +43,15 @@ __FBSDID("$FreeBSD$");
 #include <machine/stack.h>
 
 static void
-stack_capture(struct stack *st, struct unwind_state *frame)
+stack_capture(struct thread *td, struct stack *st, struct unwind_state *frame)
 {
 
        stack_zero(st);
        while (1) {
-               unwind_frame(frame);
-               if (!INKERNEL((vm_offset_t)frame->fp) ||
-                    !INKERNEL((vm_offset_t)frame->pc))
+               if (!unwind_frame(td, frame))
                        break;
+               if (!INKERNEL((vm_offset_t)frame->pc))
+                       break;
                if (stack_put(st, frame->pc) == -1)
                        break;
        }
@@ -73,7 +73,7 @@ stack_save_td(struct stack *st, struct thread *td)
        frame.fp = td->td_pcb->pcb_x[29];
        frame.pc = td->td_pcb->pcb_x[30];
 
-       stack_capture(st, &frame);
+       stack_capture(td, st, &frame);
        return (0);
 }
 
@@ -89,5 +89,5 @@ stack_save(struct stack *st)
        frame.fp = (uintptr_t)__builtin_frame_address(0);
        frame.pc = (uintptr_t)stack_save;
 
-       stack_capture(st, &frame);
+       stack_capture(curthread, st, &frame);
 }

Modified: head/sys/arm64/arm64/unwind.c
==============================================================================
--- head/sys/arm64/arm64/unwind.c       Tue Dec  8 17:57:18 2020        
(r368454)
+++ head/sys/arm64/arm64/unwind.c       Tue Dec  8 18:00:58 2020        
(r368455)
@@ -30,24 +30,26 @@
 #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(uintptr_t) * 2))
+               return (false);
+
        frame->sp = fp + sizeof(uintptr_t) * 2;
        /* FP to previous frame (X29) */
        frame->fp = ((uintptr_t *)fp)[0];
        /* LR (X30) */
        frame->pc = ((uintptr_t *)fp)[1] - 4;
 
-       return (0);
+       return (true);
 }

Modified: head/sys/arm64/include/csan.h
==============================================================================
--- head/sys/arm64/include/csan.h       Tue Dec  8 17:57:18 2020        
(r368454)
+++ head/sys/arm64/include/csan.h       Tue Dec  8 18:00:58 2020        
(r368455)
@@ -87,9 +87,9 @@ kcsan_md_unwind(void)
        nsym = 0;
 
        while (1) {
-               unwind_frame(&frame);
-               if (!INKERNEL((vm_offset_t)frame.fp) ||
-                    !INKERNEL((vm_offset_t)frame.pc))
+               if (!unwind_frame(curthread, &frame))
+                       break;
+               if (!INKERNEL((vm_offset_t)frame.pc))
                        break;
 
 #ifdef DDB

Modified: head/sys/arm64/include/stack.h
==============================================================================
--- head/sys/arm64/include/stack.h      Tue Dec  8 17:57:18 2020        
(r368454)
+++ head/sys/arm64/include/stack.h      Tue Dec  8 18:00:58 2020        
(r368455)
@@ -38,6 +38,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/cddl/dev/dtrace/aarch64/dtrace_isa.c
==============================================================================
--- head/sys/cddl/dev/dtrace/aarch64/dtrace_isa.c       Tue Dec  8 17:57:18 
2020        (r368454)
+++ head/sys/cddl/dev/dtrace/aarch64/dtrace_isa.c       Tue Dec  8 18:00:58 
2020        (r368455)
@@ -70,7 +70,7 @@ dtrace_getpcstack(pc_t *pcstack, int pcstack_limit, in
 {
        struct unwind_state state;
        int scp_offset;
-       register_t sp, fp;
+       register_t sp;
        int depth;
 
        depth = 0;
@@ -88,16 +88,11 @@ dtrace_getpcstack(pc_t *pcstack, int pcstack_limit, in
        state.pc = (uintptr_t)dtrace_getpcstack;
 
        while (depth < pcstack_limit) {
-               if (!INKERNEL(state.pc) || !INKERNEL(state.fp))
+               if (!unwind_frame(curthread, &state))
                        break;
+               if (!INKERNEL(state.pc))
+                       break;
 
-               fp = state.fp;
-               state.sp = fp + 0x10;
-               /* FP to previous frame (X29) */
-               state.fp = *(register_t *)(fp);
-               /* LR (X30) */
-               state.pc = *(register_t *)(fp + 8) - 4;
-
                /*
                 * NB: Unlike some other architectures, we don't need to
                 * explicitly insert cpu_dtrace_caller as it appears in the
@@ -274,10 +269,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("mov %0, sp" : "=&r" (sp));
 
@@ -286,7 +281,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:57:18 2020        (r368454)
+++ head/sys/cddl/dev/fbt/fbt.c Tue Dec  8 18:00:58 2020        (r368455)
@@ -140,7 +140,7 @@ fbt_excluded(const char *name)
         * Stack unwinders may be called from probe context on some
         * platforms.
         */
-#if defined(__riscv)
+#if defined(__aarch64__) || defined(__riscv)
        if (strcmp(name, "unwind_frame") == 0)
                return (1);
 #endif
_______________________________________________
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