fix(uffd): register guest memory with WRITE_PROTECT in addition to MISSING#12
fix(uffd): register guest memory with WRITE_PROTECT in addition to MISSING#12ValentaTomas wants to merge 1 commit intomainfrom
Conversation
…SSING `guest_memory_from_uffd` registered each guest memory region with `Uffd::register(...)`, which is the convenience wrapper that uses `UFFDIO_REGISTER_MODE_MISSING` only. As a result, any UFFD handler that asks the kernel to keep a copied page write-protected (by passing `UFFDIO_COPY_MODE_WP`) gets a synchronous EINVAL on the very first read fault, because the destination range was never registered with `UFFDIO_REGISTER_MODE_WP`. WRITE_PROTECT registration is what enables the standard CoW snapshot pattern: the handler serves a missing page via UFFDIO_COPY with MODE_WP, the kernel re-faults on the next write to that page, and the handler observes (and can record) which pages got dirtied after restore. Without WP registration this pattern silently breaks the resume path. Switch `register(...)` to `register_with_mode(... MISSING | WRITE_PROTECT)`. `RegisterMode::WRITE_PROTECT` lives behind the `linux5_7` feature of the `userfaultfd` crate (UFFDIO_WRITEPROTECT was added in Linux 5.7), so also enable that feature on the dependency in `src/vmm/Cargo.toml`. Firecracker's minimum supported kernel is well past 5.7. This is a strict superset of the previous registration: existing handlers that don't pass MODE_WP behave identically. Signed-off-by: ValentaTomas <valenta.and.thomas@gmail.com>
PR SummaryMedium Risk Overview Enables the Reviewed by Cursor Bugbot for commit 32330a4. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Closing — this is a duplicate. Babis already implemented the WRITE_PROTECT registration (and went further with I missed that branch when investigating because I only searched |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 32330a4. Configure here.
| mem_region.size() as _, | ||
| RegisterMode::MISSING | RegisterMode::WRITE_PROTECT, | ||
| ) | ||
| .map_err(GuestMemoryFromUffdError::Register)?; |
There was a problem hiding this comment.
UFFD WRITE_PROTECT registration breaks aarch64 on kernels below 6.10
High Severity
Unconditionally registering with RegisterMode::WRITE_PROTECT will cause UFFDIO_REGISTER to fail with EINVAL on aarch64 with kernels before 6.10, because arm64 userfaultfd write-protect page table support (pgtable_uffd_wp_supported()) was only added in Linux 6.10. Firecracker supports aarch64 on kernels 5.10 and 6.1, both of which lack this support. This is a regression that breaks all UFFD-based snapshot restores on aarch64, not just WP-tracking ones, since the registration itself fails.
Reviewed by Cursor Bugbot for commit 32330a4. Configure here.
| .create() | ||
| .map_err(GuestMemoryFromUffdError::Create)?; | ||
|
|
||
| // Register every region for both MISSING and WRITE_PROTECT faults. |
There was a problem hiding this comment.
Missing WP feature negotiation during UFFDIO_API handshake
Medium Severity
The code registers regions with RegisterMode::WRITE_PROTECT but never negotiates FeatureFlags::PAGEFAULT_FLAG_WP during the UFFDIO_API handshake. The Linux man page states the user needs to check availability of UFFD_FEATURE_PAGEFAULT_FLAG_WP via UFFDIO_API before using write-protect mode. Adding this to require_features would also provide an early, clear failure on platforms that lack WP support instead of a less informative error at registration time.
Reviewed by Cursor Bugbot for commit 32330a4. Configure here.


Changes
In
guest_memory_from_uffd(src/vmm/src/persist.rs), switch the per-region UFFD registration from theUffd::register(...)convenience wrapper (which usesUFFDIO_REGISTER_MODE_MISSINGonly) toUffd::register_with_mode(... MISSING | WRITE_PROTECT).To use
RegisterMode::WRITE_PROTECT, also enable thelinux5_7feature on theuserfaultfdcrate dependency insrc/vmm/Cargo.toml. The flag is named after the kernel that introducedUFFDIO_WRITEPROTECT(Linux 5.7), which is well below Firecracker's supported kernel range.Reason
Any UFFD handler that asks the kernel to keep a copied page write-protected by passing
UFFDIO_COPY_MODE_WPgot a synchronousEINVALon the very first read fault, because the destination range was never registered withUFFDIO_REGISTER_MODE_WP. WRITE_PROTECT registration is what enables the standard CoW snapshot pattern:UFFDIO_COPYwithMODE_WPso the page lands write-protected.Without WP registration, step 1 fails immediately with
EINVAL, breaking the resume path for any handler that opts into WP tracking. We hit this in practice with the e2b-dev/infra orchestrator's UFFD handler.This change is a strict superset of the previous registration. Handlers that never pass
MODE_WPbehave exactly as before —WRITE_PROTECTregistration on its own is a no-op until the handler explicitly write-protects pages.Repro
Trivial against the unpatched binary:
Reproduced end-to-end from the orchestrator's resume path: every read fault returned
failed to handle uffd: failed to handle uffd: invalid argument(the propagatedEINVAL) until WRITE_PROTECT was registered on the FC side.License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.
PR Checklist
CONTRIBUTING.md.cargo check -p vmm --target x86_64-unknown-linux-gnupasses.cargo build -p vmm --target x86_64-unknown-linux-gnupasses.tools/devtool checkbuild --all— not run locally (this is a draft).tools/devtool checkstyle— not run locally (this is a draft).TODO.rust-vmm(it's purely a Firecracker registration change).