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