Skip to content

Return EINVAL from getdents when the buffer is too small - #2774

Open
emkey1 wants to merge 1 commit into
ish-app:masterfrom
emkey1:fix_getdents_small_buffer
Open

Return EINVAL from getdents when the buffer is too small#2774
emkey1 wants to merge 1 commit into
ish-app:masterfrom
emkey1:fix_getdents_small_buffer

Conversation

@emkey1

@emkey1 emkey1 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

If the caller's buffer cannot hold even the first entry, the loop just breaks and sys_getdents_common returns 0. Every caller reads 0 as end of directory, so a readdir() with a small buffer quietly reports the directory as empty rather than failing.

Linux returns EINVAL for 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: getdents64 on / with an 8-byte buffer.

result
Linux (x86 Ubuntu) -1 (Invalid argument)
master 0 — reads as an empty directory
this branch -1 (Invalid argument)

Also ran a shell smoke test over an Alpine rootfs (ls, find, directory walks with normal-sized buffers, /proc reads, fork/exec churn, redirects, pipes) with no change in behavior.

🤖 Generated with Claude Code

@saagarjha saagarjha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread fs/dir.c Outdated
Comment on lines +96 to +98
// 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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.
@emkey1
emkey1 force-pushed the fix_getdents_small_buffer branch from f7a1967 to 67d6cd7 Compare July 29, 2026 18:15
@emkey1

emkey1 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

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 getdents cases. So it is a conformance fix rather than a bug report, and I do not have a real-world consumer to point at. I share your instinct that few programs pass a buffer that small.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants