Hi,

By specifying an invalid offset when reading /dev/kmem it is easy
to crash the kernel from userland.  sysctl kern.allowkmem=0 prevents
this per default

kernel: protection fault trap, code=0
Stopped at      copyout+0x53:   repe movsq      (%rsi),%es:(%rdi)
ddb> trace
copyout() at copyout+0x53
mmrw(201,ffff80002145a5f8,0) at mmrw+0x199
spec_read(ffff80002145a498) at spec_read+0xa6
VOP_READ(fffffd807c0d8068,ffff80002145a5f8,0,fffffd807f7ba300) at VOP_READ+0x41
vn_read(fffffd80792bc780,ffff80002145a5f8,1) at vn_read+0xa1
dofilereadv(ffff8000ffffd508,4,ffff80002145a5f8,1,ffff80002145a6d0) at dofilere 
adv+0x14c
sys_pread(ffff8000ffffd508,ffff80002145a678,ffff80002145a6d0) at sys_pread+0x5c
syscall(ffff80002145a740) at syscall+0x315
Xsyscall() at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7ffffe4880, count: -9
ddb> show register
rdi                   0x7f7ffffe5100
rsi                0x500bca4bc039b5b

The && when checking the direct map is wrong, it should be ||.  This
bug exists since the check was added.

----------------------------
revision 1.3
date: 2004/07/22 00:48:41;  author: art;  state: Exp;  lines: +4 -3;
Fix access to direct mapped memory through /dev/kmem.
----------------------------

When we check if a range is within the limits, we have to check the
end address including the size.  So I think we should subtract the size
from the end limit.

ok?

bluhm

Index: arch/amd64/amd64/mem.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/mem.c,v
retrieving revision 1.34
diff -u -p -r1.34 mem.c
--- arch/amd64/amd64/mem.c      19 Feb 2018 08:59:52 -0000      1.34
+++ arch/amd64/amd64/mem.c      22 Mar 2021 20:10:16 -0000
@@ -148,13 +148,13 @@ mmrw(dev_t dev, struct uio *uio, int fla
                case 1:
                        v = uio->uio_offset;
                        c = ulmin(iov->iov_len, MAXPHYS);
-                       if (v >= (vaddr_t)&start && v < kern_end) {
-                                if (v < (vaddr_t)&etext &&
+                       if (v >= (vaddr_t)&start && v < kern_end - c) {
+                                if (v < (vaddr_t)&etext - c &&
                                     uio->uio_rw == UIO_WRITE)
                                         return EFAULT;
                         } else if ((!uvm_kernacc((caddr_t)v, c,
                            uio->uio_rw == UIO_READ ? B_READ : B_WRITE)) &&
-                           (v < PMAP_DIRECT_BASE && v > PMAP_DIRECT_END))
+                           (v < PMAP_DIRECT_BASE || v > PMAP_DIRECT_END - c))
                                return (EFAULT);
                        error = uiomove((caddr_t)v, c, uio);
                        continue;

Reply via email to