NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/60504: lfs_cleanerd can kernel crash on load in lfs_fcntl; lfs_sp == NULL



Can you try the attached patch?

I think both null tests are superfluous, because sys_fcntl always
passes a non-ull data argument, and VOP_FCNTL always passes a nonnull
argument pointer:

   1391 #define LFCNRECLAIM	_FCNW_FSPRIV ('L',  4, struct lfs_write_stats)

https://nxr.netbsd.org/xref/src/sys/ufs/lfs/lfs.h?r=1.219#1391

Note that _FCNW_FSPRIV sets F_FSCTL:

    250 #define	F_FSCTL		(int)0x80000000	/* This fcntl goes to the fs */
...
    272 #define	_FCN_FSPRIV(inout, fs, num, len) \
    273 	(F_FSCTL | F_FSPRIV | inout | ((len & F_PARAM_MASK) << 16) |	\
    274 	 (fs) << 8 | (num))
...
    277 #define	_FCNW_FSPRIV(f, c, t)	_FCN_FSPRIV(F_FSOUT,   (f), (c), (int)sizeof(t))

https://nxr.netbsd.org/xref/src/sys/sys/fcntl.h?r=1.59#245

So sys_fcntl passes it through to fcntl_forfs:

    326 int
    327 sys_fcntl(struct lwp *l, const struct sys_fcntl_args *uap, register_t *retval)
    328 {
...
    384 	if ((cmd & F_FSCTL)) {
    385 		error = fcntl_forfs(fd, fp, cmd, SCARG(uap, arg));
    386 		fd_putfile(fd);
    387 		return error;
    388 	}

https://nxr.netbsd.org/xref/src/sys/kern/sys_descrip.c?r=1.54#323

And fcntl_forfs always passes a temporary buffer:

    181 static int
    182 fcntl_forfs(int fd, file_t *fp, int cmd, void *arg)
    183 {
    184 	int		error;
    185 	u_int		size;
    186 	void		*data, *memp;
    187 #define STK_PARAMS	128
    188 	char		stkbuf[STK_PARAMS];
....
    201 	if (size > sizeof(stkbuf)) {
    202 		memp = kmem_alloc(size, KM_SLEEP);
    203 		data = memp;
    204 	} else
    205 		data = stkbuf;
...
    226 	error = (*fp->f_ops->fo_fcntl)(fp, cmd, data);

https://nxr.netbsd.org/xref/src/sys/kern/sys_descrip.c?r=1.54#178

    951 int
    952 VOP_FCNTL(struct vnode *vp,
    953     u_int command,
    954     void *data,
    955     int fflag,
    956     kauth_cred_t cred)
    957 {
    958 	int error;
    959 	bool mpsafe;
    960 	struct vop_fcntl_args a;
...
    972 	error = (VCALL(vp, VOFFSET(vop_fcntl), &a));

https://nxr.netbsd.org/xref/src/sys/kern/vnode_if.c?r=1.119#972

While here, I also made sure to zero-initialize the entire output
structure up front so we don't inadvertently leak kernel stack garbage
to userland in case not all fields are initialized.  (If they are all
initialized, no big deal, the compiler can delete the memset.)
# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1785561141 0
#      Sat Aug 01 05:12:21 2026 +0000
# Branch trunk
# Node ID 1f2d855671ed4d4eee9ff4420a357756f04e271a
# Parent  5531308329cc7c89866cd27c64e657e58d8b92fc
# EXP-Topic riastradh-pr60504-lfsfcntlnpd
WIP: lfs: fcntl(LFCNRECLAIM): Fix null pointer deref.

XXX explain further

PR kern/60504: lfs_cleanerd can kernel crash on load in lfs_fcntl;
lfs_sp == NULL

diff -r 5531308329cc -r 1f2d855671ed sys/ufs/lfs/lfs_vnops.c
--- a/sys/ufs/lfs/lfs_vnops.c	Wed Jul 29 20:17:12 2026 +0000
+++ b/sys/ufs/lfs/lfs_vnops.c	Sat Aug 01 05:12:21 2026 +0000
@@ -2030,13 +2030,14 @@ lfs_fcntl(void *v)
 		LFS_CLEANERINFO(cip, fs, bp);
 		oclean = lfs_ci_getclean(fs, cip);
 		LFS_SYNC_CLEANERINFO(cip, fs, bp, 1);
+		lfs_seglock(fs, 0);
 		lfs_segwrite(ap->a_vp->v_mount, SEGM_CKP | SEGM_FORCE_CKP);
 		/* Copy out write stats */
-		if (ap != NULL && ap->a_data != NULL) {
-			lws.direct = 0;
-			lws.offset = lfs_btofsb(fs, fs->lfs_sp->bytes_written);
-			*(struct lfs_write_stats *)ap->a_data = lws;
-		}
+		memset(&lws, 0, sizeof(lws));
+		lws.direct = 0;
+		lws.offset = lfs_btofsb(fs, fs->lfs_sp->bytes_written);
+		*(struct lfs_write_stats *)ap->a_data = lws;
+		lfs_segunlock(fs);
 		lfs_preunlock(fs);
 		lfs_writer_leave(fs);
 


Home | Main Index | Thread Index | Old Index