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 <[email protected]>

core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp

@@ -143,15 +143,31 @@ void SeparateSharedLoadPages(std::vector<std::uint8_t>& buf, std::vector<Domain:
         const auto previousEnd = previous.MappedAddress + previous.MemorySize;
         if (current.MappedAddress == previous.MappedAddress || previousEnd > 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<std::size_t>(previous.Offset + (address - previous.MappedAddress))]
-                : std::uint8_t{0};
-            const auto fromCurrent = address - current.MappedAddress < current.FileSize
-                ? buf[static_cast<std::size_t>(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<std::size_t>(runEnd - address);
+            const auto previousAt = static_cast<std::size_t>(previous.Offset + (address - previous.MappedAddress));
+            const auto currentAt = static_cast<std::size_t>(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)

core/relinker/elfpatcher/src/windows/WindowsLoadImage.cpp

@@ -36,15 +36,30 @@ WindowsLoadImage::WindowsLoadImage(const std::vector<std::uint8_t>& 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<std::size_t>(previous.Offset + (address - previous.MappedAddress))]
-                : std::uint8_t{0};
-            const auto fromCurrent = address - segment.MappedAddress < segment.FileSize
-                ? source[static_cast<std::size_t>(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<std::size_t>(runEnd - address);
+            const auto previousAt = static_cast<std::size_t>(previous.Offset + (address - previous.MappedAddress));
+            const auto segmentAt = static_cast<std::size_t>(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)

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("<Q", congruent, 176 + 32, 0x602)
-        struct.pack_into("<Q", congruent, 176 + 40, 0x602)
-        source = work / "congruent-overlap.elf"
-        source.write_bytes(congruent)
-        linux_output = work / "congruent-overlap.elf.out"
-        result = subprocess.run([str(relinker), "--skip-sce-module", "--skip-syscall-check",
-                                 str(source), str(linux_output)], capture_output=True, text=True, timeout=20)
-        assert result.returncode == 0 and linux_output.exists(), (result.stdout, result.stderr)
-        produced = linux_output.read_bytes()
-        phoff = struct.unpack_from("<Q", produced, 32)[0]
-        phnum = struct.unpack_from("<H", produced, 56)[0]
-        spans = []
-        for index in range(phnum):
-            kind, _, _, address, _, _, memory, _ = struct.unpack_from("<IIQQQQQQ", produced, phoff + index * 56)
-            if kind == 1 and memory:
-                spans.append((address, memory))
-        spans.sort()
-        assert all(a[0] + a[1] <= b[0] for a, b in zip(spans, spans[1:])), spans
-        for options in image_targets:
-            target_output = work / "congruent-overlap.out"
-            if target_output.exists():
-                target_output.unlink()
-            result = subprocess.run([str(relinker), "--skip-sce-module", "--skip-syscall-check",
-                                     *options, str(source), str(target_output)],
-                                    capture_output=True, text=True, timeout=20)
-            assert result.returncode == 0 and target_output.exists(), (options, result.stdout, result.stderr)
+        phdr3 = {"flags": (4, "I"), "offset": (8, "Q"), "address": (16, "Q"),
+                 "paddr": (24, "Q"), "filesz": (32, "Q"), "memsz": (40, "Q")}
 
-        same_start = fixture(b"\x90\xc3")
-        struct.pack_into("<Q", same_start, 232 + 8, 0x200)
-        struct.pack_into("<Q", same_start, 232 + 16, 0x1200)
-        struct.pack_into("<Q", same_start, 232 + 24, 0x1200)
-        struct.pack_into("<Q", same_start, 232 + 32, 0x40)
-        struct.pack_into("<Q", same_start, 232 + 40, 0x40)
-        source = work / "same-start-overlap.elf"
-        source.write_bytes(same_start)
-        for options in image_targets:
-            output = work / "same-start-overlap.out"
+        def overlap_image(previous_file, previous_memory, extra=b"", **later):
+            image = fixture(b"\x90\xc3")
+            image += bytes(extra)
+            struct.pack_into("<Q", image, 176 + 32, previous_file)
+            struct.pack_into("<Q", image, 176 + 40, previous_memory)
+            for field, value in later.items():
+                offset, code = phdr3[field]
+                struct.pack_into("<" + code, image, 232 + offset, value)
+            return image
+
+        def load_segments(data):
+            phoff = struct.unpack_from("<Q", data, 32)[0]
+            phnum = struct.unpack_from("<H", data, 56)[0]
+            segments = []
+            for index in range(phnum):
+                kind, flags, _, address, _, _, memory, _ = struct.unpack_from("<IIQQQQQQ", data, phoff + index * 56)
+                if kind == 1 and memory:
+                    segments.append((address, memory, flags))
+            return sorted(segments)
+
+        def relink(image, options, name):
+            source = work / (name + ".elf")
+            source.write_bytes(bytes(image))
+            output = work / (name + ".out")
             if output.exists():
                 output.unlink()
             result = subprocess.run([str(relinker), "--skip-sce-module", "--skip-syscall-check",
-                                     *options, str(source), str(output)],
-                                    capture_output=True, text=True, timeout=20)
-            assert result.returncode == 2 and "Overlapping PT_LOAD memory ranges" in result.stderr \
-                and not output.exists(), (options, result.stdout, result.stderr)
+                                     *options, str(source), str(output)], capture_output=True, text=True, timeout=20)
+            return result, output
+
+        for name, image in (("congruent-tail-overlap", overlap_image(0x602, 0x602)),
+                            ("congruent-bss-overlap", overlap_image(0x20, 0x602, filesz=0, memsz=2))):
+            for options in image_targets:
+                result, output = relink(image, options, name)
+                assert result.returncode == 0 and output.exists(), (name, options, result.stdout, result.stderr)
+            result, linux_output = relink(image, [], name + "-linux")
+            segments = load_segments(linux_output.read_bytes())
+            assert all(a[0] + a[1] <= b[0] for a, b in zip(segments, segments[1:])), (name, segments)
+            assert any(memory == 0x600 for _, memory, _ in segments), (name, segments)
+
+        for name, image in (("overlap-differing-bytes", overlap_image(0x602, 0x602, offset=0x810)),
+                            ("overlap-past-later-end", overlap_image(0x700, 0x700)),
+                            ("overlap-nonzero-over-bss", overlap_image(0x602, 0x602, filesz=0, memsz=2)),
+                            ("overlap-same-start", overlap_image(0x20, 0x20, address=0x1200, paddr=0x1200, offset=0x200, filesz=0x40, memsz=0x40)),
+                            ("overlap-write-execute-unaligned", overlap_image(0x602, 0x602, flags=6))):
+            for options in image_targets:
+                result, output = relink(image, options, name)
+                assert result.returncode == 2 and "Overlapping PT_LOAD memory ranges" in result.stderr \
+                    and not output.exists(), (name, options, result.stdout, result.stderr)
+
+        aligned = overlap_image(0xe02, 0xe02, extra=bytes(0x2000),
+                                flags=6, offset=0x1000, address=0x2000, paddr=0x2000, filesz=2, memsz=2)
+        for options in image_targets:
+            result, output = relink(aligned, options, "overlap-write-execute-aligned")
+            assert result.returncode == 0 and output.exists(), (options, result.stdout, result.stderr)
+        result, windows = relink(aligned, ["--windows"], "overlap-write-execute-aligned-windows")
+        data = windows.read_bytes()
+        lfanew = struct.unpack_from("<I", data, 0x3c)[0]
+        table = lfanew + 24 + struct.unpack_from("<H", data, lfanew + 20)[0]
+        for index in range(struct.unpack_from("<H", data, lfanew + 6)[0]):
+            characteristics = struct.unpack_from("<I", data, table + index * 40 + 36)[0]
+            assert not (characteristics & 0x80000000 and characteristics & 0x20000000), (index, hex(characteristics))
+
+        result, output = relink(fixture(b"\x90\xc3"), [], "no-overlap-control")
+        assert result.returncode == 0 and output.exists(), (result.stdout, result.stderr)
+        control = load_segments(output.read_bytes())
+        assert any(memory == 0x20 for _, memory, _ in control) and any(memory == 2 for _, memory, _ in control), control
     print("Executable segment tests passed")
 
 

docs/dev/debt/relinker.md

@@ -30,7 +30,7 @@
 - The length-changing path of the [instruction rewriter](../../../core/relinker/codegen/src/x86/X64InstructionRewriter.cpp) is not used by the [converter](../../../core/relinker/codegen/src/Amd64OnlyConverter.cpp): it does not adjust data-to-code references (relocations, FDEs, jump tables) or segment sizes, so every substitution keeps the instruction length. The rewriter conservatively rejects length-changing edits when a surviving truncated relative branch would move relative to its target (only one of the two lies after the cut) or when its section-relative target falls outside the 16-bit range, because the runtime address base needed to resolve a wrapped destination is unavailable; truncated branches that keep their distance and stay in range are accepted.
 - The [converter](../../../core/relinker/codegen/src/Amd64OnlyConverter.cpp) takes the branch targets that keep an instruction in place from a linear scan of the code segment, so data in that segment decodes as branches too: OpenSSL's x86-64 assembly keeps strings such as "SHA1 block transform for x86_64, CRYPTOGAMS by <appro@openssl.org>" 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.