> Date: Thu, 9 Jan 2020 14:01:32 +0100
> From: Alexander Bluhm <alexander.bl...@gmx.net>
>
> On Thu, Jan 09, 2020 at 12:01:07PM +0100, Mark Kettenis wrote:
> > By splitting this out, the "retain the kernel lock" bit of the
> > comment doesn't make a lot of sense anymore...
>
> Then move the comment to the return where we retain it.
Yes, I think that's better.
ok kettenis@
> Index: arch/amd64/amd64/db_trace.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/db_trace.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 db_trace.c
> --- arch/amd64/amd64/db_trace.c 10 Nov 2019 10:03:33 -0000 1.47
> +++ arch/amd64/amd64/db_trace.c 9 Jan 2020 12:25:54 -0000
> @@ -150,7 +150,7 @@ db_stack_trace_print(db_expr_t addr, int
> name = NULL;
> }
>
> - if (lastframe == 0 && sym == NULL) {
> + if (lastframe == 0 && sym == NULL && callpc != 0) {
> /* Symbol not found, peek at code */
> unsigned long instr = db_get_value(callpc, 8, 0);
>
> Index: arch/amd64/amd64/trap.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/trap.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 trap.c
> --- arch/amd64/amd64/trap.c 6 Sep 2019 12:22:01 -0000 1.77
> +++ arch/amd64/amd64/trap.c 9 Jan 2020 12:53:10 -0000
> @@ -77,6 +77,7 @@
> #include <sys/signal.h>
> #include <sys/syscall.h>
> #include <sys/syscall_mi.h>
> +#include <sys/stdarg.h>
>
> #include <uvm/uvm_extern.h>
>
> @@ -132,6 +133,23 @@ static inline void verify_smap(const cha
> static inline void debug_trap(struct trapframe *_frame, struct proc *_p,
> long _type);
>
> +static inline void
> +fault(const char *format, ...)
> +{
> + static char faultbuf[512];
> + va_list ap;
> +
> + /*
> + * Save the fault info for DDB. Kernel lock protects
> + * faultbuf from being overwritten by another CPU.
> + */
> + va_start(ap, format);
> + vsnprintf(faultbuf, sizeof faultbuf, format, ap);
> + va_end(ap);
> + printf("%s\n", faultbuf);
> + faultstr = faultbuf;
> +}
> +
> /*
> * pageflttrap(frame, usermode): page fault handler
> * Returns non-zero if the fault was handled (possibly by generating
> @@ -160,17 +178,21 @@ pageflttrap(struct trapframe *frame, int
> KERNEL_LOCK();
>
> if (!usermode) {
> - extern struct vm_map *kernel_map;
> -
> /* This will only trigger if SMEP is enabled */
> - if (cr2 <= VM_MAXUSER_ADDRESS && frame->tf_err & PGEX_I)
> - panic("attempt to execute user address %p "
> + if (cr2 <= VM_MAXUSER_ADDRESS && frame->tf_err & PGEX_I) {
> + fault("attempt to execute user address %p "
> "in supervisor mode", (void *)cr2);
> + /* retain kernel lock */
> + return 0;
> + }
> /* This will only trigger if SMAP is enabled */
> if (pcb->pcb_onfault == NULL && cr2 <= VM_MAXUSER_ADDRESS &&
> - frame->tf_err & PGEX_P)
> - panic("attempt to access user address %p "
> + frame->tf_err & PGEX_P) {
> + fault("attempt to access user address %p "
> "in supervisor mode", (void *)cr2);
> + /* retain kernel lock */
> + return 0;
> + }
>
> /*
> * It is only a kernel address space fault iff:
> @@ -211,17 +233,10 @@ pageflttrap(struct trapframe *frame, int
> frame->tf_rip = (u_int64_t)pcb->pcb_onfault;
> return 1;
> } else {
> - /*
> - * Bad memory access in the kernel; save the fault
> - * info for DDB and retain the kernel lock to keep
> - * faultbuf from being overwritten by another CPU.
> - */
> - static char faultbuf[512];
> - snprintf(faultbuf, sizeof faultbuf,
> - "uvm_fault(%p, 0x%llx, 0, %d) -> %x",
> + /* bad memory access in the kernel */
> + fault("uvm_fault(%p, 0x%llx, 0, %d) -> %x",
> map, cr2, ftype, error);
> - printf("%s\n", faultbuf);
> - faultstr = faultbuf;
> + /* retain kernel lock */
> return 0;
> }
> } else {
> @@ -395,7 +410,7 @@ trap_print(struct trapframe *frame, int
> printf(" in %s mode\n", KERNELMODE(frame->tf_cs, frame->tf_rflags) ?
> "supervisor" : "user");
> printf("trap type %d code %llx rip %llx cs %llx rflags %llx cr2 "
> - " %llx cpl %x rsp %llx\n",
> + "%llx cpl %x rsp %llx\n",
> type, frame->tf_err, frame->tf_rip, frame->tf_cs,
> frame->tf_rflags, rcr2(), curcpu()->ci_ilevel, frame->tf_rsp);
> printf("gsbase %p kgsbase %p\n",
> Index: ddb/db_examine.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/ddb/db_examine.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 db_examine.c
> --- ddb/db_examine.c 7 Nov 2019 13:16:25 -0000 1.26
> +++ ddb/db_examine.c 9 Jan 2020 12:25:54 -0000
> @@ -288,8 +288,10 @@ void
> db_print_loc_and_inst(vaddr_t loc)
> {
> db_printsym(loc, DB_STGY_PROC, db_printf);
> - db_printf(":\t");
> - (void) db_disasm(loc, 0);
> + if (loc != 0) {
> + db_printf(":\t");
> + db_disasm(loc, 0);
> + }
> }
>
> /* local copy is needed here so that we can trace strlcpy() in libkern */
>
