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

The following reply was made to PR kern/60504; it has been noted by GNATS.

From: Taylor R Campbell <[email protected]>
To: Shinichi Doyashiki <[email protected]>,
	"Konrad E. Schroeder" <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: kern/60504: lfs_cleanerd can kernel crash on load in lfs_fcntl; lfs_sp == NULL
Date: Sat, 1 Aug 2026 05:24:00 +0000

 This is a multi-part message in MIME format.
 --=_rhGMeJlKcnVU63TmxERqST609Awrm+9s
 Content-Transfer-Encoding: quoted-printable
 
 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=3D1.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=3D1.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 =3D 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=3D1.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 =3D kmem_alloc(size, KM_SLEEP);
     203 		data =3D memp;
     204 	} else
     205 		data =3D stkbuf;
 ...
     226 	error =3D (*fp->f_ops->fo_fcntl)(fp, cmd, data);
 
 https://nxr.netbsd.org/xref/src/sys/kern/sys_descrip.c?r=3D1.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 =3D (VCALL(vp, VOFFSET(vop_fcntl), &a));
 
 https://nxr.netbsd.org/xref/src/sys/kern/vnode_if.c?r=3D1.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.)
 
 --=_rhGMeJlKcnVU63TmxERqST609Awrm+9s
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr60504-lfsfcntlnpd"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr60504-lfsfcntlnpd.patch"
 
 # HG changeset patch
 # User Taylor R Campbell <[email protected]>
 # 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 =3D=3D 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 =3D 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 !=3D NULL && ap->a_data !=3D NULL) {
 -			lws.direct =3D 0;
 -			lws.offset =3D lfs_btofsb(fs, fs->lfs_sp->bytes_written);
 -			*(struct lfs_write_stats *)ap->a_data =3D lws;
 -		}
 +		memset(&lws, 0, sizeof(lws));
 +		lws.direct =3D 0;
 +		lws.offset =3D lfs_btofsb(fs, fs->lfs_sp->bytes_written);
 +		*(struct lfs_write_stats *)ap->a_data =3D lws;
 +		lfs_segunlock(fs);
  		lfs_preunlock(fs);
  		lfs_writer_leave(fs);
 =20
 
 --=_rhGMeJlKcnVU63TmxERqST609Awrm+9s--