msdosfs: don't pass NULL proc pointer to detrunc()
authorcheloha <cheloha@openbsd.org>
Tue, 23 Aug 2022 20:37:16 +0000 (20:37 +0000)
committercheloha <cheloha@openbsd.org>
Tue, 23 Aug 2022 20:37:16 +0000 (20:37 +0000)
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
sys/msdosfs/msdosfs_lookup.c
sys/msdosfs/msdosfs_vnops.c

index 466ac47..4fa1e0a 100644 (file)
@@ -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);
index f880e29..d081a61 100644 (file)
@@ -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;
                }
 
index d1cd91c..5be8a20 100644 (file)
@@ -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;
                }