DragonFlyBSD Kernel Audit
DF-0862 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/vfs/hpfs/hpfs_vfsops.c b/sys/vfs/hpfs/hpfs_vfsops.c
--- a/sys/vfs/hpfs/hpfs_vfsops.c
+++ b/sys/vfs/hpfs/hpfs_vfsops.c
@@ -38,6 +38,7 @@
 #include <sys/buf.h>
 #include <sys/fcntl.h>
 #include <sys/malloc.h>
+#include <sys/diskslice.h>	/* DIOCGMEDIASIZE / DIOCGSECTORSIZE */
 
 #include <machine/inttypes.h>
 
@@ -290,6 +291,39 @@
 		goto failed;
 	}
 
+	/*
+	 * su_btotal is an unvalidated u32 read straight off disk and is used
+	 * below both to size in-memory allocations and as a loop bound in
+	 * hpfs_bminit().  A crafted image can claim arbitrarily many blocks
+	 * (e.g. su_btotal in [0xFFFFC001,0xFFFFFFFF] wraps the band-count
+	 * arithmetic in hpfs_bminit to 0 -> kmalloc(0) -> ZERO_LENGTH_PTR
+	 * deref -> page fault on mount).  Reject images whose su_btotal
+	 * claims more blocks than the backing device actually provides.
+	 * Best-effort: if the device does not answer the geometry ioctls
+	 * (e.g. a non-disk vnode) we fall through and rely on the band-count
+	 * widening in hpfs_bminit() instead.
+	 */
+	{
+		off_t mediasize = 0;
+		u_int secsize = DEV_BSIZE;
+
+		if (VOP_IOCTL(devvp, DIOCGSECTORSIZE, (caddr_t)&secsize,
+			      FREAD, NOCRED, NULL) == 0 &&
+		    VOP_IOCTL(devvp, DIOCGMEDIASIZE, (caddr_t)&mediasize,
+			      FREAD, NOCRED, NULL) == 0) {
+			if (secsize == 0)
+				secsize = DEV_BSIZE;
+			if ((off_t)sup->su_btotal * (off_t)secsize >
+			    mediasize) {
+				kprintf("hpfs_mountfs: su_btotal %u exceeds "
+					"device capacity %lld\n",
+					sup->su_btotal, (long long)mediasize);
+				error = EINVAL;
+				goto failed;
+			}
+		}
+	}
+
 	mp->mnt_data = (qaddr_t)hpmp;
 	hpmp->hpm_devvp = devvp;
 	hpmp->hpm_dev = dev;
diff --git a/sys/vfs/hpfs/hpfs_subr.c b/sys/vfs/hpfs/hpfs_subr.c
--- a/sys/vfs/hpfs/hpfs_subr.c
+++ b/sys/vfs/hpfs/hpfs_subr.c
@@ -106,7 +106,21 @@
 
 	dprintf(("hpfs_bminit: "));
 
-	hpmp->hpm_dbnum = (hpmp->hpm_su.su_btotal + 0x3FFF) / 0x4000;
+	/*
+	 * su_btotal is an unvalidated u32 read from disk
+	 * (hpfs_vfsops.c).  The original (su_btotal + 0x3FFF) / 0x4000 was
+	 * evaluated in u32 arithmetic and wrapped to 0 for su_btotal in
+	 * [0xFFFFC001, 0xFFFFFFFF], which made the kmalloc()s below return
+	 * ZERO_LENGTH_PTR while the bitmap scan loop a few lines down still
+	 * iterated su_btotal>>5 (up to ~134M) times and dereferenced it -- a
+	 * non-canonical-address page fault on mount.  hpm_dbnum is u_long, so
+	 * compute the band count in 64-bit; also reject a zero total.
+	 */
+	if (hpmp->hpm_su.su_btotal == 0) {
+		dprintf(("hpfs_bminit: su_btotal == 0\n"));
+		return (EINVAL);
+	}
+	hpmp->hpm_dbnum = ((u_long)hpmp->hpm_su.su_btotal + 0x3FFF) / 0x4000;
 
 	dprintf(("0x%lx data bands, ", hpmp->hpm_dbnum));