sea: keep ELF segments on separate pages in --build-sea output - #65564
Open
codebytere wants to merge 1 commit into
Open
sea: keep ELF segments on separate pages in --build-sea output#65564codebytere wants to merge 1 commit into
codebytere wants to merge 1 commit into
Conversation
When node itself is not position independent (the official Linux
binaries, and any build with a toolchain that does not default to PIE),
LIEF made room for the extra program header by moving the header table
into the largest gap between two PT_LOAD segments and extending the
earlier segment across that gap. Whenever the gap it picked was the one
between the read-only data and the read-write segment, whose boundary
is not page aligned, the extended segment ended inside the first page
of the next one. Linux 4.17 to 5.3, and RHEL 8's 4.18 kernel, map an
executable's segments with MAP_FIXED_NOREPLACE and refuse the second
mapping, so the single executable was killed with SIGSEGV before it ran
a single instruction ('elf segment at ... requested but the memory is
mapped already' in the kernel log). Which gap is largest depends on
section sizes, so roughly one build in three produced such binaries.
Ask LIEF to place the table after .bss for non-PIE executables instead,
which leaves every existing segment as the linker laid it out; the
output grows by the size of .bss. A test checks that no two PT_LOAD
segments of a --build-sea executable share a page.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
needs-ci
panva
added
fast-track
Contributor
|
Fast-track has been requested by @panva. Please 👍 to approve. |
github-actions
Bot
removed
the
request-ci
This comment has been minimized.
This comment has been minimized.
Member
|
That's not ideal - merge conflict on the Attempting a test in the stress job with 100 iterations: rhel8-x64 and rhel9-x64 as we really need the results on rhel8-x64 ASAP. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65564 +/- ##
=======================================
Coverage 90.05% 90.06%
=======================================
Files 751 751
Lines 254420 254423 +3
Branches 47975 47986 +11
=======================================
+ Hits 229121 229148 +27
+ Misses 16483 16444 -39
- Partials 8816 8831 +15
🚀 New features to boost your workflow:
|
panva
added
the
flaky-test
sxa
approved these changes
Aug 26, 2026
sxa
left a comment
sxa
left a comment
Member
There was a problem hiding this comment.
Annoying (as usual...) but https://ci.nodejs.org/job/node-stress-single-test/nodes=rhel8-x64/848/console which was without this patch seemed to be passing too. But on the basis this has been tested in an environment where it was reproducible I'm approving.
Out of interest, did you manage to come to any conclusion abot what change could have caused this to start going wrong recently given that it doesn't seem to have been package updates?
This comment has been minimized.
This comment has been minimized.
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
sea/*failures on the rhel8-x64 CI hosts (SIGSEGV, empty stderr, https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md) come with a kernel line,elf segment at ... requested but the memory is mapped already(nodejs/build#4433 (comment)), which isexecverefusing the injected executable. Whennodeis not PIE (RHEL toolchains, and the official Linux binaries), LIEF makes room for the new program header by moving the header table into the largest gap between twoPT_LOADsegments and extending the earlier segment across it; when that gap is the one between the read-only data and the read-write segment, whose boundary is not page aligned, the extended segment ends inside the next segment's first page. Linux 4.17 to 5.3 and RHEL 8's 4.18 map an executable's segments withMAP_FIXED_NOREPLACE, so the second mapping fails past the point of no return and the process is killed; newer kernels don't check. Which gap is largest depends on section sizes, so it comes and goes from build to build.This asks LIEF for its after-
.bssplacement when the executable isET_EXEC, which leaves every existing segment where the linker put it and gives the header table pages of its own; the output grows by the size of.bss(about 340 KB fornode). PIE builds are unchanged. postject makes the same choice in its own copy of LIEF, sotest-single-executable-application.jsandtest_sea_addon, which still inject with it, aren't covered by this.Tests:
test/sea/test-build-sea-elf-segments.jsasserts no twoPT_LOADsegments of a--build-seaexecutable share a page;test/seapassesnodebuilt with RHEL 8's clang whose read-only/read-write gap is the largest: on AlmaLinux 8.10 (4.18.0-553.150.1.el8_10) the SEA built before this change segfaults with the same kernel line and the one built after it runs; both run on a 6.12 kernelRefs: nodejs/build#4433
Disclosure: the code, test, investigation and this description were written by Claude Code, directed and reviewed by @codebytere.