Return EINVAL from getdents when the buffer is too small - #2774
Conversation
saagarjha
left a comment
There was a problem hiding this comment.
This looks semantically correct but I am not entirely sure how you ran into this behavior; I cannot imagine that there are many programs that pass in a buffer this small. Some tips for the comment style though
| // Linux returns EINVAL when the buffer cannot hold even the first | ||
| // entry. Returning 0 here instead would read as end-of-directory, | ||
| // so a readdir() with a small buffer sees an empty directory. |
There was a problem hiding this comment.
The default behavior for iSH is to match what Linux does, so there is no need to call attention to it. In fact it is generally a good idea to point out places where behavior (intentionally) diverges.
| // Linux returns EINVAL when the buffer cannot hold even the first | |
| // entry. Returning 0 here instead would read as end-of-directory, | |
| // so a readdir() with a small buffer sees an empty directory. | |
| // If the buffer isn't large enough to read even a single dirent, | |
| // reset the fd and return _EINVAL. |
If the caller's buffer cannot hold even the first entry, the loop breaks and sys_getdents_common returns 0. Callers read 0 as end of directory, so a readdir() with a small buffer reports the directory as empty instead of failing. Linux returns EINVAL. Rewind to the entry that did not fit and return that instead; only the first iteration can be affected, since once something has been written a short count is correct.
f7a1967 to
67d6cd7
Compare
|
Comment reworded as suggested, and noted for future patches — flag divergence, not conformance. Pushed. On how I ran into it: honestly, not from a program failing. I was running a differential conformance suite against real Linux — same static i386 binary on both, compare the results — and this fell out of the The one thing that made it seem worth sending is the failure mode: returning 0 is indistinguishable from end-of-directory, so a caller with an undersized buffer sees an empty directory rather than an error. That is the sort of thing that would be very hard to diagnose from the guest side if it ever did bite. But if the bar is an observed problem, that is a fair call and I am happy for you to close it. |
If the caller's buffer cannot hold even the first entry, the loop just breaks and
sys_getdents_commonreturns 0. Every caller reads 0 as end of directory, so areaddir()with a small buffer quietly reports the directory as empty rather than failing.Linux returns
EINVALfor this case. This rewinds to the entry that did not fit and returns that instead. Only the first iteration can be affected — once something has been written, returning a short count is correct, so the existing behavior for partial fills is unchanged.Testing
Repro:
getdents64on/with an 8-byte buffer.-1 (Invalid argument)0— reads as an empty directory-1 (Invalid argument)Also ran a shell smoke test over an Alpine rootfs (
ls,find, directory walks with normal-sized buffers,/procreads, fork/exec churn, redirects, pipes) with no change in behavior.🤖 Generated with Claude Code