Conversation
A coalesced region can extend past the requested source range. Treat an overlap in the growth tail as occupied. MAYMOVE then relocates instead of clearing live bytes.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test-mremap.c">
<violation number="1" location="tests/test-mremap.c:417">
P3: The test's discriminating power rests on elfuse coalescing the two adjacent anonymous mappings into one tracked region; nothing verifies that coalescing happened. If region tracking stops merging adjacent anonymous mappings, both the ENOMEM and the relocation assertions pass without ever exercising the buggy in-place-growth path, and the test silently stops guarding this regression. Document the coalescing requirement, or assert it (for example, check /proc/self/maps shows one entry spanning both pages, or add a positive control growing into a free adjacent page) so the test cannot degrade into a no-op.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| unsigned char *first = mmap(base, page, PROT_READ | PROT_WRITE, | ||
| MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); | ||
| unsigned char *neighbor = |
There was a problem hiding this comment.
P3: The test's discriminating power rests on elfuse coalescing the two adjacent anonymous mappings into one tracked region; nothing verifies that coalescing happened. If region tracking stops merging adjacent anonymous mappings, both the ENOMEM and the relocation assertions pass without ever exercising the buggy in-place-growth path, and the test silently stops guarding this regression. Document the coalescing requirement, or assert it (for example, check /proc/self/maps shows one entry spanning both pages, or add a positive control growing into a free adjacent page) so the test cannot degrade into a no-op.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test-mremap.c, line 417:
<comment>The test's discriminating power rests on elfuse coalescing the two adjacent anonymous mappings into one tracked region; nothing verifies that coalescing happened. If region tracking stops merging adjacent anonymous mappings, both the ENOMEM and the relocation assertions pass without ever exercising the buggy in-place-growth path, and the test silently stops guarding this regression. Document the coalescing requirement, or assert it (for example, check /proc/self/maps shows one entry spanning both pages, or add a positive control growing into a free adjacent page) so the test cannot degrade into a no-op.</comment>
<file context>
@@ -397,6 +398,70 @@ static void test_source_range_hole(void)
+
+ unsigned char *first = mmap(base, page, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
+ unsigned char *neighbor =
+ mmap(base + page, page, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
</file context>
| bool can_grow = | ||
| !region_range_overlaps(g, grow_off, grow_off + grow_len); |
There was a problem hiding this comment.
Refusing in-place growth here sends a source whose region extends past grow_off down the move path, which is right, and it removes the old path's worst case: a memset of grow_len zeros straight into a live MAP_SHARED file overlay, which wrote those zeros to the backing file.
One hole survives the move. cleanup_overlays_in_range rounds its teardown up to the host page (ALIGN_UP(end, hps), 16 KiB on Apple Silicon) while guest ranges are 4 KiB granular, and hvf_restore_slab_backing re-mmaps fresh slab without repopulating from the file, so growing the first 4 KiB of a shared file mapping leaves the next 12 KiB of the surviving suffix reading slab instead of file contents. That was already reachable through a separate mapping at grow_off, so it is a follow-up rather than something this change owes.
| unsigned char *first = mmap(base, page, PROT_READ | PROT_WRITE, | ||
| MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); | ||
| unsigned char *neighbor = | ||
| mmap(base + page, page, PROT_READ | PROT_WRITE, | ||
| MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); |
There was a problem hiding this comment.
This case reaches the branch the fix changed only while guest_region_add still merges the two mappings into one region. If regions_mergeable_layout ever stops merging separate same-prot anonymous mmaps, neighbor becomes its own region starting at grow_off, the unpatched code refuses growth for the right reason, and the test passes without exercising the defect.
A second case that grows the first page of a single mmap(NULL, 2 * page, ...) hits the same occupied-suffix path by construction, since one mapping is one region regardless of the merge rules.
jserv
left a comment
jserv
left a comment
There was a problem hiding this comment.
Check https://github.com/sysprog21/elfuse/blob/main/CONTRIBUTING.md carefully.
2 participants
Problem
Compatible adjacent anonymous mappings can coalesce into one tracked region.
The in-place mremap growth check skipped an overlapping region when its start
matched the source address, even when that region extended past the requested
source range.
The growth path then treated the occupied suffix as free and zeroed live bytes
in the adjacent mapping. This surfaced as a consistent crash while Alpine apk
resolved packages from local repository indexes.
Linux permits in-place expansion only when the requested source range ends at
the end of its VMA. An occupied suffix must force relocation when
MREMAP_MAYMOVE is present, or ENOMEM otherwise.
Reproduction
Map two adjacent anonymous pages with matching permissions, fill each page
with a distinct pattern, and grow the first page to two pages.
Before this change, elfuse returned the original address and cleared the
second page. After this change:
zeroes the extension, and leaves the adjacent mapping unchanged.
Environment
Validation
make check-formatbuild/elfuse build/test-mremap: 13 passedtest-mremapunder the QEMU aarch64 Linux reference kernel: 13 passedapk --no-network --simulate add git: three consecutive runscompleted with status 0
make check: all 99 core tests and all mremap suites passed; the finalBusyBox
nslookupsmoke test failed because no DNS server was reachableCloses #389
Summary by cubic
Protects adjacent coalesced mappings from being overwritten during in-place
mremapgrowth. Previously, growth could zero live bytes in a neighboring mapping when the region extended past the source range; now it returnsENOMEMwithoutMREMAP_MAYMOVEand relocates withMREMAP_MAYMOVE, preserving both mappings.Written for commit 60b4185. Summary will update on new commits.