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 #include #include +#include /* DIOCGMEDIASIZE / DIOCGSECTORSIZE */ #include @@ -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));