From 19f38e0b6b12b10852fa5f008eeadbfdbda19d98 Mon Sep 17 00:00:00 2001 From: cheloha Date: Tue, 23 Aug 2022 20:37:16 +0000 Subject: [PATCH] msdosfs: don't pass NULL proc pointer to detrunc() detrunc()'s proc pointer argument may be passed to vinvalbuf(9), which under certain conditions will pass the given proc pointer to VOP_FSYNC(9), which always asserts that the given proc pointer is equal to curproc. msdosfs_write(), msdosfs_inactive(), createde(), and deextend() all pass NULL for detrunc()'s proc pointer argument. I have no idea why. If these detrunc() calls ever reach VOP_FSYNC(9) the kernel will panic. So, for example, any user with write access to an msdosfs partition can panic the kernel by writing to the partition until they cause ENOSPC. That particular panic looks like this: panic: kernel diagnostic assertion "p == curproc" failed: file "/usr/src/sys/kern/vfs_vops.c", line 305 Stopped at db_enter+0xa: popq %rbp TID PID UID PRFLAGS PFLAGS CPU COMMAND *500294 8955 0 0x100003 0 1K ksh db_enter() at db_enter+0xa panic(ffffffff81f1b0cf) at panic+0xc4 __assert(ffffffff81fa361c,ffffffff81ee8329,131,ffffffff81f7229b) at assert+0x3b VOP_FSYNC(fffffd8449a78b30,ffffffffffffffff,1,0) at VOP_FSYNC+Oxd6 vinvalbuf(fffffd8449a78b30,3,ffffffffffffffff,0,0,ffffffffffffffff) at vinvalbuf+0xd5 detrunc(ffff80000186f900,1fe,0,ffffffffffffffff,0) at detrunc+0x239 msdosfs_write(ffff800055774b98) at msdosf_write+0x4a4 VOP_WRITE(fffffd8449a78b30,ffff800055774d10,3,fffffd8370e8d5d0) at VOP_WRITE+0x59 vn_write(fffffd83c723b860,ffff800055774d10,0) at vn_write+0xc0 dofilewritev(ffff8000556ecfc0,1,ffff800055774d10,0.ffff800055774dc0) at dofilewritev+0x14d sys_write(ffff8000556ecfc0,ffff800055774dd0,ffff800055774dc0) at sys_write+0x6a syscall(ffff800055774e70) at syscall+0x39b Xsyscall() at Xsyscall+0x128 end of kernel end trace frame: 0x7f7ffffd8bf0, count: 2 This patch tweaks all the detrunc() calls in the aforementioned msdosfs functions to pass curproc instead of a NULL pointer to detrunc(). We don't appear to have curproc stashed anywhere in msdosfs_write() or deextend(), so for those calls we explicitly pass curproc. This might have unforseen consequences I can't anticipate. However, with this patch I can no longer panic the kernel by filling an msdosfs partition, which seems like an improvement. With advice from gnezdo@. ok gnezdo@ --- sys/msdosfs/msdosfs_denode.c | 6 +++--- sys/msdosfs/msdosfs_lookup.c | 5 +++-- sys/msdosfs/msdosfs_vnops.c | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/sys/msdosfs/msdosfs_denode.c b/sys/msdosfs/msdosfs_denode.c index 466ac47a0bc..4fa1e0a2ffb 100644 --- a/sys/msdosfs/msdosfs_denode.c +++ b/sys/msdosfs/msdosfs_denode.c @@ -1,4 +1,4 @@ -/* $OpenBSD: msdosfs_denode.c,v 1.66 2020/02/27 09:10:31 mpi Exp $ */ +/* $OpenBSD: msdosfs_denode.c,v 1.67 2022/08/23 20:37:16 cheloha Exp $ */ /* $NetBSD: msdosfs_denode.c,v 1.23 1997/10/17 11:23:58 ws Exp $ */ /*- @@ -545,7 +545,7 @@ deextend(struct denode *dep, uint32_t length, struct ucred *cred) error = extendfile(dep, count, NULL, NULL, DE_CLEAR); if (error) { /* truncate the added clusters away again */ - (void) detrunc(dep, dep->de_FileSize, 0, cred, NULL); + (void) detrunc(dep, dep->de_FileSize, 0, cred, curproc); return (error); } } @@ -652,7 +652,7 @@ msdosfs_inactive(void *v) MNT_RDONLY); #endif if (dep->de_refcnt <= 0 && (vp->v_mount->mnt_flag & MNT_RDONLY) == 0) { - error = detrunc(dep, (uint32_t)0, 0, NOCRED, NULL); + error = detrunc(dep, (uint32_t)0, 0, NOCRED, ap->a_p); dep->de_Name[0] = SLOT_DELETED; } deupdat(dep, 0); diff --git a/sys/msdosfs/msdosfs_lookup.c b/sys/msdosfs/msdosfs_lookup.c index f880e294918..d081a614951 100644 --- a/sys/msdosfs/msdosfs_lookup.c +++ b/sys/msdosfs/msdosfs_lookup.c @@ -1,4 +1,4 @@ -/* $OpenBSD: msdosfs_lookup.c,v 1.34 2022/08/15 01:47:09 jsg Exp $ */ +/* $OpenBSD: msdosfs_lookup.c,v 1.35 2022/08/23 20:37:16 cheloha Exp $ */ /* $NetBSD: msdosfs_lookup.c,v 1.34 1997/10/18 22:12:27 ws Exp $ */ /*- @@ -621,7 +621,8 @@ createde(struct denode *dep, struct denode *ddep, struct denode **depp, - ddep->de_FileSize; dirclust = de_clcount(pmp, diroffset); if ((error = extendfile(ddep, dirclust, 0, 0, DE_CLEAR)) != 0) { - (void)detrunc(ddep, ddep->de_FileSize, 0, NOCRED, NULL); + (void)detrunc(ddep, ddep->de_FileSize, 0, NOCRED, + cnp->cn_proc); return error; } diff --git a/sys/msdosfs/msdosfs_vnops.c b/sys/msdosfs/msdosfs_vnops.c index d1cd91cd03c..5be8a2051a3 100644 --- a/sys/msdosfs/msdosfs_vnops.c +++ b/sys/msdosfs/msdosfs_vnops.c @@ -1,4 +1,4 @@ -/* $OpenBSD: msdosfs_vnops.c,v 1.138 2022/06/26 05:20:42 visa Exp $ */ +/* $OpenBSD: msdosfs_vnops.c,v 1.139 2022/08/23 20:37:16 cheloha Exp $ */ /* $NetBSD: msdosfs_vnops.c,v 1.63 1997/10/17 11:24:19 ws Exp $ */ /*- @@ -745,11 +745,11 @@ msdosfs_write(void *v) errexit: if (error) { if (ioflag & IO_UNIT) { - detrunc(dep, osize, ioflag & IO_SYNC, NOCRED, NULL); + detrunc(dep, osize, ioflag & IO_SYNC, NOCRED, curproc); uio->uio_offset -= resid - uio->uio_resid; uio->uio_resid = resid; } else { - detrunc(dep, dep->de_FileSize, ioflag & IO_SYNC, NOCRED, NULL); + detrunc(dep, dep->de_FileSize, ioflag & IO_SYNC, NOCRED, curproc); if (uio->uio_resid != resid) error = 0; } -- 2.20.1