Conversation
handle_op_read_file is stateless: filp_open, read at an offset, filp_close,
every call. That works for a regular file, whose bytes live at stable
offsets, and cannot work for a synthetic filesystem. procfs, sysfs and
debugfs generate their contents at open time and hand them out through
sequential reads of that one open file; the files report st_size 0, and a
seq_file's byte offset is not a stable cursor into a fixed object. So a
chunked read re-generates the content per chunk, and a caller that gets
nothing has no way to learn why.
That is not theoretical: reading /proc through the existing op fails on all
15 penguin CI arches, and the symptom was a process-tree cross-check
reporting "no processes" and a socket graph reporting "no sockets" on a
guest that reads /proc perfectly well itself.
Three ops, appended at the END of PORTAL_OP_LIST so no existing op number
shifts:
vfs_open(path) -> {error, handle, fs_magic}
vfs_read(handle, len) -> {error, nbytes, eof} + payload
vfs_close(handle) -> {error}
The file stays open across reads and the kernel's own f_pos advances, so a
seq_file is consumed exactly as a guest process cat-ing it would.
Errors are returned as DATA, never as absence: each handler replies
READ_OK with a result struct carrying a negative errno. Signalling failure
by returning no payload is what made an unreadable file indistinguishable
from an empty one; an errno also makes the failure diagnosable (-ENOENT
path missing, -EACCES permissions, -EINVAL a file the kernel will not read
this way). fs_magic reports sb->s_magic so the host can name the
filesystem instead of guessing. EOF (n == 0) is reported as eof=1 with
error=0 -- a successful read of nothing -- where read_file conflates it
with failure and returns READ_FILE_FAIL.
Handle hygiene: the handle is (generation << 8) | (slot + 1), so 0 is never
valid and a read after close fails loudly instead of hitting whatever file
has since taken the slot. The 16-slot table reclaims its oldest entry when
full, with a warning, so a host that leaks handles degrades into one stale
handle rather than a permanent inability to read any file.
Built for armel/6.13 and armel/4.10 (the latter exercises the pre-4.14
kernel_read signature); handlers confirmed linked into igloo.ko.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
handle_op_read_fileis stateless:filp_open, read at an offset,filp_close— every call. Fine for a regular file, whose bytes live at stable offsets. It cannot work for a synthetic filesystem:st_size0So a chunked read re-generates content per chunk, and a caller that gets nothing cannot learn why.
Not theoretical: reading
/procthrough the existing op fails on all 15 penguin CI arches. The symptom was never an error — a process-tree cross-check reportedmatched=0("no processes") and a socket graph reportedsockets=0("no sockets"), on a guest that reads/procperfectly well itself (proc_self.yamlreadlinks/proc/self/exe).The ops
Appended at the end of
PORTAL_OP_LIST, so no existing op number shifts:vfs_open{error, handle, fs_magic}vfs_read{error, nbytes, eof}+ payloadvfs_close{error}The file stays open across reads and the kernel's own
f_posadvances, so a seq_file is consumed exactly as a guest processcat-ing it would.Errors are data, never absence
Each handler replies
READ_OKwith a result struct carrying a negative errno. Signalling failure by returning no payload is precisely what made an unreadable file indistinguishable from an empty one. An errno also makes the failure diagnosable —-ENOENT(path missing) vs-EACCES(permissions) vs-EINVAL(a file the kernel won't read this way) — which is currently impossible, because every failure path inhandle_op_read_filelogs only viaigloo_pr_debug.fs_magicreportssb->s_magic, so the host can name the filesystem instead of guessing.EOF is not an error:
n == 0returnseof=1, error=0.handle_op_read_fileconflates the two today and reports a legitimate empty read asREAD_FILE_FAIL.Handle hygiene
Handle is
(generation << 8) | (slot + 1):0is never a valid handle, so a zeroed/unset value can't address slot 0Build
Built for armel/6.13 and armel/4.10 — the latter deliberately, to exercise the pre-4.14
kernel_readsignature behind the version guard. Handlers confirmed linked intoigloo.ko:What lands next
The penguin side needs a release to build against. Once this merges and cuts one,
fs.read_filegrows a synthetic-fs path that uses these ops (open once → read until eof → close), andprocesses.peers()'s/proc/net/*join starts resolving on a live guest. penguin#938 already makes the current failure loud and correctly typed, so the transition is observable rather than silent.Worth noting for review: this PR does not delete or change
handle_op_read_file. If the errno turns out to say the old op's failure was something trivially fixable, that fix is still worth having for regular files, and these ops remain the right answer for synthetic ones.