Download raw body.
ffs_readdir
On Wed, 20 May 2026 15:22:03 +0200,
Claudio Jeker <cjeker@diehard.n-r-g.com> wrote:
>
> > - if (memchr(u.dn.d_name, '/', u.dn.d_namlen) != NULL) {
> > - error = EINVAL;
> > + if ((error = UFS_BUFATOFF(ip, off, &dirbuf, &bp)) != 0)
> > + break;
> > + size = bp->b_bcount - bp->b_resid -
> > + (dirbuf - (char *)bp->b_data);
>
> Can you explain why you calculate the size like this?
size is calculated that way because UFS_BUFATOFF() returns a buffer and
dirbuf; dirbuf, actually, may point into the middle of bp->b_data, and
readdir() needs the number of valid bytes at dirbuf, not in the whole
buffer.
Here bp->b_bcount - bp->b_resid is valid bytes count in the buffer, and
dirbuf - bp->b_data is the offset inside the buffer.
> Is there no other way?
>
Not that I found as a generic way.
ffs_readdir() may use bsize and blkoffset, but not a generic one.
> > + if (error == 0 &&
> > + (char *)dp + offsetof(struct direct, d_name) < edp &&
> > + dp->d_reclen <= offsetof(struct direct, d_name))
> > + error = EIO;
>
> Why did you add the error == 0 case here?
> In the old code it would do this check in all cases and it should not
> matter. Now it can clobber the EINVAL return error.
>
Frankly? Because it was inherited from early ffs_readdir() version, where I
had tried to preserve uiomove() error.
Thanks for catch that up.
Here updated diff:
Index: sys/ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /home/cvs/src/sys/ufs/ufs/ufs_vnops.c,v
diff -u -p -r1.164 ufs_vnops.c
--- sys/ufs/ufs/ufs_vnops.c 18 Oct 2024 05:52:33 -0000 1.164
+++ sys/ufs/ufs/ufs_vnops.c 20 May 2026 18:47:35 -0000
@@ -1323,18 +1323,22 @@ int
ufs_readdir(void *v)
{
struct vop_readdir_args *ap = v;
- struct uio auio, *uio = ap->a_uio;
- struct iovec aiov;
+ struct uio *uio = ap->a_uio;
+ struct vnode *vp = ap->a_vp;
+ struct inode *ip = VTOI(vp);
union {
struct dirent dn;
char __pad[roundup(sizeof(struct dirent), 8)];
} u;
off_t off = uio->uio_offset;
+ off_t bytesinfile;
struct direct *dp;
+ struct buf *bp;
char *edp;
- caddr_t diskbuf;
+ char *dirbuf;
size_t count, entries;
- int bufsize, readcnt, error;
+ int done, error, readcnt, xfersize;
+ long size;
if (uio->uio_rw != UIO_READ)
return (EINVAL);
@@ -1346,75 +1350,98 @@ ufs_readdir(void *v)
if (count <= entries)
return (EINVAL);
- /*
- * Convert and copy back the on-disk struct direct format to
- * the user-space struct dirent format, one entry at a time
- */
-
/* read from disk, stopping on a block boundary, max 64kB */
readcnt = min(count, 64*1024) - entries;
- auio = *uio;
- auio.uio_iov = &aiov;
- auio.uio_iovcnt = 1;
- auio.uio_resid = readcnt;
- auio.uio_segflg = UIO_SYSSPACE;
- aiov.iov_len = readcnt;
- bufsize = readcnt;
- diskbuf = malloc(bufsize, M_TEMP, M_WAITOK);
- aiov.iov_base = diskbuf;
- error = VOP_READ(ap->a_vp, &auio, 0, ap->a_cred);
- readcnt -= auio.uio_resid;
- dp = (struct direct *)diskbuf;
- edp = &diskbuf[readcnt];
+ off = uio->uio_offset;
+ done = 0;
+ error = 0;
+ bp = NULL;
memset(&u, 0, sizeof(u));
- /*
- * While
- * - we haven't failed to VOP_READ or uiomove()
- * - there's space in the read buf for the head of an entry
- * - that entry has a valid d_reclen, and
- * - there's space for the *entire* entry
- * then we're good to process this one.
- */
- while (error == 0 &&
- (char *)dp + offsetof(struct direct, d_name) < edp &&
- dp->d_reclen > offsetof(struct direct, d_name) &&
- (char *)dp + dp->d_reclen <= edp) {
- u.dn.d_reclen = roundup(dp->d_namlen+1 +
- offsetof(struct dirent, d_name), 8);
- if (u.dn.d_reclen > uio->uio_resid)
+ while (error == 0 && done == 0 && readcnt > 0) {
+ bp = NULL;
+ if ((bytesinfile = DIP(ip, size) - off) <= 0)
break;
- off += dp->d_reclen;
- u.dn.d_off = off;
- u.dn.d_fileno = dp->d_ino;
- u.dn.d_type = dp->d_type;
- u.dn.d_namlen = dp->d_namlen;
- memcpy(u.dn.d_name, dp->d_name, u.dn.d_namlen);
- memset(u.dn.d_name + u.dn.d_namlen, 0, u.dn.d_reclen
- - u.dn.d_namlen - offsetof(struct dirent, d_name));
-
- if (memchr(u.dn.d_name, '/', u.dn.d_namlen) != NULL) {
- error = EINVAL;
+ if ((error = UFS_BUFATOFF(ip, off, &dirbuf, &bp)) != 0)
break;
+ size = bp->b_bcount - bp->b_resid -
+ (dirbuf - (char *)bp->b_data);
+ if (size <= 0)
+ break;
+
+ xfersize = readcnt;
+ if (size < xfersize)
+ xfersize = size;
+ if (bytesinfile < xfersize)
+ xfersize = bytesinfile;
+
+ /*
+ * Convert and copy back the on-disk struct direct format to
+ * the user-space struct dirent format, one entry at a time
+ */
+ dp = (struct direct *)dirbuf;
+ edp = dirbuf + xfersize;
+
+ /*
+ * While
+ * - we haven't failed to read or uiomove()
+ * - there's space in the read buf for the head of an entry
+ * - that entry has a valid d_reclen, and
+ * - there's space for the *entire* entry
+ * then we're good to process this one.
+ */
+ while (error == 0 &&
+ (char *)dp + offsetof(struct direct, d_name) < edp &&
+ dp->d_reclen > offsetof(struct direct, d_name) &&
+ (char *)dp + dp->d_reclen <= edp) {
+ u.dn.d_reclen = roundup(dp->d_namlen + 1 +
+ offsetof(struct dirent, d_name), 8);
+ if (u.dn.d_reclen > uio->uio_resid) {
+ done = 1;
+ break;
+ }
+ off += dp->d_reclen;
+ u.dn.d_off = off;
+ u.dn.d_fileno = dp->d_ino;
+ u.dn.d_type = dp->d_type;
+ u.dn.d_namlen = dp->d_namlen;
+ memcpy(u.dn.d_name, dp->d_name, u.dn.d_namlen);
+ memset(u.dn.d_name + u.dn.d_namlen, 0,
+ u.dn.d_reclen - u.dn.d_namlen -
+ offsetof(struct dirent, d_name));
+
+ if (memchr(u.dn.d_name, '/', u.dn.d_namlen) != NULL) {
+ error = EINVAL;
+ break;
+ }
+
+ error = uiomove(&u.dn, u.dn.d_reclen, uio);
+ dp = (struct direct *)((char *)dp + dp->d_reclen);
}
- error = uiomove(&u.dn, u.dn.d_reclen, uio);
- dp = (struct direct *)((char *)dp + dp->d_reclen);
+ /*
+ * If there was room for an entry in what we read but its
+ * d_reclen is bogus, fail
+ */
+ if ((char *)dp + offsetof(struct direct, d_name) < edp &&
+ dp->d_reclen <= offsetof(struct direct, d_name))
+ error = EIO;
+
+ brelse(bp);
+ bp = NULL;
+ readcnt -= xfersize;
}
-
- /*
- * If there was room for an entry in what we read but its
- * d_reclen is bogus, fail
- */
- if ((char *)dp + offsetof(struct direct, d_name) < edp &&
- dp->d_reclen <= offsetof(struct direct, d_name))
- error = EIO;
- free(diskbuf, M_TEMP, bufsize);
+ if (bp != NULL)
+ brelse(bp);
uio->uio_offset = off;
- *ap->a_eofflag = DIP(VTOI(ap->a_vp), size) <= off;
+ *ap->a_eofflag = DIP(ip, size) <= off;
+
+ if (!(vp->v_mount->mnt_flag & MNT_NOATIME) ||
+ (ip->i_flag & (IN_CHANGE | IN_UPDATE)))
+ ip->i_flag |= IN_ACCESS;
return (error);
}
--
wbr, Kirill
ffs_readdir