Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/arch Improve ddb disassembly for mips (and riscv, cribbe...



details:   https://anonhg.NetBSD.org/src/rev/07d96a961da3
branches:  trunk
changeset: 983439:07d96a961da3
user:      dholland <dholland%NetBSD.org@localhost>
date:      Sun May 23 23:22:55 2021 +0000

description:
Improve ddb disassembly for mips (and riscv, cribbed from mips).

- use db_read_bytes to get instructions
- move the address check logic previously attached only to fetching
  instructions for disassembly to db_read_bytes (and db_write_bytes)

Motivated by related x86 changes this afternoon.

Note that the address check logic is not as sophisticated as what the
x86 code does, but it's what we had before. (Except that riscv will
now also try to fetch usermode instructions instead of just failing.)

diffstat:

 sys/arch/mips/mips/db_disasm.c    |  29 ++++++++++-------------------
 sys/arch/mips/mips/db_interface.c |  33 +++++++++++++++++++++++++++++++--
 sys/arch/riscv/riscv/db_disasm.c  |  18 ++++++------------
 sys/arch/riscv/riscv/db_machdep.c |  32 ++++++++++++++++++++++++++++++--
 4 files changed, 77 insertions(+), 35 deletions(-)

diffs (253 lines):

diff -r 1bc80367e678 -r 07d96a961da3 sys/arch/mips/mips/db_disasm.c
--- a/sys/arch/mips/mips/db_disasm.c    Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/mips/mips/db_disasm.c    Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_disasm.c,v 1.42 2021/04/12 11:35:22 simonb Exp $    */
+/*     $NetBSD: db_disasm.c,v 1.43 2021/05/23 23:22:55 dholland Exp $  */
 
 /*-
  * Copyright (c) 1991, 1993
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.42 2021/04/12 11:35:22 simonb Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.43 2021/05/23 23:22:55 dholland Exp $");
 
 #include <sys/param.h>
 #include <sys/cpu.h>
@@ -47,6 +47,7 @@
 
 #include <machine/db_machdep.h>
 
+#include <ddb/db_access.h>
 #include <ddb/db_user.h>
 #include <ddb/db_interface.h>
 #include <ddb/db_output.h>
@@ -218,11 +219,6 @@
  * "next instruction" does NOT mean the next instruction to
  * be executed but the 'linear' next instruction.
  */
-#ifdef _LP64
-#define        DISASM_KERN_START       MIPS_XKSEG_START
-#else
-#define        DISASM_KERN_START       MIPS_KSEG0_START
-#endif
 db_addr_t
 db_disasm(db_addr_t loc, bool altfmt)
 {
@@ -232,19 +228,14 @@
         * Take some care with addresses to not UTLB here as it
         * loses the current debugging context.  KSEG2 and XKSEG
         * are not checked.
+        * Update: db_read_bytes is supposed to do that, and now
+        * does, so we can use that.
+        *
+        * XXX db_read_bytes_can't return failure but instead zeros
+        * the output. That's ok here, but if ever improved the
+        * proper thing here on error is to return the original loc.
         */
-       if (loc < (db_addr_t)DISASM_KERN_START) {
-#ifdef _KERNEL
-               if (ufetch_32((void *)loc, &instr) != 0) {
-                       db_printf("invalid address.\n");
-                       return loc;
-               }
-#else
-               return loc;
-#endif
-       } else {
-               instr =  *(uint32_t *)loc;
-       }
+       db_read_bytes(loc, sizeof(instr), (void *)&instr);
 
        return (db_disasm_insn(instr, loc, altfmt));
 }
diff -r 1bc80367e678 -r 07d96a961da3 sys/arch/mips/mips/db_interface.c
--- a/sys/arch/mips/mips/db_interface.c Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/mips/mips/db_interface.c Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_interface.c,v 1.93 2021/04/12 02:23:41 mrg Exp $    */
+/*     $NetBSD: db_interface.c,v 1.94 2021/05/23 23:22:55 dholland Exp $       */
 
 /*
  * Mach Operating System
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_interface.c,v 1.93 2021/04/12 02:23:41 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_interface.c,v 1.94 2021/05/23 23:22:55 dholland Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_multiprocessor.h"
@@ -180,6 +180,23 @@
 db_read_bytes(vaddr_t addr, size_t size, char *data)
 {
        const char *src = (char *)addr;
+       int err;
+
+       /*
+        * If asked to fetch from userspace, do it safely.
+        * Note that MIPS_KSEG0_START is the proper address for
+        * both 32-bit and 64-bit kernels.
+        */
+       if (addr < (vaddr_t)MIPS_KSEG0_START) {
+               err = copyin(src, data, size);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", src);
+#endif
+                       memset(data, 0, size);
+               }
+               return;
+       }
 
        if (size <= 8 && (size & (size-1)) == 0 && (addr & (size-1)) == 0
            && ((uintptr_t)data & (size-1)) == 0) {
@@ -205,6 +222,18 @@
 {
        char *p = (char *)addr;
        size_t n = size;
+       int err;
+
+       /* If asked to fetch from userspace, do it safely */
+       if (addr < (vaddr_t)MIPS_KSEG0_START) {
+               err = copyout(data, p, size);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", p);
+#endif
+               }
+               return;
+       }
 
        if (size <= 8 && (size & (size-1)) == 0 && (addr & (size-1)) == 0
            && ((uintptr_t)data & (size-1)) == 0) {
diff -r 1bc80367e678 -r 07d96a961da3 sys/arch/riscv/riscv/db_disasm.c
--- a/sys/arch/riscv/riscv/db_disasm.c  Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/riscv/riscv/db_disasm.c  Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_disasm.c,v 1.7 2021/05/01 06:48:51 skrll Exp $      */
+/*     $NetBSD: db_disasm.c,v 1.8 2021/05/23 23:22:55 dholland Exp $   */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
 
 #include <sys/cdefs.h>
 
-__RCSID("$NetBSD: db_disasm.c,v 1.7 2021/05/01 06:48:51 skrll Exp $");
+__RCSID("$NetBSD: db_disasm.c,v 1.8 2021/05/23 23:22:55 dholland Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -39,6 +39,7 @@
 #include <riscv/db_machdep.h>
 #include <riscv/insn.h>
 
+#include <ddb/db_access.h>
 #include <ddb/db_user.h>
 #include <ddb/db_interface.h>
 #include <ddb/db_output.h>
@@ -1463,23 +1464,16 @@
        unsigned n, i;
        uint32_t insn32;
 
-#ifdef _KERNEL
-       if ((intptr_t) loc >= 0) {
-               db_printf("%s: %#"PRIxVADDR" is not a kernel address\n",
-                   __func__, loc);
-               return loc;
-       }
-#endif
-
        /*
         * Fetch the instruction. The first halfword tells us how many
         * more there are, and they're always in little-endian order.
         */
-       insn[0] = ((const uint16_t *)loc)[0];
+       db_read_bytes(loc, sizeof(insn[0]), (void *)&insn[0]);
        n = INSN_HALFWORDS(insn[0]);
        KASSERT(n > 0 && n <= 5);
        for (i = 1; i < n; i++) {
-               insn[i] = ((const uint16_t *)loc)[i];
+               db_read_bytes(loc + i * sizeof(insn[i]), sizeof(insn[i]),
+                             (void *)&insn[i]);
        }
 
        switch (n) {
diff -r 1bc80367e678 -r 07d96a961da3 sys/arch/riscv/riscv/db_machdep.c
--- a/sys/arch/riscv/riscv/db_machdep.c Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/riscv/riscv/db_machdep.c Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_machdep.c,v 1.7 2021/04/14 06:32:20 dholland Exp $  */
+/*     $NetBSD: db_machdep.c,v 1.8 2021/05/23 23:22:55 dholland Exp $  */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
 
 #include <sys/cdefs.h>
 
-__RCSID("$NetBSD: db_machdep.c,v 1.7 2021/04/14 06:32:20 dholland Exp $");
+__RCSID("$NetBSD: db_machdep.c,v 1.8 2021/05/23 23:22:55 dholland Exp $");
 
 #include <sys/param.h>
 
@@ -42,6 +42,7 @@
 #include <ddb/db_interface.h>
 #include <ddb/db_extern.h>
 #include <ddb/db_variables.h>
+#include <ddb/db_output.h>
 
 int db_active = 0;
 
@@ -225,6 +226,19 @@
 db_read_bytes(db_addr_t addr, size_t len, char *data)
 {
        const char *src = (char *)addr;
+       int err;
+
+       /* If asked to fetch from userspace, do it safely */
+       if ((intptr_t)addr >= 0) {
+               err = copyin(src, data, len);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", src);
+#endif
+                       memset(data, 0, len);
+               }
+               return;
+       }
 
        while (len--) {
                *data++ = *src++;
@@ -237,6 +251,19 @@
 void
 db_write_bytes(vaddr_t addr, size_t len, const char *data)
 {
+       int err;
+
+       /* If asked to fetch from userspace, do it safely */
+       if ((intptr_t)addr >= 0) {
+               err = copyout(data, (char *)addr, len);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", (char *)addr);
+#endif
+               }
+               return;
+       }
+
        if (len == 8) {
                *(uint64_t *)addr = *(const uint64_t *) data;
        } else if (len == 4) {
@@ -244,6 +271,7 @@
        } else if (len == 2) {
                *(uint16_t *)addr = *(const uint16_t *) data;
        } else {
+               KASSERT(len == 1);
                *(uint8_t *)addr = *(const uint8_t *) data;
        }
        __asm("fence rw,rw; fence.i");



Home | Main Index | Thread Index | Old Index