kern/57550: amd64: Setting a watch point on a buffer for read(2) causes the kernel to panic

[pho%netbsd.org@localhost]

>Number:         57550
>Category:       kern
>Synopsis:       amd64: Setting a watch point on a buffer for read(2) causes the kernel to panic
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 30 03:45:00 +0000 2023
>Originator:     PHO
>Release:        -CURRENT
>Organization:
TNF
>Environment:
System: NetBSD netbsd-current.home.cielonegro.org 10.99.6 NetBSD 10.99.6 (GENERIC) #0: Sun Jul 23 21:06:54 JST 2023  pho%netbsd-current.home.cielonegro.org@localhost:/home/pho/sysbuild/amd64/obj/usr/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
When a hardware watch point is set in a memory region of a user process, and it's passed to a syscall, the kernel panics instead of generating a SIGTRAP like this:
---
[ 213838.7597637] fatal trace trap in supervisor mode
[ 213838.7597637] trap type 5 code 0 rip 0xffffffff80234353 cs 0x8 rflags 0x50202 cr2 0x2bd4df ilevel 0 rsp 0xffffde0139397ca8
[ 213838.7597637] curlwp 0xfffffee02d9c3940 pid 29253.29253 lowest kstack 0xffffde01393932c0
Stopped in pid 29253.29253 (hack) at	netbsd:copyout+0x33:	repe movsq	(%rsi),%es:(%rdi)
copyout() at netbsd:copyout+0x33
ubc_uiomove() at netbsd:ubc_uiomove+0x165
ffs_read() at netbsd:ffs_read+0xf0
VOP_READ() at netbsd:VOP_READ+0x42
vn_read() at netbsd:vn_read+0x16f
dofileread() at netbsd:dofileread+0x79
sys_read() at netbsd:sys_read+0x49
syscall() at netbsd:syscall+0x196
[ 213838.7597637] --- syscall (number 3) ---
---

The issue is confirmed to affect netbsd-10 and netbsd-9. It possibly affects netbsd-8 too.
>How-To-Repeat:
Compile the following program with -g:
---
#include <fcntl.h>
#include <unistd.h>

char buf[16];

int main(void) {
    int fd = open("/dev/urandom", O_RDONLY);
    read(fd, buf, sizeof(buf));
    return 0;
}
---

Open it in gdb, do "watch buf" and "run".
>Fix:
The following patch at least fixes the panic, but still doesn't generate SIGTRAP. Help, I couldn't figure out why:

diff --git a/sys/arch/amd64/amd64/trap.c b/sys/arch/amd64/amd64/trap.c
index 6e555545ecd..2645c3f0acf 100644
--- a/sys/arch/amd64/amd64/trap.c
+++ b/sys/arch/amd64/amd64/trap.c
@@ -617,19 +617,15 @@ pagefltcommon:
 
 	case T_TRCTRAP:
 		/*
-		 * Ignore debug register trace traps due to
+		 * Handle debug register trace traps due to
 		 * accesses in the user's address space, which
 		 * can happen under several conditions such as
 		 * if a user sets a watchpoint on a buffer and
 		 * then passes that buffer to a system call.
-		 * We still want to get TRCTRAPS for addresses
-		 * in kernel space because that is useful when
-		 * debugging the kernel.
 		 */
-		if (x86_dbregs_user_trap())
-			break;
-
-		goto we_re_toast;
+		if (!x86_dbregs_user_trap())
+			goto we_re_toast;
+		/* FALLTHROUGH */
 
 	case T_BPTFLT|T_USER:		/* bpt instruction fault */
 	case T_TRCTRAP|T_USER:		/* trace trap */
diff --git a/sys/arch/i386/i386/trap.c b/sys/arch/i386/i386/trap.c
index 98769168f5c..eff4b78f73a 100644
--- a/sys/arch/i386/i386/trap.c
+++ b/sys/arch/i386/i386/trap.c
@@ -732,19 +732,15 @@ faultcommon:
 
 	case T_TRCTRAP:
 		/*
-		 * Ignore debug register trace traps due to
+		 * Handle debug register trace traps due to
 		 * accesses in the user's address space, which
 		 * can happen under several conditions such as
 		 * if a user sets a watchpoint on a buffer and
 		 * then passes that buffer to a system call.
-		 * We still want to get TRCTRAPS for addresses
-		 * in kernel space because that is useful when
-		 * debugging the kernel.
 		 */
-		if (x86_dbregs_user_trap())
-			break;
-
-		goto we_re_toast;
+		if (!x86_dbregs_user_trap())
+			goto we_re_toast;
+		/* FALLTHROUGH */
 
 	case T_BPTFLT|T_USER:		/* bpt instruction fault */
 	case T_TRCTRAP|T_USER:		/* trace trap */
diff --git a/sys/arch/x86/x86/dbregs.c b/sys/arch/x86/x86/dbregs.c
index b473c696cc4..2bb3dc6c710 100644
--- a/sys/arch/x86/x86/dbregs.c
+++ b/sys/arch/x86/x86/dbregs.c
@@ -47,7 +47,11 @@ static struct dbreg initdbstate;
 	X86_DR6_DR2_BREAKPOINT_CONDITION_DETECTED | \
 	X86_DR6_DR3_BREAKPOINT_CONDITION_DETECTED )
 
-#define X86_GLOBAL_BREAKPOINT	( \
+#define X86_ANY_BREAKPOINTS	( \
+	X86_DR7_LOCAL_DR0_BREAKPOINT | \
+	X86_DR7_LOCAL_DR1_BREAKPOINT | \
+	X86_DR7_LOCAL_DR2_BREAKPOINT | \
+	X86_DR7_LOCAL_DR3_BREAKPOINT | \
 	X86_DR7_GLOBAL_DR0_BREAKPOINT | \
 	X86_DR7_GLOBAL_DR1_BREAKPOINT | \
 	X86_DR7_GLOBAL_DR2_BREAKPOINT | \
@@ -202,9 +206,9 @@ x86_dbregs_user_trap(void)
 	register_t bp;
 
 	dr7 = rdr7();
-	if ((dr7 & X86_GLOBAL_BREAKPOINT) == 0) {
+	if ((dr7 & X86_ANY_BREAKPOINTS) == 0) {
 		/*
-		 * All Global Breakpoint bits are zero, thus the trap couldn't
+		 * All Breakpoint bits are zero, thus the trap couldn't
 		 * have been caused by the hardware debug registers.
 		 */
 		return 0;