Skip to content

Commit e23e282

Browse files
committed
mm: preserve file_vma type when cloning a child address space on fork
clone_address_space() rebuilt EVERY child VMA as an anon_vma, so a fork child's file-backed mappings (file_vma -- e.g. the ELF loader's file-backed .text, mapped via map_file) became anonymous in the child. A demand fault on a child file-backed page that the parent had not already resident-loaded then dispatched to the base zero-fill path instead of file_vma::fault reading the file: the child read (or executed) zeros. This is the "wild-branch" bug that blocked file-backed fork workloads such as PostgreSQL, whose checkpointer child faults large cold .text regions the postmaster never touched. Preserve each parent VMA's dynamic type across the clone: - In the VMA-clone loop (under the parent's vmas_mutex), dynamic_cast each vma to file_vma. For a file_vma, rebuild the child's copy via the file's own mmap() -- the SAME call map_file and file_vma::split use -- so the child gets the correct page_allocator (map_file_page_read for MAP_PRIVATE, map_file_page_mmap for a cached/shared mapping) and its demand faults read the file. The new file_vma holds its own fileref. Anon vmas still clone as anon_vma. file::mmap()/default_file_mmap()/map_file_mmap() are pure allocation (no locks, no I/O), matching the anon_vma allocation this loop already performs under the lock. - In destroy_address_space(), dispose the child's owned vma objects before freeing the address space. ~file_vma releases the fileref this child took (and deletes its page_allocator), closing the file-reference leak the type preservation would otherwise introduce -- and also the pre-existing anon_vma + edge-marker leak. ~vma touches no global list, so disposing here is safe. The whole change is inside #if CONF_fork, so a conf_fork=0 build is byte-identical. Adds tests/tst-fork-file-mmap.cc (gated via the fork test group): a parent mmaps a read-only ROFS file MAP_PRIVATE without faulting a mid-file page, forks, and the child faults that page first -- it must read the real file bytes, not zeros. Reproduces the bug on the unfixed tree (child reads 0x00) and passes with this fix (child reads the real byte). Signed-off-by: Greg Burd <greg@burd.me>
1 parent 95a650c commit e23e282

3 files changed

Lines changed: 165 additions & 2 deletions

File tree

core/mmu.cc

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,13 +487,36 @@ address_space *clone_address_space(address_space *parent)
487487
// Structurally clone the parent's VMAs into the child's vma_list so
488488
// the fault path can resolve child faults (permissions, backing). The
489489
// physical pages are already wired via the cloned page tables above.
490+
//
491+
// Preserve each parent vma's DYNAMIC TYPE. A file_vma (e.g. the ELF
492+
// loader's file-backed .text, created via map_file) must clone as a
493+
// file_vma bound to the SAME file + offset -- otherwise the child's
494+
// demand faults dispatch to the base zero-fill path and it executes
495+
// zeros over its own .text (the fork COW-clone wild-branch bug). A
496+
// file-backed vma is rebuilt via the file's own mmap() -- the SAME
497+
// call map_file and file_vma::split use -- so the child gets the
498+
// correct page_allocator (map_file_page_read for MAP_PRIVATE,
499+
// map_file_page_mmap for a cached/shared mapping) and its demand
500+
// faults read the file instead of zero-filling. The new file_vma
501+
// holds its OWN fileref; ~file_vma in destroy_address_space releases
502+
// it. file::mmap()/default_file_mmap()/map_file_mmap() are pure
503+
// allocation (no locks, no I/O), so calling them here under
504+
// vmas_mutex is safe -- the same as the anon_vma allocation this loop
505+
// already does under the lock.
490506
for (auto &v : *parent->vmas) {
491507
// Skip the edge marker VMAs (size 0); the child's vma_list_type
492508
// constructor already inserted its own edge markers.
493509
if (v.size() == 0) {
494510
continue;
495511
}
496-
auto *nv = new anon_vma(addr_range(v.start(), v.end()), v.perm(), v.flags());
512+
vma *nv;
513+
auto *fv = dynamic_cast<file_vma *>(&v);
514+
if (fv) {
515+
nv = fv->file()->mmap(addr_range(v.start(), v.end()),
516+
v.flags(), v.perm(), fv->offset()).release();
517+
} else {
518+
nv = new anon_vma(addr_range(v.start(), v.end()), v.perm(), v.flags());
519+
}
497520
child->vmas->insert(*nv);
498521
}
499522
return child;
@@ -565,6 +588,16 @@ void destroy_address_space(address_space *as)
565588
}
566589
memory::free_page(phys_to_virt(pml4_phys));
567590
}
591+
// Dispose the child's own vma objects (the clones built in
592+
// clone_address_space, plus the vma_list edge markers). ~vma is a no-op
593+
// for the base/anon case; for a file_vma it deletes the page_allocator and
594+
// releases the fileref this child took in clone_address_space -- so the
595+
// file-backed clone does not leak its file reference. (This also closes
596+
// the pre-existing anon_vma + edge-marker leak.) ~vma touches no global
597+
// list, so disposing here is safe.
598+
if (as->owned_vmas) {
599+
as->owned_vmas->clear_and_dispose([](vma *v) { delete v; });
600+
}
568601
delete as;
569602
live_child_address_spaces.fetch_sub(1, std::memory_order_relaxed);
570603
}

modules/tests/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so tst-mq-smoke.so tst-bsd-evh.
131131
misc-ctxsw.so tst-read.so tst-symlink.so tst-openat.so \
132132
tst-eventfd.so tst-remove.so misc-wake.so tst-epoll.so misc-lfring.so \
133133
tst-fork.so tst-execve.so payload-exit7.so \
134-
tst-fork-cow.so tst-fork-deep.so \
134+
tst-fork-cow.so tst-fork-deep.so tst-fork-file-mmap.so \
135135
misc-fsx.so tst-sleep.so tst-resolve.so tst-except.so \
136136
misc-tcp-sendonly.so tst-tcp-nbwrite.so misc-tcp-hash-srv.so \
137137
misc-loadbalance.so misc-scheduler.so tst-console.so tst-app.so \

tests/tst-fork-file-mmap.cc

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Copyright (C) 2026 Greg Burd
3+
*
4+
* This work is open source software, licensed under the terms of the
5+
* BSD license as described in the LICENSE file in the top-level directory.
6+
*
7+
* REGRESSION TEST for [W-branch]: a fork child that demand-faults a page of a
8+
* FILE-backed (MAP_PRIVATE) mapping which the PARENT never touched before the
9+
* fork. Before the clone_address_space type-preservation fix this returned
10+
* ZEROS (the bug): clone_address_space rebuilt EVERY child vma as an anon_vma,
11+
* so a file_vma (e.g. PostgreSQL's file-backed .text, mapped by the ELF loader)
12+
* became anonymous in the child and its demand fault hit the base zero-fill
13+
* path instead of file_vma::fault reading the file. It now PASSES (the child
14+
* reads the real file bytes).
15+
*
16+
* The mapping is of a READ-ONLY ROFS file -- matching the real case (an
17+
* executable's file-backed .text, demand-paged from the read-only image), and
18+
* avoiding the writable-fs access-time update that would otherwise write to the
19+
* inode during a read fault.
20+
*
21+
* Root cause (proven via KVM+hbreak, see /tmp/w-branch-kvm.txt):
22+
* core/mmu.cc clone_address_space():
23+
* struct vma_snap { start, end, perm, flags; }; // dropped dynamic type
24+
* snap.push_back({v.start(), v.end(), v.perm(), v.flags()});
25+
* auto *nv = new anon_vma(...); // ALWAYS anon_vma
26+
* The child's file-backed VA thus faulted to a fresh ZERO anon page. The
27+
* existing tst-fork* tests only exercise ANON memory, so they miss it; PG is
28+
* the first fork whose child demand-faults a file-backed .text page the
29+
* parent never resident-loaded.
30+
*
31+
* Mechanism the test recreates:
32+
* - parent opens a ROFS file and reads the EXPECTED bytes of a mid-file page
33+
* via pread(2) on a SEPARATE fd (this never faults the mapping's PTE);
34+
* - parent mmaps the file MAP_PRIVATE but does NOT read the target page (so
35+
* no PTE for it exists in the parent -> the COW page-table clone hands the
36+
* child an absent PTE);
37+
* - fork();
38+
* - the CHILD reads the target page FIRST -> demand fault -> must read the
39+
* real file bytes. On buggy HEAD it reads zeros (MISMATCH).
40+
*
41+
* Deterministic on -smp 1. Gated CONF_fork.
42+
*
43+
* To reproduce (in arena-dev, CONF_fork=y build):
44+
* ./scripts/build conf_fork=1 fs=rofs image=tests, then:
45+
* ./scripts/run.py -c 1 -e /tests/tst-fork-file-mmap.so
46+
*/
47+
48+
#include <unistd.h>
49+
#include <sys/wait.h>
50+
#include <sys/types.h>
51+
#include <sys/mman.h>
52+
#include <sys/stat.h>
53+
#include <fcntl.h>
54+
#include <cstdio>
55+
#include <cstring>
56+
#include <cstdint>
57+
#include <cerrno>
58+
59+
int main()
60+
{
61+
printf("=== tst-fork-file-mmap ===\n");
62+
const size_t page = (size_t)sysconf(_SC_PAGESIZE);
63+
64+
// A read-only ROFS file present on the test image (~47 KiB).
65+
const char *path = "/rofs/mmap-file-test1";
66+
int fd = open(path, O_RDONLY);
67+
if (fd < 0) {
68+
// fall back to the /tmp copy the image also ships
69+
fd = open("/tmp/mmap-file-test1", O_RDONLY);
70+
}
71+
if (fd < 0) { printf("FAIL: open %s (%s)\n", path, strerror(errno)); return 1; }
72+
73+
struct stat st;
74+
if (fstat(fd, &st) != 0) { printf("FAIL: fstat\n"); return 1; }
75+
size_t size = (size_t)st.st_size;
76+
if (size < 6 * page) { printf("FAIL: file too small (%zu)\n", size); return 1; }
77+
78+
// Target page the child will fault: a mid-file page (well past page 0) that
79+
// the parent never touches through the mapping.
80+
const size_t target_pg = 5;
81+
const off_t target_off = (off_t)(target_pg * page);
82+
83+
// Read the EXPECTED bytes of that page via a SEPARATE fd + pread -- this
84+
// does not fault the mapping's PTE, so the child still demand-faults it.
85+
unsigned char *want = new unsigned char[page];
86+
{
87+
int fd2 = open(path, O_RDONLY);
88+
if (fd2 < 0) fd2 = open("/tmp/mmap-file-test1", O_RDONLY);
89+
if (fd2 < 0) { printf("FAIL: open2\n"); return 1; }
90+
ssize_t n = pread(fd2, want, page, target_off);
91+
close(fd2);
92+
if (n != (ssize_t)page) { printf("FAIL: pread expected page (%zd)\n", n); return 1; }
93+
}
94+
95+
// mmap the file MAP_PRIVATE, read-only. Do NOT touch the target page.
96+
unsigned char *p = (unsigned char *)mmap(nullptr, size, PROT_READ,
97+
MAP_PRIVATE, fd, 0);
98+
if (p == MAP_FAILED) { printf("FAIL: mmap (%s)\n", strerror(errno)); return 1; }
99+
close(fd); // mmap holds its own fileref
100+
101+
pid_t pid = fork();
102+
if (pid == 0) {
103+
// CHILD: first access to the target page -> demand fault. On buggy HEAD
104+
// this reads a zero-filled anon page; with the fix it reads file bytes.
105+
volatile unsigned char got0 = p[target_off];
106+
int bad = memcmp((const void *)(p + target_off), want, page);
107+
printf("child: page %zu byte0=0x%02x want=0x%02x %s\n",
108+
target_pg, (unsigned)got0, (unsigned)want[0],
109+
bad ? "MISMATCH" : "OK");
110+
_exit(bad ? 70 : 55);
111+
}
112+
113+
int status = 0;
114+
waitpid(pid, &status, 0);
115+
munmap(p, size);
116+
delete[] want;
117+
118+
if (WIFEXITED(status) && WEXITSTATUS(status) == 55) {
119+
printf("PASS: fork child reads real file bytes from an untouched "
120+
"file-backed mmap page\n");
121+
return 0;
122+
}
123+
if (WIFEXITED(status) && WEXITSTATUS(status) == 70) {
124+
printf("FAIL/BUG [W-branch]: child saw ZEROS/wrong data -- file_vma "
125+
"cloned as anon_vma (status=0x%x)\n", status);
126+
} else {
127+
printf("FAIL: child aborted (status=0x%x)\n", status);
128+
}
129+
return 1;
130+
}

0 commit comments

Comments
 (0)