From dbf358ef8148a64bbfd8752871deb60552249ff4 Mon Sep 17 00:00:00 2001 From: tablefrogpillow <184222310+tablefrogpillow@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:25:03 +0000 Subject: [PATCH] fix(relinker): refuse write-execute PT_LOAD overlaps and bound the congruence check Addresses the #12591 review. Because the Windows image ORs page permissions, refuse a congruent overlap when one segment is executable and the other writable and the later segment does not start on a page boundary, which would otherwise share a read-write-execute page; the rule is applied on both loaders so their acceptance decision matches. Bound the congruence check to the file-backed part of the overlap and compare spans rather than walking the implicit zero bytes of a large bss overlap. Expand test_executable_segments.py: byte-congruent tail and bss overlaps accepted with their trimmed sizes and no output overlap; a write-execute overlap refused at an unaligned boundary and accepted at an aligned one with the Windows image carrying no write-execute section; differing bytes, an overlap past the later end, a non-zero file byte over the later segment's bss, and a shared start address refused; and a no-overlap control. Keep only the PPSA ids in the relinker.md note. Co-Authored-By: Claude Opus 4.8 --- .../elfpatcher/src/linux/LinuxElfPatcher.cpp | 32 ++++-- .../src/windows/WindowsLoadImage.cpp | 31 +++-- .../tests/test_executable_segments.py | 107 +++++++++++------- docs/dev/debt/relinker.md | 2 +- 4 files changed, 114 insertions(+), 58 deletions(-) diff --git a/core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp b/core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp index 59b76fde32..2d14881751 100644 --- a/core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp +++ b/core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp @@ -143,15 +143,31 @@ void SeparateSharedLoadPages(std::vector& buf, std::vector current.MappedAddress + current.MemorySize) throw Domain::RelinkerException("Overlapping PT_LOAD memory ranges", current.MappedAddress); - for (auto address = current.MappedAddress; address < previousEnd; ++address) { - const auto fromPrevious = address - previous.MappedAddress < previous.FileSize - ? buf[static_cast(previous.Offset + (address - previous.MappedAddress))] - : std::uint8_t{0}; - const auto fromCurrent = address - current.MappedAddress < current.FileSize - ? buf[static_cast(current.Offset + (address - current.MappedAddress))] - : std::uint8_t{0}; - if (fromPrevious != fromCurrent) + if (((previous.Flags | current.Flags) & (PF_W | PF_X)) == (PF_W | PF_X) + && current.MappedAddress % kDefaultLoadAlignment != 0) + throw Domain::RelinkerException("Overlapping PT_LOAD memory ranges", current.MappedAddress); + const auto previousFileEnd = previous.MappedAddress + previous.FileSize; + const auto currentFileEnd = current.MappedAddress + current.FileSize; + const auto compareEnd = std::min(previousEnd, std::max(previousFileEnd, currentFileEnd)); + for (auto address = current.MappedAddress; address < compareEnd;) { + const bool previousBacked = address < previousFileEnd; + const bool currentBacked = address < currentFileEnd; + auto runEnd = compareEnd; + if (previousBacked) + runEnd = std::min(runEnd, previousFileEnd); + if (currentBacked) + runEnd = std::min(runEnd, currentFileEnd); + const auto length = static_cast(runEnd - address); + const auto previousAt = static_cast(previous.Offset + (address - previous.MappedAddress)); + const auto currentAt = static_cast(current.Offset + (address - current.MappedAddress)); + const bool congruent = previousBacked && currentBacked + ? std::equal(buf.begin() + previousAt, buf.begin() + previousAt + length, buf.begin() + currentAt) + : previousBacked + ? std::all_of(buf.begin() + previousAt, buf.begin() + previousAt + length, [](const std::uint8_t byte) { return byte == 0; }) + : std::all_of(buf.begin() + currentAt, buf.begin() + currentAt + length, [](const std::uint8_t byte) { return byte == 0; }); + if (!congruent) throw Domain::RelinkerException("Overlapping PT_LOAD memory ranges", current.MappedAddress); + address = runEnd; } previous.MemorySize = current.MappedAddress - previous.MappedAddress; if (previous.FileSize > previous.MemorySize) diff --git a/core/relinker/elfpatcher/src/windows/WindowsLoadImage.cpp b/core/relinker/elfpatcher/src/windows/WindowsLoadImage.cpp index 07e3bab7ec..d3298f237a 100644 --- a/core/relinker/elfpatcher/src/windows/WindowsLoadImage.cpp +++ b/core/relinker/elfpatcher/src/windows/WindowsLoadImage.cpp @@ -36,15 +36,30 @@ WindowsLoadImage::WindowsLoadImage(const std::vector& source, cons const auto previousEnd = previous.MappedAddress + previous.MemorySize; if (segment.MappedAddress == previous.MappedAddress || previousEnd > segment.MappedAddress + segment.MemorySize) throw Domain::RelinkerException("Overlapping PT_LOAD memory ranges", segment.MappedAddress); - for (auto address = segment.MappedAddress; address < previousEnd; ++address) { - const auto fromPrevious = address - previous.MappedAddress < previous.FileSize - ? source[static_cast(previous.Offset + (address - previous.MappedAddress))] - : std::uint8_t{0}; - const auto fromCurrent = address - segment.MappedAddress < segment.FileSize - ? source[static_cast(segment.Offset + (address - segment.MappedAddress))] - : std::uint8_t{0}; - if (fromPrevious != fromCurrent) + if (((previous.Flags | segment.Flags) & 3u) == 3u && segment.MappedAddress % SectionAlignment != 0) + throw Domain::RelinkerException("Overlapping PT_LOAD memory ranges", segment.MappedAddress); + const auto previousFileEnd = previous.MappedAddress + previous.FileSize; + const auto segmentFileEnd = segment.MappedAddress + segment.FileSize; + const auto compareEnd = std::min(previousEnd, std::max(previousFileEnd, segmentFileEnd)); + for (auto address = segment.MappedAddress; address < compareEnd;) { + const bool previousBacked = address < previousFileEnd; + const bool segmentBacked = address < segmentFileEnd; + auto runEnd = compareEnd; + if (previousBacked) + runEnd = std::min(runEnd, previousFileEnd); + if (segmentBacked) + runEnd = std::min(runEnd, segmentFileEnd); + const auto length = static_cast(runEnd - address); + const auto previousAt = static_cast(previous.Offset + (address - previous.MappedAddress)); + const auto segmentAt = static_cast(segment.Offset + (address - segment.MappedAddress)); + const bool congruent = previousBacked && segmentBacked + ? std::equal(source.begin() + previousAt, source.begin() + previousAt + length, source.begin() + segmentAt) + : previousBacked + ? std::all_of(source.begin() + previousAt, source.begin() + previousAt + length, [](const std::uint8_t byte) { return byte == 0; }) + : std::all_of(source.begin() + segmentAt, source.begin() + segmentAt + length, [](const std::uint8_t byte) { return byte == 0; }); + if (!congruent) throw Domain::RelinkerException("Overlapping PT_LOAD memory ranges", segment.MappedAddress); + address = runEnd; } previous.MemorySize = segment.MappedAddress - previous.MappedAddress; if (previous.FileSize > previous.MemorySize) diff --git a/core/relinker/relinker/tests/test_executable_segments.py b/core/relinker/relinker/tests/test_executable_segments.py index 7e6ea8af2c..b33880a526 100644 --- a/core/relinker/relinker/tests/test_executable_segments.py +++ b/core/relinker/relinker/tests/test_executable_segments.py @@ -133,51 +133,76 @@ def main(): assert result.returncode == 2 and "Overlapping PT_LOAD memory ranges" in result.stderr, (name, options, result) assert not output.exists(), (name, options) - congruent = fixture(b"\x90\xc3") - struct.pack_into("" in its code, and in SuperTuxKart (PPSA69421) and Mortalfin (PPSA99141) the `jae` read from "sf" points into the VEX prefix of a 256-bit store. A direct branch target inside a scanned instruction counts only where every byte before it is a legacy prefix or REX, which a real branch can skip (a `lock` prefix, for example); a target in a VEX prefix, opcode or operand is ignored, as a jump-table entry that starts no instruction already is. A real branch to such a target, overlapping instructions as hand-written code can, is not seen, and the instruction it enters can be moved. - A `PT_SCE_RELRO` segment that no `PT_LOAD` covers, the PS4 layout, is [loaded](../../../core/relinker/relinker/src/parsing/ElfReader.cpp) as a read-write `PT_LOAD` and stays writable after relocation; FreeBSD rtld makes `PT_GNU_RELRO` read-only once relocation is done, and what the console does with `PT_SCE_RELRO` is not checked. -- A read-only `PT_LOAD` whose tail runs into a later `PT_LOAD` with byte-identical content (a read-only segment over the `PT_GNU_RELRO` start of a following read-write one, shared by Photo Splat VR PPSA99770 #12183, FoxHollow PPSA99573 and WWE 13 PPSA92013) is accepted by trimming the earlier segment to the later one's start, so the later segment owns the shared range ([Linux](../../../core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp), [Windows](../../../core/relinker/elfpatcher/src/windows/WindowsLoadImage.cpp)). On Linux the preserved `PT_GNU_RELRO` becomes read-only after relocation under ld.so; the Windows builder has no RELRO step, so the range stays writable, as for an uncovered `PT_SCE_RELRO` above. Differing bytes (#10564) or an overlap past the later segment are still refused. +- A read-only `PT_LOAD` whose tail runs into a later `PT_LOAD` with byte-identical content (a read-only segment over the `PT_GNU_RELRO` start of a following read-write one, shared by PPSA99770 #12183, PPSA99573 and PPSA92013) is accepted by trimming the earlier segment to the later one's start, so the later segment owns the shared range ([Linux](../../../core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp), [Windows](../../../core/relinker/elfpatcher/src/windows/WindowsLoadImage.cpp)). On Linux the preserved `PT_GNU_RELRO` becomes read-only after relocation under ld.so; the Windows builder has no RELRO step, so the range stays writable, as for an uncovered `PT_SCE_RELRO` above. Differing bytes (#10564), an overlap past the later segment, a shared start address, and an executable-and-writable flag combination at a boundary that is not page-aligned (which would share a read-write-execute page in the Windows image) are still refused. - In Windows output, a guest instruction that addresses memory through GS becomes `UD2` and NOP padding of its own length ([TLS builder](../../../core/relinker/elfpatcher/src/windows/WindowsTlsBuilder.cpp)): the host owns GS, and the guest GS base that FreeBSD's `sysarch(AMD64_SET_GSBASE)` would set is not emulated. A converted module runs until such an instruction is reached, which then faults. Prospero Win (PPSA99995) has six in its Wine `ntdll`. - In executables with a `PT_SCE_DYNLIBDATA` segment, the PS4 layout, the executable segment also holds read-only data after the code (CUSA00900: 37 MiB after its last function). The [relinker pipeline](../../../core/relinker/relinker/src/pipeline/RelinkerPipeline.cpp) therefore scans for syscalls and import call sites only up to the end of the PLT that follows the last function of the unwind table; code after the PLT without an unwind entry is not scanned. `--to-intel` and the `unused-filter=1` and `unused-filter=2` analyses are not limited this way. - The [syscall scan of `sce_module` modules](../../../core/relinker/relinker/src/guest/GuestModuleBuilder.cpp) with a `PT_SCE_DYNLIBDATA` segment stops at the same end, or, when the PLT comes before the code, at the end of the last function of the unwind table; code after it is not scanned.