[src/trunk]: src/sys/arch/amd64/amd64 ddb/amd64: Don't go out of the way to d...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/cf5bcb13ae54
branches:  trunk
changeset: 1021281:cf5bcb13ae54
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun May 23 08:59:08 2021 +0000

description:
ddb/amd64: Don't go out of the way to detect invalid addresses.

db_disasm had logic to detect invalid addresses before trying to
disassemble them.  But when disassembling a null instruction address,
the logic to detect invalid addresses itself tried to dereference an
invalid address.

db_get_value can already handle this situation gracefully, so there is
no need for this faulty fault-avoidance logic.

Fixes double-fault in ddb on calling null function pointers.  With
any luck, this should make diagnosing such bugs easier in the future!

diffstat:

 sys/arch/amd64/amd64/db_disasm.c |  29 ++---------------------------
 1 files changed, 2 insertions(+), 27 deletions(-)

diffs (52 lines):

diff -r 56f4226baedb -r cf5bcb13ae54 sys/arch/amd64/amd64/db_disasm.c
--- a/sys/arch/amd64/amd64/db_disasm.c  Sun May 23 08:42:47 2021 +0000
+++ b/sys/arch/amd64/amd64/db_disasm.c  Sun May 23 08:59:08 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_disasm.c,v 1.27 2019/03/09 08:42:25 maxv Exp $      */
+/*     $NetBSD: db_disasm.c,v 1.28 2021/05/23 08:59:08 riastradh Exp $ */
 
 /* 
  * Mach Operating System
@@ -33,7 +33,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.27 2019/03/09 08:42:25 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.28 2021/05/23 08:59:08 riastradh Exp $");
 
 #ifndef _KERNEL
 #include <sys/types.h>
@@ -1191,33 +1191,8 @@
        uint64_t imm64;
        int     len;
        struct i_addr   address;
-#ifdef _KERNEL
-       pt_entry_t *pte, *pde;
-#endif
        u_int   rex = 0;
 
-#ifdef _KERNEL
-       /*
-        * Don't try to disassemble the location if the mapping is invalid.
-        * If we do, we'll fault, and end up debugging the debugger!
-        * in the case of largepages, "pte" is really the pde and "pde" is
-        * really the entry for the pdp itself.
-        */
-       if ((vaddr_t)loc >= VM_MIN_KERNEL_ADDRESS)
-               pte = kvtopte((vaddr_t)loc);
-       else
-               pte = vtopte((vaddr_t)loc);
-       if ((vaddr_t)pte >= VM_MIN_KERNEL_ADDRESS)
-               pde = kvtopte((vaddr_t)pte);
-       else
-               pde = vtopte((vaddr_t)pte);
-
-       if ((*pde & PTE_P) == 0 || (*pte & PTE_P) == 0) {
-               db_printf("invalid address\n");
-               return (loc);
-       }
-#endif
-
        get_value_inc(inst, loc, 1, false);
        short_addr = false;
        size = LONG;