Skip to content

Commit 26e0d44

Browse files
committed
fix(fixup): patchelf a copy + atomic rename — never rewrite live mappings
The fixup pipeline now runs on every toolchain install path, which means the process executing it can itself be linked against the very libraries being patched (a self-hosted mcpp loads the sandbox glibc/libgcc_s). In-place patchelf rewrites the backing file of those live mappings and corrupts the running process — reproduced deterministically on fresh- sandbox CI as an exit-time SIGSEGV in _dl_fini jumping to an unrelocated address (0x45a0), while a control run of main on the same fresh sandbox passed (the old code only ever patched from the statically-linked bootstrap mcpp, so the hazard was structural but unexposed). patchelf now operates on a copy and atomically rename()s it into place: the patched content gets a fresh inode, live processes keep the old one.
1 parent 24d138e commit 26e0d44

1 file changed

Lines changed: 29 additions & 4 deletions

File tree

src/toolchain/post_install.cppm

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,46 @@ export void patchelf_walk(const std::filesystem::path& dir,
6060
mcpp::platform::shell::quote(path.string()));
6161
auto probeResult = mcpp::platform::process::capture(probe);
6262
bool hasInterp = (probeResult.exit_code == 0 && !probeResult.output.empty());
63+
64+
// Patch a COPY and atomically rename it into place. The payload can
65+
// contain libraries the CURRENT process has mmapped (a self-hosted
66+
// mcpp links the sandbox glibc/libgcc_s, and since the fixup
67+
// pipeline runs on every install path, the patching process may BE
68+
// such a consumer). In-place patchelf rewrites the backing file of
69+
// those live mappings and corrupts the running process — observed
70+
// on CI as an exit-time SIGSEGV in _dl_fini jumping to an
71+
// unrelocated address. rename() gives the patched content a fresh
72+
// inode while live processes keep the old one.
73+
auto tmp = path;
74+
tmp += ".mcpp-patch.tmp";
75+
{
76+
std::error_code cec;
77+
std::filesystem::copy_file(
78+
path, tmp, std::filesystem::copy_options::overwrite_existing, cec);
79+
if (cec) continue;
80+
std::filesystem::permissions(
81+
tmp, std::filesystem::status(path, cec).permissions(),
82+
std::filesystem::perm_options::replace, cec);
83+
}
84+
bool patched = true;
6385
if (hasInterp) {
64-
(void)mcpp::platform::process::run_silent(std::format(
86+
patched = (mcpp::platform::process::run_silent(std::format(
6587
"{} --set-interpreter {} {} 2>/dev/null",
6688
mcpp::platform::shell::quote(patchelfBin.string()),
6789
mcpp::platform::shell::quote(loader.string()),
68-
mcpp::platform::shell::quote(path.string())));
90+
mcpp::platform::shell::quote(tmp.string()))) == 0) && patched;
6991
}
7092
// Always set RUNPATH (works on .so too — they need to find deps).
7193
if (!rpath.empty()) {
72-
(void)mcpp::platform::process::run_silent(std::format(
94+
patched = (mcpp::platform::process::run_silent(std::format(
7395
"{} --set-rpath {} {} 2>/dev/null",
7496
mcpp::platform::shell::quote(patchelfBin.string()),
7597
mcpp::platform::shell::quote(rpath),
76-
mcpp::platform::shell::quote(path.string())));
98+
mcpp::platform::shell::quote(tmp.string()))) == 0) && patched;
7799
}
100+
std::error_code rec;
101+
if (patched) std::filesystem::rename(tmp, path, rec);
102+
if (!patched || rec) std::filesystem::remove(tmp, rec);
78103
}
79104
}
80105

0 commit comments

Comments
 (0)