soo: stream capsule snapshots, and release what a capsule leaves behind - #326
Merged
Conversation
write_snapshot() dereferenced the user-space buffer directly to pick up the snapshot size, then fed that value to dma_alloc_coherent() and BUG_ON()'d the result. A caller passing an uninitialized or corrupted buffer therefore killed the agency: the size read back was garbage (1.7 GB in the observed case), the allocation warned in the page allocator and returned NULL, and the kernel died in an ioctl with the interrupts off. Read the size with copy_from_user(), reject a size that no snapshot could ever have, and return -EINVAL/-ENOMEM/-EFAULT instead of taking the machine down. The error is propagated through the WRITE_SNAPSHOT ioctl so that user space can report it. Observed with the EMISO engine restoring a snapshot whose archive held no capsule payload.
Saving or restoring a capsule made the agency allocate a DMA buffer as large as the whole snapshot, i.e. the capsule slot plus its header. On virt64 that is 128 MB of physically contiguous memory taken from a 160 MB CMA zone, and the EMISO engine holds a copy of the same snapshot in user space at that very moment. cma_alloc() then has to migrate the movable pages sitting in the way, fails with -EBUSY, and the fallback to the buddy allocator asks for an order-16 block which cannot exist. The staged protocol already moves the capsule memory 4 MB at a time, so only one chunk is ever in flight: pass a bounce buffer of that size instead of the whole snapshot. AVZ now reads and writes each chunk at the beginning of the buffer, bounded by the size the agency advertises, and the agency copies it to or from user space as the stages progress. The buffer is reserved once, at init time, while the CMA zone is still pristine. The snapshot is no longer held twice in RAM, and neither path depends on a large contiguous allocation any more.
The injection path had the same weaknesses as the snapshot one: the size came straight from user space with no bound, dma_alloc_coherent() was followed by a BUG_ON(), and the image itself was read with a plain memcpy() from a user pointer. Reject an implausible size, report -ENOMEM instead of dying when the CMA zone cannot serve the request, copy the image with copy_from_user(), and propagate the failure through the INJECT_CAPSULE ioctl. The staged loop also gives up if AVZ stops advancing the cursor, as on the snapshot paths. The ITB is parsed as a whole by AVZ, so unlike a snapshot it still needs one contiguous allocation -- a few MB, taken once per injection.
The console focus is a plain index over the slots, so it keeps pointing at a capsule which is being snapshotted or has been shut down, and every keystroke is then handed to s3c_cons_sendc() for that slot. That path is not prepared for it. It runs from the serial interrupt and goes through vdevback_processing_begin(), which waits for the frontend to be connected: a capsule suspended for a snapshot, resuming, or gone has none, so the keystroke slept in the interrupt and hung the console for good. get_console() returning NULL for a vanished domain was dereferenced right away as well. Skip, when cycling the focus, every slot whose capsule is not living -- any other state is a capsule in transit or gone. Bail out of the send path when no console device answers for the domain, and take the new non-blocking vdevback_processing_try_begin(), which reports the frontend is not usable rather than waiting for it. vdevfront_is_connected() also read the device private data before testing the pointer for NULL; do it the other way round.
Snapshotting a capsule suspends it, reads its memory and resumes it: a capsule which was living stays living, which is the whole point of snapshotting a running one. A caller which shuts the capsule down right after -- pausing it, in the EMISO engine -- gets the opposite of what it wants, though: the capsule is woken up only to be killed, and runs for a moment, diverging from the snapshot just taken. Add an AVZ_STAGE_FINALIZE_HOLD stage, which completes the snapshot and leaves the capsule suspended, and an AGENCY_IOCTL_READ_SNAPSHOT_HOLD reaching it. AGENCY_IOCTL_READ_SNAPSHOT is untouched and still resumes. shutdown_capsule() has to know about that state: it asks the capsule to terminate through the DC_SHUTDOWN handshake, and do_sync_dom() waits for the answer without any deadline. A suspended capsule is not scheduled any more and can never answer, exactly like the stopped and killed ones the handshake is already skipped for, so reap it directly as well. While at it, a snapshot which fails halfway no longer leaves the capsule suspended for good: every error path now goes through the resuming stage, whichever flavour was asked for.
Every snapshot of a living capsule printed its state on the console, which is debug output that outlived its purpose.
Restoring a snapshot re-implanted the capsule memory four bytes too early, so the capsule resumed on shifted contents and drowned in synchronous exceptions -- or jumped into an unmapped vector and got killed, depending on what the shift landed on. The size AVZ stores at the beginning of a snapshot does not count itself: only the value it hands back to the agency includes those four bytes. Deriving the header size from the stored value therefore misses them, and every chunk was read from the wrong offset. Add the size field back where the header is computed, and read one more byte than the stored size says when handing the header over. AVZ also reads the domain context out of the buffer again at the FINALIZE stage, to restore the EL2 frame of the capsule; the CHUNK stages having filled the buffer with capsule memory by then, put the header back before finalizing. Validated end to end on virt64: two consecutive pause/resume cycles through the EMISO engine, no contiguous allocation involved.
sudo_session_start() gated the build on `sudo -v`, which validates the user through the 'validate' pseudo-command. A sudoers file granting NOPASSWD to commands may still refuse that one, and the build then stopped on "failed to acquire sudo credentials" while every privileged task it was about to run needed no authentication at all. Probe with a harmless command instead, and only fall back to `sudo -v` when that fails -- which is where a password prompt genuinely belongs.
A capsule closes its frontends and drops its vbstore entries as part of the DC_SHUTDOWN handshake. One which is suspended, stopped or already dead never gets there, and everything it owned outlives it: the backends stay registered, so the next capsule landing in that slot makes vbus_dev_changed() BUG() on a device which already exists, and the vbstore entries stay too, so a capsule restored there comes up on top of what the previous one left. Do it for such a capsule, in shutdown_capsule(), where the handshake is skipped: unregister the backends bound to that domain, and remove its vbstore subtrees -- backend/<type>/<domID>, device/<domID> and soo/s3c/<domID>, mirroring what vbus_probe_backend() walks and what the capsule removes for itself. The bus walk reports one device at a time, holding it, so that device_unregister() runs outside of it. This already applied to a capsule killed after a fault, whose slot could not be reused afterwards.
complete_domain_destroy() released the shared page, the stack and the domain structure, but never walked d->gnttab. A domain revokes its grants one by one as its frontends close, so the list is empty by then and the leak never showed -- except for a domain destroyed before it gets there: suspended for a snapshot, crashed, or killed outright. Its entries then outlive the domain they describe, and the memory is handed out again to the next one, which inherits list links pointing into it. Add gnttab_destroy(), symmetric with gnttab_init(), and call it where the rest of the domain is released.
daniel-rossier
force-pushed
the
fix/capsule-snapshot-guard
branch
from
September 7, 2026 15:27
eaeb51b to
fc6c9e8
Compare
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
1 participant
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.
Saving or restoring a capsule made the agency allocate a DMA buffer as large as
the whole snapshot -- 128 MB of physically contiguous memory on virt64, taken
from a 160 MB CMA zone while the EMISO engine held a copy of the same snapshot
in user space.
cma_alloc()failed to migrate the movable pages out of the way,the fallback asked the buddy allocator for an order-16 block, and the agency
died on a
BUG_ON()in an ioctl with the interrupts off.The staged protocol already moves the capsule memory 4 MB at a time, so only one
chunk is ever in flight: the snapshot is now streamed through a bounce buffer of
that size, reserved once at init while the CMA zone is still pristine. Neither
pause nor resume depends on a large contiguous allocation any more, and the
snapshot is no longer held twice in RAM.
Pulling that thread surfaced a family of related weaknesses, each fixed here:
BUG_ON()d,on both the snapshot and the injection paths; the capsule image was read with
a plain
memcpy()from a user pointer;and the send path slept in the serial interrupt waiting for a frontend that
was gone -- freezing the console;
DC_SHUTDOWNhandshake (suspended, stopped,or killed after a fault) left its backends registered and its vbstore entries
behind, so its slot could not be reused;
grants, and the recycled memory took the next domain's list links with it.
A snapshot can also be taken without resuming the capsule
(
AVZ_STAGE_FINALIZE_HOLD), for a caller which shuts it down right after --waking it up only to kill it lets it run, and diverge from the snapshot just
taken, for nothing.
Validated end to end on virt64, driven through the EMISO Docker API: six
consecutive pause/resume cycles, no
BUG(), no oops, engine healthythroughout.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Di7T124oudbKPpE28mJnb4