feat(uffd): handle UFFD_EVENT_REMOVE; track per-page state; race-safe COPY by ValentaTomas · Pull Request #2520 · e2b-dev/infra

ValentaTomas · 2026-04-29T00:23:10Z

Handles UFFD_EVENT_REMOVE so balloon-deflate / madvise(MADV_DONTNEED) transitions pages to a removed state instead of leaving stale faulted mappings. Drains the REMOVE batch under settleRequests.Lock; workers hold settleRequests.RLock across the state read → UFFDIO_COPY → SetRange sequence so a concurrent REMOVE can't slip between the read and the install. Soft-fails UFFDIO_COPY EAGAIN / partial copies onto a deferredFaults queue and wakes the poll loop via a self-pipe.

Depends on #2545.

cursor · 2026-04-29T00:23:18Z

PR Summary

High Risk
High risk because it changes core userfaultfd serve-loop concurrency/locking and introduces deferred fault retry, where subtle races can deadlock or corrupt page state.

Overview
The Serve loop now handles UFFD_EVENT_REMOVE, introduces deferred retry for UFFDIO_COPY returning EAGAIN, and changes how prefaulting and zero-filling are installed, but the worker goroutine captures loop variables (pf, addr, offset) from the range loop, so concurrent workers can operate on the wrong fault and requeue the wrong pointer. The new state short-circuiting (treating block.Dirty as “already satisfied”) can incorrectly drop a real fault if pages ever become swappable or state is stale, leaving the faulting thread blocked. Prefault writes block.Dirty state even when it installs via a read/WP path, which can hide subsequent correctness issues around clean vs dirty semantics and interacts with the short-circuit.

^{Reviewed by Cursor Bugbot for commit 611d0d6. Bugbot is set up for automated code reviews on this repo. Configure here.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da912303d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…ernel redeliver Folds audit findings #1 and #7 into one commit since they share the same error arm in faultPage. The kernel surfaces concurrent mm churn (e.g. balloon-driven madvise(MADV_DONTNEED), mremap, fork against the same mm) through UFFDIO_COPY in two distinct ways: as an EAGAIN errno from the syscall, or — once UFFD_FEATURE_EVENT_REMOVE is enabled — through the partial-copy convention where the syscall returns 0 and cpy.copy carries either -EAGAIN or 0..pagesize. Hugetlb pages can also surface a positive short copy if a fault preempts the operation mid-page (#7). Pre-#2520 the latter path went through fmt.Errorf("UFFDIO_COPY copied N bytes...") and fell into the catch-all writeErr != nil arm — which calls onFailure() / fdExit.SignalExit(), tears the uffd serve loop down, and crashes the sandbox the moment the guest touches an unmapped page. The pre-existing errno-EAGAIN soft handler covered only the syscall errno path. Move the partial-copy classification into a small helper so both surfaces collapse onto the existing EAGAIN-returning-(false, nil) branch in faultPage. No retry budget — matches Firecracker's reference handler in src/firecracker/examples/uffd/uffd_utils.rs (Err(PartiallyCopied(n)) if n == 0 || n == -EAGAIN ⇒ return false). Add a uffd.copy_eagain span attribute for observability. Tests: unit-test classifyCopyResult directly. faultPage doesn't expose an Fd seam to mock UFFDIO_COPY without an interface refactor that would materially expand the diff; per the audit's "smallest pragmatic test" guidance the classifier covers the new branching and the existing cross-process matrix tests cover the integration path.

…onrpc over unix socket (#2519) Replace the cross-process userfaultfd test harness's pipes + signals (`SIGUSR1` shutdown, `SIGUSR2` page-state snapshot, ready/offsets/gate-cmd/gate-sync pipes) with one Unix socket carrying stdlib `net/rpc` + `net/rpc/jsonrpc`. The userfaultfd and the rpc socketpair half are passed via `ExtraFiles`. Production change: one `atomic.Pointer[func(uintptr, faultPhase)]` field on `Userfaultfd` and three nil-checked inline call sites. Test builds install the hook via `SetTestFaultHook` defined in a `_test.go` file. Stacked follow-ups: - `UFFD_EVENT_REMOVE` handling + matrix tests — #2520 - Stale-source / madvise-deadlock / faulted-short-circuit race tests — #2521 - Stale-source race fix — #2512

…loon Adds the FC-side integration plumbing for free page reporting on top of the UFFD REMOVE-event handling in #2520: - template-manager proto: optional bool freePageReporting (field 17). - TemplateConfig + sandbox.Config gain a FreePageReporting bool that flows from template create → build phases (base/steps/finalize) → sandbox factory → fc.Process.Create. - fc.apiClient.enableFreePageReporting calls PUT /balloon with free_page_reporting=true after entropy setup and before VM start. - fcversion.HasFreePageReporting gates rollout to FC v1.14+. - Adds free-page-reporting LaunchDarkly feature flag. - create-build CLI: --free-page-reporting flag, defaults to enabled when FC version supports it. - smoketest: opportunistically enables FPR when the FC version under test supports it. UFFD-side changes (REMOVE handling, page tracker, race tests, fix) remain in #2520; this PR is purely the production rollout path.

Replace the generic block.StateTracker[S] with a non-generic block.Tracker over a fixed block.State enum (NotPresent default, Dirty, Zero). The same three states cover both UFFD page management (NotPresent=missing, Dirty=faulted, Zero=removed via DONTNEED/balloon) and upcoming NBD overlay tracking (NotPresent=fall-through, Dirty=overlay-owned, Zero=explicit zero/discard).

Production: - UFFDIO_REGISTER_MODE_REMOVE is requested so the kernel reports MADV_DONTNEED'd pages via UFFD_EVENT_REMOVE. - Userfaultfd.Serve splits read events into removes + pagefaults, drains the REMOVE batch under settleRequests.Lock (calling pageTracker.SetRange(.., removed) with BlockIdx-computed indices), then dispatches the pagefault batch. - Worker dispatch switches on pageTracker.Get(idx): faulted -> short-circuit, removed -> zero-fill (source = nil), missing -> copy from u.src. The state read happens inside the worker under settleRequests.RLock so a concurrent REMOVE can't slip between the read and the install. - faultPage gains zero-fill paths for source == nil (4K read = DONTWAKE zero + WP + wake; 4K write = zero + wake; hugepage = copy(EmptyHugePage)) and returns (handled, err) so the worker can defer UFFDIO_COPY EAGAIN back into a deferredFaults queue. - wakeupPipe + deferredFaults wake the poll loop when a worker defers, so a deferred fault doesn't sit waiting for an unrelated UFFD event. The received uffd fd is marked FD_CLOEXEC. - Prefault short-circuits for faulted || removed. Tests: - testConfig gains removeEnabled; the parent unregisters the UFFD region on cleanup when REMOVE is on so munmap doesn't block on un-acked events. - Page-state wire format exposes removed via helpers_test.go. - operationModeRemove + executeRemove (madvise MADV_DONTNEED). - runMatrix wraps every existing generic test in remove-off and remove-on subtests so the no-REMOVE path (still used by production templates) stays covered while the new path is exercised. The matrix-level t.Parallel() is intentionally omitted to cap peak concurrency in CI. - remove_test.go: TestRemove, TestRemoveThenFault, TestRemoveThenWriteGated, TestWriteThenRemoveGated. Gated tests are //nolint:tparallel — a paused gated handler keeps a faulting goroutine suspended in the kernel pagefault path; a STW GC pause from a parallel test would wait forever for that goroutine to reach a safe point. - race_test.go: deterministic stale-source / madvise-deadlock / faulted-short-circuit regressions, serialised, with the FD_CLOEXEC and UFFDIO_COPY-EAGAIN fixes covered.

A worker holding settleRequests.RLock must never block readEvents, because madvise(MADV_DONTNEED) blocks the producer until userspace reads the UFFD_EVENT_REMOVE — and the producer can be the FC balloon thread that other syscalls depend on. Use a dedicated readSerial mutex (not settleRequests) to serialize serve-loop iterations with snapshot-time Export, while keeping the existing settleRequests discipline (workers RLock, REMOVE batch Lock) intact so readEvents remains lock-free relative to workers. Restores liveness for TestNoMadviseDeadlockWithInflightCopy while closing the read-vs-apply race that motivated the prior buggy commit (345f7e9, now amended).

codecov · 2026-05-04T19:00:52Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
2467	2	2465	5

View the full list of 2 ❄️ flaky test(s)

github.com/e2b-dev/infra/tests/integration/internal/tests/api/sandboxes::TestUpdateNetworkConfig

Flake rate in main: 57.14% (Passed 6 times, Failed 8 times)

Stack Traces | 25.9s run time

=== RUN   TestUpdateNetworkConfig
=== PAUSE TestUpdateNetworkConfig
=== CONT  TestUpdateNetworkConfig
--- FAIL: TestUpdateNetworkConfig (25.92s)

github.com/e2b-dev/infra/tests/integration/internal/tests/api/sandboxes::TestUpdateNetworkConfig/pause_resume_preserves_allow_internet_access_false

Flake rate in main: 50.00% (Passed 6 times, Failed 6 times)

Stack Traces | 1.01s run time

=== RUN   TestUpdateNetworkConfig/pause_resume_preserves_allow_internet_access_false
Executing command curl in sandbox iyudvtlbvekiggskvnu7o
    sandbox_network_update_test.go:372: Command [curl] output: event:{start:{pid:1350}}
    sandbox_network_update_test.go:372: Command [curl] output: event:{end:{exit_code:35 exited:true status:"exit status 35" error:"exit status 35"}}
Executing command curl in sandbox iyudvtlbvekiggskvnu7o
    sandbox_network_update_test.go:372: Command [curl] output: event:{start:{pid:1351}}
    sandbox_network_update_test.go:372: Command [curl] output: event:{end:{exit_code:35 exited:true status:"exit status 35" error:"exit status 35"}}
    sandbox_network_update_test.go:391: Command [curl] output: event:{start:{pid:1352}}
    sandbox_network_update_test.go:391: Command [curl] output: event:{data:{stdout:"HTTP/2 302 \r\nx-content-type-options: nosniff\r\nlocation: https://dns.google/\r\ndate: Tue, 05 May 2026 00:48:31 GMT\r\ncontent-type: text/html; charset=UTF-8\r\nserver: HTTP server (unknown)\r\ncontent-length: 216\r\nx-xss-protection: 0\r\nx-frame-options: SAMEORIGIN\r\nalt-svc: h3=\":443\"; ma=2592000,h3-29=\":443\"; ma=2592000\r\n\r\n"}}
    sandbox_network_update_test.go:391: Command [curl] output: event:{end:{exited:true status:"exit status 0"}}
    sandbox_network_update_test.go:391: Command [curl] completed successfully in sandbox iyudvtlbvekiggskvnu7o
    sandbox_network_update_test.go:391: 
        	Error Trace:	.../api/sandboxes/sandbox_network_out_test.go:74
        	            				.../api/sandboxes/sandbox_network_update_test.go:60
        	            				.../api/sandboxes/sandbox_network_update_test.go:391
        	Error:      	An error is expected but got nil.
        	Test:       	TestUpdateNetworkConfig/pause_resume_preserves_allow_internet_access_false
        	Messages:   	https://8.8.8.8 should be blocked
--- FAIL: TestUpdateNetworkConfig/pause_resume_preserves_allow_internet_access_false (1.01s)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d3c128b0a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

✅ Fixed: uint32 truncation in REMOVE range calculation overflows for >4GB
- Changed to divide (rm.end-rm.start) by uint64(pageSize) before casting to uint32, preventing byte-length truncation for REMOVE events spanning >4GB.

Or push these changes by commenting:

@cursor push 2ebb482dd8

Preview (2ebb482dd8)

diff --git a/packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go b/packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go
--- a/packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go
+++ b/packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go
@@ -289,7 +289,7 @@
 					}
 
 					startIdx := uint32(header.BlockIdx(startOff, int64(u.pageSize)))
-					endIdx := startIdx + uint32(rm.end-rm.start)/uint32(u.pageSize)
+					endIdx := startIdx + uint32((rm.end-rm.start)/uint64(u.pageSize))
 					u.pageTracker.SetRange(startIdx, endIdx, block.Zero)
 				}
 				u.settleRequests.Unlock()

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit dee755c. Configure here.}

uint32(rm.end-rm.start) truncated the byte length to 32 bits before dividing by pageSize, silently dropping pages from REMOVE events larger than 4 GiB. Divide first; uint32 block indices still address up to ~16 TiB at 4 KiB pages.

… hush self-wake faultPage now returns a three-valued faultOutcome (Installed/Deferred/ Discarded). ESRCH maps to Discarded so the caller no longer marks a never-installed page as Dirty in the tracker. EAGAIN maps to Deferred (same behaviour as before). Panic recover returns Discarded + err so the Serve loop exits rather than looping. Serve gained a top-level defer wg.Wait() so workers can't outlive Serve on error-return paths and race with Close() on wakeupPipe / the uffd fd. Stop incrementing noDataCounter on wakeupPipe self-wakes — those are expected (a worker deferred a fault); they were polluting the uffd-no-data metric.

Adds `block.Tracker`, a non-generic state tracker over a fixed `block.State` enum with three universal values that cover both UFFD memory handling and NBD overlay tracking: - `NotPresent` (default) — fall through to the previous layer - `Dirty` — materialized in this layer (UFFD faulted page, NBD overlay-owned block) - `Zero` — known-zero without consulting the previous layer (UFFD page removed via `DONTNEED`/balloon, NBD explicit zero/discard) Backed by two roaring bitmaps (`dirty`, `zero`); `NotPresent` indices are implicit. Exposes `SetRange`, `Get`, and `Export` (returns clones). First consumer is #2520 (UFFD per-page state); also used by #2546 for NBD zero-page tracking.

bchalios

LGTM

…loon Adds the FC-side integration plumbing for free page reporting on top of the UFFD REMOVE-event handling in #2520: - template-manager proto: optional bool freePageReporting (field 17). - TemplateConfig + sandbox.Config gain a FreePageReporting bool that flows from template create → build phases (base/steps/finalize) → sandbox factory → fc.Process.Create. - fc.apiClient.enableFreePageReporting calls PUT /balloon with free_page_reporting=true after entropy setup and before VM start. - fcversion.HasFreePageReporting gates rollout to FC v1.14+. - Adds free-page-reporting LaunchDarkly feature flag. - create-build CLI: --free-page-reporting flag, defaults to enabled when FC version supports it. - smoketest: opportunistically enables FPR when the FC version under test supports it. UFFD-side changes (REMOVE handling, page tracker, race tests, fix) remain in #2520; this PR is purely the production rollout path.

…loon Adds the FC-side integration plumbing for free page reporting on top of the UFFD REMOVE-event handling in #2520: - template-manager proto: optional bool freePageReporting (field 17). - TemplateConfig + sandbox.Config gain a FreePageReporting bool that flows from template create → build phases (base/steps/finalize) → sandbox factory → fc.Process.Create. - fc.apiClient.enableFreePageReporting calls PUT /balloon with free_page_reporting=true after entropy setup and before VM start. - fcversion.HasFreePageReporting + auto-detect in TemplateCreate gate rollout to FC v1.14+. - LaunchDarkly free-page-reporting flag (default off).

Wires Firecracker balloon free-page-reporting through the template build into `pkg/sandbox/fc`. In the orchestrator path FPR turns on only when `fcInfo.HasFreePageReporting()` (FC v1.14+) AND the `free-page-reporting` LaunchDarkly flag are both true; `cmd/create-build` defaults to the FC-version-derived value and accepts an explicit `--free-page-reporting` override. Depends on #2545 → #2520 — without the REMOVE-event handling from #2520, FC's `madvise(MADV_DONTNEED)` on balloon deflate would race in-flight pagefault workers.

ValentaTomas requested review from dobrac and jakubno as code owners April 29, 2026 00:23

e2b-request-same-site-reviewers Bot assigned levb Apr 29, 2026

chatgpt-codex-connector Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

cursor Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go

claude Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/prefault.go

claude Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go

claude Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

ValentaTomas marked this pull request as draft May 1, 2026 18:05

ValentaTomas mentioned this pull request May 2, 2026

refactor(uffd-tests): replace SIGUSR/env-var/pipe IPC with net/rpc/jsonrpc over unix socket #2519

Merged

ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from da91230 to efa01a5 Compare May 2, 2026 08:48

ValentaTomas changed the title ~~feat(uffd): add UFFD_EVENT_REMOVE handling, removed pageState, and matrix-mode tests~~ feat(uffd): handle UFFD_EVENT_REMOVE and add matrix-mode tests May 2, 2026

Base automatically changed from refactor/uffd-test-harness to main May 2, 2026 19:32

ValentaTomas changed the title ~~feat(uffd): handle UFFD_EVENT_REMOVE and add matrix-mode tests~~ feat(uffd): UFFD_EVENT_REMOVE handling, race tests, and stale-source fix May 3, 2026

ValentaTomas mentioned this pull request May 3, 2026

feat(uffd,fc): wire balloon free-page-reporting #2541

Merged

cursor Bot reviewed May 3, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go

This was referenced May 3, 2026

refactor(uffd-tests): pre-fault uffd-registered pages via madvise to eliminate STW-deadlock class #2542

Closed

test(uffd): rapid state-machine + chaos-source coverage #2544

Closed

ValentaTomas changed the title ~~feat(uffd): UFFD_EVENT_REMOVE handling, race tests, and stale-source fix~~ feat(uffd): handle UFFD_EVENT_REMOVE; track per-page state; race-safe COPY May 3, 2026

cursor Bot reviewed May 3, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/prefault.go Outdated

ValentaTomas added 3 commits May 4, 2026 11:50

refactor(state-tracker): trim verbose doc comments

1949d9b

refactor(block): SetRange takes uint32 bounds

dcb7de4

ValentaTomas force-pushed the feat/block-state-tracker branch from 37be905 to dcb7de4 Compare May 4, 2026 18:50

claude Bot reviewed May 4, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go

ValentaTomas and others added 4 commits May 4, 2026 11:57

fix(uffd): hold settle Lock across PrefetchData read

19f5006

chore: auto-commit generated changes

4d3c128

ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from e44d433 to 4d3c128 Compare May 4, 2026 18:59

chore: auto-commit generated changes

dee755c

chatgpt-codex-connector Bot reviewed May 4, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

cursor Bot reviewed May 4, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

claude Bot reviewed May 4, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go Outdated

ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from a02b428 to 89d54b1 Compare May 4, 2026 19:50

claude Bot reviewed May 4, 2026

View reviewed changes

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/prefault.go Outdated

Base automatically changed from feat/block-state-tracker to main May 4, 2026 23:27

Merge branch 'main' into feat/uffd-remove-events-matrix

611d0d6

ValentaTomas requested review from bchalios and removed request for jakubno May 5, 2026 02:25

bchalios approved these changes May 5, 2026

View reviewed changes

ValentaTomas merged commit b01a6a1 into main May 5, 2026
48 checks passed

ValentaTomas deleted the feat/uffd-remove-events-matrix branch May 5, 2026 08:01

Conversation

ValentaTomas commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 2 Tests Failed:

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bchalios left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ValentaTomas commented Apr 29, 2026 •

edited

Loading

cursor Bot commented Apr 29, 2026 •

edited

Loading

codecov Bot commented May 4, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading