Repository navigation
fix(relinker): require the Linux entry, relocation offsets and DT_PLTGOT in PT_LOAD memory - #10402
roboterspinne wants to merge 1 commit into
Conversation
…GOT in PT_LOAD memory The Windows and macOS patchers reject an entry point outside executable PT_LOAD memory and a relocation offset or DT_PLTGOT outside PT_LOAD memory through WindowsLoadImage::GetRva. The Linux patcher copied these values unchecked and reported success; with a bad entry or relocation offset the output crashed with SIGSEGV at load. LinuxElfPatcher::Patch now requires e_entry in an executable load and every non-NONE RELA and JMPREL r_offset and DT_PLTGOT to fit inside one PT_LOAD's MemorySize, in the fixed-base and --lazy-binding paths. A relocation needs the bytes ld.so writes: 4 for PC32 and 32, the output symbol's st_size (at least 1) for COPY, and 8 otherwise. DT_PLTGOT needs 8 bytes with eager binding and 24 with --lazy-binding, where ld.so writes GOT[1] and GOT[2]. Two test fixtures that set e_entry to a file offset outside their only PT_LOAD now use the matching virtual address.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the Linux PT_LOAD validation and the new synthetic test matrix. The checks are applied before fixed-base and lazy-binding output, skip R_X86_64_NONE, account for PC32/32 and COPY widths, and include BSS/RELRO cases. The existing tests were corrected from file offsets to their mapped virtual entry addresses. Hosted relinker matrix and conventions pass; full product builds are skipped by repository policy. No blocking issue found.
MarcZX10
left a comment
There was a problem hiding this comment.
Blocking correctness issue in the new relocation-width check: RelocationWidth() returns 8 for R_X86_64_32S, but the x86-64 psABI specifies a 32-bit relocation field for that type. This repository accepts R_X86_64_32S in RebasedRelocations(), so a valid 32S relocation whose 4-byte field ends at the PT_LOAD boundary will be rejected as requiring 8 mapped bytes. Please classify R_X86_64_32S as width 4 and add a boundary fixture analogous to abs32-past-load-end that succeeds when the 4-byte field fits. The ABI’s relocation table lists R_X86_64_32S as word32: https://refspecs.linuxfoundation.org/elf/x86_64-SysV-psABI.pdf
Thanks for the close look. |
What
The Linux output did not check that
e_entry, relocation offsets and DT_PLTGOT lie in PT_LOAD memory: the relink returned 0 and the output crashed at load. Windows and macOS already reject such inputs throughWindowsLoadImage(WindowsLoadImage.cpp:57-70, used for relocation targets inWindowsRelocationBuilder.cpp:31and for DT_PLTGOT inWindowsPePatcher.cpp:43-44andMacOsMachOPatcher.cpp:65-66).LinuxElfPatcher::Patchnow checks, before both output paths (fixed base and--lazy-binding) and with the same rule asGetRva(the whole range inside one PT_LOAD'sMemorySize):e_entry: 1 byte in a PT_LOAD withPF_X.r_offsetexceptR_X86_64_NONE, over the bytes ld.so writes: 4 forPC32and32, the reference symbol'sst_size(at least 1) forCOPY, 8 otherwise.--lazy-binding, where glibc'self_machine_runtime_setupwrites GOT[1] and GOT[2].Messages name the field, for example
e_entry 0x7ffff000 is not inside an executable PT_LOAD segmentandRELA r_offset 0xffc (8 bytes) does not fit inside a PT_LOAD segment.ElfReaderturns aPT_SCE_RELROthat no PT_LOAD covers into a PT_LOAD beforePatch, so the usual PS5 GOT layout stays accepted. Windows, macOS, guest modules and the Linux output layout are unchanged.test_linux_load_alignment.pyandtest_linux_eh_frame_header.pysete_entryto 0x4000, a file offset that their only PT_LOAD maps at virtual address 0. They now use 0 and check the same things as before.Tested
Ubuntu 24.04 x86_64, glibc 2.39, relinker-only Release build (
-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON); synthetic fixtures only, no title or homebrew.linux_load_bounds(0.2 s), in the default mode and with--lazy-binding: 17 rejected cases (entry outside a load or in a non-executable one; RELA, JMPREL andCOPYoffsets outside a load or crossing its end; DT_PLTGOT outside a load; lazy DT_PLTGOT too close to its end) and 23 accepted ones (4-byte types andCOPYin the last bytes of a load, offsets in.bssand in an uncoveredPT_SCE_RELRO,R_X86_64_NONEoutside a load, eager DT_PLTGOT at 0xff0). WithLinuxElfPatcher.cppfrommain, all 17 rejected cases relink with rc 0.ctest --test-dir build -j2: 241/241 passed.main, thetest_linux_entry_argvfixture withe_entry = 0x7ffff000and thetest_optional_pltfixture with a RELATIVEr_offset = 0x7ffff000relink with rc 0 and the outputs exit with SIGSEGV. On this branch both fail with rc 2 and the messages above.LinuxElfPatcher.cppor these tests (fix(linux): enter the guest entry on a main stack at the console's address #7532, fix(relinker): reject unsupported extended program header counts #10341, refactor(relinker): remove relocation types validation already rejects #10357) apply on top of this branch withgit apply --3way --check.python3 tools/check_conventions.py --base upstream/mainwith this title and description passed.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)