Skip to content

Commit cfe352d

Browse files
codebytereaduh95
authored andcommitted
sea: keep ELF segments on separate pages in --build-sea output
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> PR-URL: #65564 Refs: nodejs/build#4433 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Stewart X Addison <sxa@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 8b7e78f commit cfe352d

2 files changed

Lines changed: 47 additions & 0 deletions

File tree

src/node_sea_bin.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ InjectOutput InjectIntoELF(const std::vector<uint8_t>& executable,
126126
{},
127127
SPrintF("Failed to create new ELF note %s", note_name)};
128128
}
129+
// Left to itself LIEF moves a non-PIE executable's program headers into
130+
// the largest gap between two PT_LOADs and extends the earlier one over it,
131+
// which can leave that segment sharing a page with the next; kernels that
132+
// map segments with MAP_FIXED_NOREPLACE (Linux < 5.4, RHEL 8) then refuse
133+
// to run the file. After .bss every segment keeps its own pages.
134+
if (binary->header().file_type() == LIEF::ELF::Header::FILE_TYPE::EXEC &&
135+
binary->relocate_phdr_table(LIEF::ELF::Binary::PHDR_RELOC::BSS_END) ==
136+
0) {
137+
return {InjectResult::kError, {}, "Failed to relocate ELF program headers"};
138+
}
129139
binary->add(*new_note);
130140

131141
LIEF::ELF::Builder::config_t cfg;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
// This tests that --build-sea emits an ELF whose PT_LOAD segments never share
3+
// a page, since kernels that map them with MAP_FIXED_NOREPLACE (Linux < 5.4,
4+
// RHEL 8) refuse to execute such a file.
5+
6+
const common = require('../common');
7+
if (process.platform !== 'linux')
8+
common.skip('ELF-specific');
9+
10+
const { buildSEA, skipIfBuildSEAIsNotSupported } = require('../common/sea');
11+
skipIfBuildSEAIsNotSupported();
12+
13+
const assert = require('assert');
14+
const { readFileSync } = require('fs');
15+
const fixtures = require('../common/fixtures');
16+
const tmpdir = require('../common/tmpdir');
17+
18+
tmpdir.refresh();
19+
const elf = readFileSync(buildSEA(fixtures.path('sea', 'basic')));
20+
assert.strictEqual(elf[4], 2); // ELFCLASS64
21+
const phoff = Number(elf.readBigUInt64LE(0x20));
22+
const phentsize = elf.readUInt16LE(0x36);
23+
const phnum = elf.readUInt16LE(0x38);
24+
const loads = [];
25+
for (let i = 0; i < phnum; i++) {
26+
const at = phoff + i * phentsize;
27+
if (elf.readUInt32LE(at) !== 1) continue; // PT_LOAD
28+
const vaddr = elf.readBigUInt64LE(at + 0x10);
29+
const memsz = elf.readBigUInt64LE(at + 0x28);
30+
if (memsz > 0n) loads.push({ first: vaddr >> 12n, last: (vaddr + memsz - 1n) >> 12n });
31+
}
32+
loads.sort((a, b) => (a.first < b.first ? -1 : 1));
33+
for (let i = 1; i < loads.length; i++) {
34+
assert(loads[i].first > loads[i - 1].last,
35+
`PT_LOAD ${i} starts on page 0x${loads[i].first.toString(16)}, ` +
36+
`which PT_LOAD ${i - 1} already covers`);
37+
}

0 commit comments

Comments
 (0)