linux: Bound native getdents and getdents64 batches

[hayzam_gmail.com]

Summary

linux_getdents() and linux_getdents64() round their native read size up
to DEV_BSIZE before converting entries. For an unaligned caller buffer,
the native batch can contain more entries than fit in the Linux buffer.
Conversion then returns EINVAL even after copying complete entries,
and the native read has already advanced the directory position.

For counts of at least DEV_BSIZE, use min(args->count, MAXBSIZE) instead.
A 768-byte request, such as rustix's initial directory read, now uses a
768-byte native buffer rather than 1024. The converted Linux dirent and
dirent64 records are no larger than their native counterparts, so the
whole translated batch fits. Returning a short batch is allowed, and
consuming the whole native batch avoids needing to rewind the directory
position.

Keep the existing sizing for counts below DEV_BSIZE. This fixes the
oversized-batch case in both getdents interfaces, including legacy
getdents for 32-bit and 64-bit callers. The remaining small-buffer,
copyout-failure and old single-entry readdir problems are outside the
scope of this change.

Fixes: 110f2567cb51 ("linux_file.c: Fix handling of NFS getdents() emulation")
Sponsored by: Alchemilla Ventures Private Limited

Test Plan

On UFS/ZFS, the earlier patch still left legacy getdents failures with unaligned buffers.
The updated patch passed both getdents and getdents64 checks for 32-bit and 64-bit callers,
including CAP_READ-only reads. Shared-reader and legacy buffer-sweep checks also passed.

[kevans]

sys/compat/linux/linux_file.c

572–579

Trying to wrap my head around this change, but what's wrong with the current assumption if we instead only consider this path an error if retval == 0? We might over-allocate for buf, but we'd presumably fit more into one call.

[hayzam_gmail.com]

sys/compat/linux/linux_file.c

572–579

I believe it's the file position that gets in the way. kern_getdirentries() has already advanced past the whole native batch before this loop runs, so returning a short result here would skip the entries left in buf.

I tried that on UFS and ZFS: with a 768-byte buffer, enumeration completes without an error, but 112 of the 1002 entries are missing. This change returns all 1002. We'd need to preserve the remainder or arrange to commit the position after the last returned entry to handle partial batches correctly. Rounding down avoids that for counts >= DEV_BSIZE, though you're right that it can return fewer entries per call.

[kevans]

sys/compat/linux/linux_file.c

572–579

Ah, ok, so the cause here being that linux_getdirentries will loop on kern_getdirentries calls. VOP_READDIR implementations at places like [0] would normally avoid an EINVAL because we wrote something out, but this is a subsequent call to kern_getdirentries() -- we wrote something out in a previous one, but not in this one. linux_getdirentries can't reallly distinguish between this and other EINVAL cases, so we don't have much option beyond undersizing without plumbing something else through kern_getdirentries().

Does that sound like an accurate assessment?

[0] https://cgit.freebsd.org/src/tree/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c#n1838

[hayzam_gmail.com]

sys/compat/linux/linux_file.c

572–579

I added some tracing to understand this better. For the failing 768-byte request on UFS and ZFS, I see one successful kern_getdirentries() call producing 1000 native bytes, with no retry in linux_getdirentries(). EINVAL then comes from the conversion check: 720 Linux bytes have been copied, 48 remain in the caller’s buffer, and the next translated directory entry needs 96 bytes.

It looks like this reproducer is running out of space when converting into the Linux buffer, after the native read has completed. Could that be a different path from the retry case you had in mind?

[hayzam_gmail.com]

Adjusted the buffer sizing and added the matching fix for legacy getdents, thanks Kyle for the insights.

[kevans]

FWIW: we discussed this out of band, and I think this change is a reasonable stopgap. I had proposed a few others, but they have the downside of forcing an fseek to rewind when we overshoot.

I think long-term we want to look at giving linux_getdirentries enough detail (applicable filtering (?), linux buffer size) that it can avoid the situation more intelligently (and we can revert this for a modest performance win).

[adrian]

@kevans ok so is this ready to merge into -head? do we want to get this quickly into the next -15 release? Cause now's the time.

[kevans]

In D60760#1405250, @adrian wrote:

@kevans ok so is this ready to merge into -head? do we want to get this quickly into the next -15 release? Cause now's the time.

Yes, I meant to mention it to @markj while we were here in case he wanted to object and completely spaced it. I'll land it when I get a minute, which may end up being tomorrow.