The logic in mmrw() to check whether an address is within direct
authorbluhm <bluhm@openbsd.org>
Wed, 24 Mar 2021 14:26:39 +0000 (14:26 +0000)
committerbluhm <bluhm@openbsd.org>
Wed, 24 Mar 2021 14:26:39 +0000 (14:26 +0000)
map was the wrong way around.  The && prevented an EFAULT error and
could pass userland addresses as kernel source to copyout(9).  The
kernel could crash with protection fault due to an invalid offset
when reading /dev/kmem.
Also make the range checks stricter.  Not only the start address
must be valid, but also the end address must be within the region
to be copied.
Note that sysctl kern.allowkmem=0 makes the bug unreachable by
default.
OK deraadt@

sys/arch/amd64/amd64/mem.c

index da186df..c6c9ffd 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mem.c,v 1.34 2018/02/19 08:59:52 mpi Exp $ */
+/*     $OpenBSD: mem.c,v 1.35 2021/03/24 14:26:39 bluhm Exp $ */
 /*
  * Copyright (c) 1988 University of Utah.
  * Copyright (c) 1982, 1986, 1990, 1993
@@ -148,13 +148,13 @@ mmrw(dev_t dev, struct uio *uio, int flags)
                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;