From 39092e79edeaa61d26a34b1decbe2a6b95697f6a Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Tue, 14 Mar 2006 16:13:55 +0000 Subject: [PATCH] Don't allow userland to set hardware watch points on kernel memory at all. Previously, we tried to allow this only for root. However, we were calling suser() on the *target* process rather than the current process. This means that if you can ptrace() a process running as root you can set a hardware watch point in the kernel. In practice I think you probably have to be root in order to pass the p_candebug() checks in ptrace() to attach to a process running as root anyway. Rather than fix the suser(), I just axed the entire idea, as I can't think of any good reason _at all_ for userland to set hardware watch points for KVM. MFC after: 3 days Also thinks hardware watch points on KVM from userland are bad: bde, rwatson --- sys/amd64/amd64/machdep.c | 43 ++++++++++++++++++--------------------- sys/i386/i386/machdep.c | 43 ++++++++++++++++++--------------------- sys/pc98/pc98/machdep.c | 43 ++++++++++++++++++--------------------- 3 files changed, 60 insertions(+), 69 deletions(-) diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c index 964b95fbea9..67364be89e3 100644 --- a/sys/amd64/amd64/machdep.c +++ b/sys/amd64/amd64/machdep.c @@ -1749,8 +1749,7 @@ set_dbregs(struct thread *td, struct dbreg *dbregs) * could halt the system by setting a breakpoint in the kernel * (if ddb was enabled). Thus, we need to check to make sure * that no breakpoints are being enabled for addresses outside - * process's address space, unless, perhaps, we were called by - * uid 0. + * process's address space. * * XXX - what about when the watched area of the user's * address space is written into from within the kernel @@ -1758,27 +1757,25 @@ set_dbregs(struct thread *td, struct dbreg *dbregs) * from within kernel mode? */ - if (suser(td) != 0) { - if (dbregs->dr[7] & 0x3) { - /* dr0 is enabled */ - if (dbregs->dr[0] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } - if (dbregs->dr[7] & 0x3<<2) { - /* dr1 is enabled */ - if (dbregs->dr[1] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } - if (dbregs->dr[7] & 0x3<<4) { - /* dr2 is enabled */ - if (dbregs->dr[2] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } - if (dbregs->dr[7] & 0x3<<6) { - /* dr3 is enabled */ - if (dbregs->dr[3] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & 0x3) { + /* dr0 is enabled */ + if (dbregs->dr[0] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } + if (dbregs->dr[7] & 0x3<<2) { + /* dr1 is enabled */ + if (dbregs->dr[1] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } + if (dbregs->dr[7] & 0x3<<4) { + /* dr2 is enabled */ + if (dbregs->dr[2] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } + if (dbregs->dr[7] & 0x3<<6) { + /* dr3 is enabled */ + if (dbregs->dr[3] >= VM_MAXUSER_ADDRESS) + return (EINVAL); } pcb->pcb_dr0 = dbregs->dr[0]; diff --git a/sys/i386/i386/machdep.c b/sys/i386/i386/machdep.c index 80a16a44547..ab431d35dc4 100644 --- a/sys/i386/i386/machdep.c +++ b/sys/i386/i386/machdep.c @@ -2801,8 +2801,7 @@ set_dbregs(struct thread *td, struct dbreg *dbregs) * could halt the system by setting a breakpoint in the kernel * (if ddb was enabled). Thus, we need to check to make sure * that no breakpoints are being enabled for addresses outside - * process's address space, unless, perhaps, we were called by - * uid 0. + * process's address space. * * XXX - what about when the watched area of the user's * address space is written into from within the kernel @@ -2810,30 +2809,28 @@ set_dbregs(struct thread *td, struct dbreg *dbregs) * from within kernel mode? */ - if (suser(td) != 0) { - if (dbregs->dr[7] & 0x3) { - /* dr0 is enabled */ - if (dbregs->dr[0] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & 0x3) { + /* dr0 is enabled */ + if (dbregs->dr[0] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } - if (dbregs->dr[7] & (0x3<<2)) { - /* dr1 is enabled */ - if (dbregs->dr[1] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & (0x3<<2)) { + /* dr1 is enabled */ + if (dbregs->dr[1] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } - if (dbregs->dr[7] & (0x3<<4)) { - /* dr2 is enabled */ - if (dbregs->dr[2] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & (0x3<<4)) { + /* dr2 is enabled */ + if (dbregs->dr[2] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } - if (dbregs->dr[7] & (0x3<<6)) { - /* dr3 is enabled */ - if (dbregs->dr[3] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & (0x3<<6)) { + /* dr3 is enabled */ + if (dbregs->dr[3] >= VM_MAXUSER_ADDRESS) + return (EINVAL); } pcb->pcb_dr0 = dbregs->dr[0]; diff --git a/sys/pc98/pc98/machdep.c b/sys/pc98/pc98/machdep.c index 7b98290d980..fe6cfa5937d 100644 --- a/sys/pc98/pc98/machdep.c +++ b/sys/pc98/pc98/machdep.c @@ -2632,8 +2632,7 @@ set_dbregs(struct thread *td, struct dbreg *dbregs) * could halt the system by setting a breakpoint in the kernel * (if ddb was enabled). Thus, we need to check to make sure * that no breakpoints are being enabled for addresses outside - * process's address space, unless, perhaps, we were called by - * uid 0. + * process's address space. * * XXX - what about when the watched area of the user's * address space is written into from within the kernel @@ -2641,30 +2640,28 @@ set_dbregs(struct thread *td, struct dbreg *dbregs) * from within kernel mode? */ - if (suser(td) != 0) { - if (dbregs->dr[7] & 0x3) { - /* dr0 is enabled */ - if (dbregs->dr[0] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & 0x3) { + /* dr0 is enabled */ + if (dbregs->dr[0] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } - if (dbregs->dr[7] & (0x3<<2)) { - /* dr1 is enabled */ - if (dbregs->dr[1] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & (0x3<<2)) { + /* dr1 is enabled */ + if (dbregs->dr[1] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } - if (dbregs->dr[7] & (0x3<<4)) { - /* dr2 is enabled */ - if (dbregs->dr[2] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & (0x3<<4)) { + /* dr2 is enabled */ + if (dbregs->dr[2] >= VM_MAXUSER_ADDRESS) + return (EINVAL); + } - if (dbregs->dr[7] & (0x3<<6)) { - /* dr3 is enabled */ - if (dbregs->dr[3] >= VM_MAXUSER_ADDRESS) - return (EINVAL); - } + if (dbregs->dr[7] & (0x3<<6)) { + /* dr3 is enabled */ + if (dbregs->dr[3] >= VM_MAXUSER_ADDRESS) + return (EINVAL); } pcb->pcb_dr0 = dbregs->dr[0];