From 1541244b784734db508b390681072729e5c5de21 Mon Sep 17 00:00:00 2001 From: fablevoid <340827264+fable-void@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:06:21 +0000 Subject: [PATCH] fix(relinker): treat jump table targets as branch targets when moving instructions The AMD-only converter moves the instructions next to a short AMD-only instruction into its stub, and refuses to move one that a branch enters. It knew only direct jump and call targets, so a switch case entered through a jump table could land inside the jump written over the moved instructions. The converter now also reads relative jump tables: a RIP-relative LEA whose address lies in a loaded segment starts a table of 32-bit offsets from that address, read while each target starts an instruction of the segment being converted. To read them, Convert takes the program headers and converts their executable load segments; the relinker passes all headers, and ElfReader::ReadCodeSegments, now unused, is removed. The converter test builds a dispatch through such a table whose second case enters the instruction after a 4-byte 256-bit store, and checks that the store takes the instruction before it instead. It also checks that reading stops at an entry that does not start an instruction, that only a LEA forms a table address, that tables are read only inside loaded segments, that only executable load segments are converted, and that a segment past the end of the file fails the conversion. PPSA12544: one 256-bit store in Il2cppUserAssemblies moved the instruction after it, which is case 1 of a switch; it now goes through an island. The other relinked files are unchanged. AI-assisted: yes --- .../include/codegen/IAmd64OnlyConverter.hpp | 2 +- .../codegen/src/Amd64OnlyConverter.cpp | 60 +++++++++++++++---- .../codegen/tests/Amd64OnlyConverterTests.cpp | 38 ++++++++++++ core/relinker/main.cpp | 4 +- .../include/relinker/domain/IElfReader.hpp | 1 - .../include/relinker/parsing/ElfReader.hpp | 1 - .../relinker/src/guest/GuestModuleBuilder.cpp | 2 +- .../relinker/src/parsing/ElfReader.cpp | 12 ---- 8 files changed, 90 insertions(+), 30 deletions(-) diff --git a/core/relinker/codegen/include/codegen/IAmd64OnlyConverter.hpp b/core/relinker/codegen/include/codegen/IAmd64OnlyConverter.hpp index ca82119d7..e133c533f 100644 --- a/core/relinker/codegen/include/codegen/IAmd64OnlyConverter.hpp +++ b/core/relinker/codegen/include/codegen/IAmd64OnlyConverter.hpp @@ -14,7 +14,7 @@ public: [[nodiscard]] virtual ConvertResult Convert( std::vector fileBytes, - const std::vector& codeSegments + const std::vector& segments ) const = 0; }; diff --git a/core/relinker/codegen/src/Amd64OnlyConverter.cpp b/core/relinker/codegen/src/Amd64OnlyConverter.cpp index 2f57bd9fd..6990cf40f 100644 --- a/core/relinker/codegen/src/Amd64OnlyConverter.cpp +++ b/core/relinker/codegen/src/Amd64OnlyConverter.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -29,6 +30,9 @@ auto _atFileOffset(const Domain::FileByteOffset base, const TOperation& operatio } constexpr std::int64_t kShortJumpSize = 2; +constexpr std::uint32_t kLoadSegment = 1; +constexpr std::uint32_t kExecutableSegment = 1; +constexpr std::uint8_t kLea = 0x8D; constexpr std::uint64_t kIslandSearchBytes = 0x100; struct Placement { @@ -209,7 +213,7 @@ public: [[nodiscard]] ConvertResult Convert( std::vector fileBytes, - const std::vector& codeSegments + const std::vector& segments ) const override; private: @@ -225,30 +229,59 @@ private: void _convertSegment( std::vector& fileBytes, const Domain::ProgramHeader& ph, + const std::vector& segments, ConvertResult& result ) const; [[nodiscard]] static std::set _collectBranchTargets( const std::vector& seg, const std::vector& matches, - const Domain::ProgramHeader& ph + const Domain::ProgramHeader& ph, + const std::vector& fileBytes, + const std::vector& segments ); }; std::set Amd64OnlyConverter::_collectBranchTargets( const std::vector& seg, const std::vector& matches, - const Domain::ProgramHeader& ph + const Domain::ProgramHeader& ph, + const std::vector& fileBytes, + const std::vector& segments ) { + const auto startsInstruction = [&](const std::uint64_t address) { + const auto offset = address - ph.MappedAddress; + const auto match = std::lower_bound(matches.begin(), matches.end(), offset, [](const InstructionMatch& item, const std::uint64_t value) { return item.Offset < value; }); + return match != matches.end() && match->Offset == offset; + }; const X64InstructionDecoder decoder; std::set targets; for (const auto& match : matches) { const auto info = decoder.DecodeInstruction(seg.data() + match.Offset, match.Length); - if (!info.HasBranchTarget || info.HasRipRelativeDisp) - continue; - const auto target = static_cast(ph.MappedAddress + match.Offset + info.Length) + info.BranchDisp; - if (target >= 0) - targets.insert(static_cast(target)); + const auto next = static_cast(ph.MappedAddress + match.Offset + info.Length); + if (info.HasBranchTarget && !info.HasRipRelativeDisp) { + const auto target = next + info.BranchDisp; + if (target >= 0) + targets.insert(static_cast(target)); + } else if (info.HasRipRelativeDisp && info.Opcode == kLea) { + std::int32_t displacement; + std::memcpy(&displacement, seg.data() + match.Offset + info.RipRelativeDispOffset, sizeof(displacement)); + const auto table = static_cast(next + displacement); + for (const auto& data : segments) { + if (data.Type != kLoadSegment || table < data.MappedAddress || table - data.MappedAddress >= data.FileSize) + continue; + if (data.Offset > fileBytes.size() || data.FileSize > fileBytes.size() - data.Offset) + throw CodegenException("Segment exceeds the file", data.Offset); + for (auto entry = table; entry - data.MappedAddress + sizeof(std::int32_t) <= data.FileSize; entry += sizeof(std::int32_t)) { + std::int32_t relative; + std::memcpy(&relative, fileBytes.data() + data.Offset + (entry - data.MappedAddress), sizeof(relative)); + const auto target = table + static_cast(static_cast(relative)); + if (!startsInstruction(target)) + break; + targets.insert(target); + } + } + } } return targets; } @@ -256,6 +289,7 @@ std::set Amd64OnlyConverter::_collectBranchTargets( void Amd64OnlyConverter::_convertSegment( std::vector& fileBytes, const Domain::ProgramHeader& ph, + const std::vector& segments, ConvertResult& result ) const { const auto segOffset = static_cast(ph.Offset); @@ -283,7 +317,7 @@ void Amd64OnlyConverter::_convertSegment( const bool needsBranchTargets = std::any_of(pending.begin(), pending.end(), [](const Pending& item) { return item.Substitution.Lowering == Amd64OnlyLowering::Trampoline; }); - const auto branchTargets = needsBranchTargets ? _collectBranchTargets(seg, matches, ph) : std::set{}; + const auto branchTargets = needsBranchTargets ? _collectBranchTargets(seg, matches, ph, fileBytes, segments) : std::set{}; std::set pendingIndices; for (const auto& item : pending) @@ -395,11 +429,13 @@ void Amd64OnlyConverter::_convertSegment( ConvertResult Amd64OnlyConverter::Convert( std::vector fileBytes, - const std::vector& codeSegments + const std::vector& segments ) const { ConvertResult result{}; - for (const auto& ph : codeSegments) - _convertSegment(fileBytes, ph, result); + for (const auto& ph : segments) { + if (ph.Type == kLoadSegment && (ph.Flags & kExecutableSegment) != 0) + _convertSegment(fileBytes, ph, segments, result); + } result.Bytes = std::move(fileBytes); return result; } diff --git a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp index 16e891044..8c4e4e009 100644 --- a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp +++ b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp @@ -886,6 +886,43 @@ void converterWideStoreWindows() { requireFailure([&] { (void)convert(withStore(far, tail)); }, "A 256-bit store whose movable neighbours are all beyond a short jump was kept"); } +void converterJumpTableTargets() { + const auto converter = Codegen::MakeAmd64OnlyConverter(Codegen::Amd64OnlyTarget::Rosetta); + const Bytes leaTable = {0x48, 0x8D, 0x0D, 0xF9, 0x1F, 0x00, 0x00}; + const Bytes dispatch = {0x48, 0x63, 0x04, 0x81, 0x48, 0x01, 0xC8, 0xFF, 0xE0, 0x48, 0x83, 0xC1, 0x01, 0xC5, 0xFC, 0x11, 0x07, 0x48, 0x83, 0xC2, 0x01, 0xC3}; + const Bytes store = {0xC5, 0xFC, 0x11, 0x07}; + const Domain::ProgramHeader code{1, 5, 0x200, 0x1000, 0, 29, 29, 16}; + const Domain::ProgramHeader data{1, 4, 0x300, 0x3000, 0, 0x40, 0x40, 16}; + const auto table = [](const std::uint64_t base, const std::vector& targets) { + Bytes bytes; + for (const auto target : targets) { + const auto relative = static_cast(target - base); + for (std::size_t byte = 0; byte < 4; ++byte) + bytes.push_back(static_cast(relative >> (byte * 8))); + } + return bytes; + }; + const auto convert = [&](const Bytes& first, const std::vector>& placed, const std::vector& headers) { + Bytes file(0x400, 0xCC); + std::copy(first.begin(), first.end(), file.begin() + 0x200); + std::copy(dispatch.begin(), dispatch.end(), file.begin() + 0x207); + for (const auto& [offset, bytes] : placed) + std::copy(bytes.begin(), bytes.end(), file.begin() + static_cast(offset)); + return converter->Convert(file, headers); + }; + const auto placedAt = [](const Codegen::ConvertResult& result, const Domain::FileByteOffset offset) { + return result.Trampolines.size() == 1 && result.Trampolines[0].Offset == offset && result.Trampolines[0].Length == 8; + }; + + require(placedAt(convert(leaTable, {{0x300, table(0x3000, {0x1010, 0x1018})}, {0x330, store}}, {code, data}), 0x210), "A 256-bit store moved the instruction after it although a jump table enters that instruction"); + require(placedAt(convert(leaTable, {{0x300, table(0x3000, {0x1010, 0x1019, 0x1018})}}, {code, data}), 0x214), "A jump table was read past an entry that does not start an instruction"); + require(placedAt(convert({0x48, 0x8B, 0x0D, 0xF9, 0x1F, 0x00, 0x00}, {{0x300, table(0x3000, {0x1010, 0x1018})}}, {code, data}), 0x214), "A jump table was read at an address loaded rather than formed by LEA"); + require(placedAt(convert({0x48, 0x8D, 0x0D, 0xF7, 0x1F, 0x00, 0x00}, {{0x2FE, table(0x2FFE, {0x1018})}}, {code, data}), 0x214), "A jump table was read at an address outside every segment"); + require(placedAt(convert(leaTable, {{0x300, table(0x3000, {0x1010})}, {0x380, table(0x3000, {0x1018})}}, {code, data, {0x61000000, 4, 0x380, 0x3000, 0, 0x10, 0x10, 16}}), 0x214), "A jump table was read from a segment that is not loaded"); + require(placedAt(convert(leaTable, {{0x300, table(0x3000, {0x1010, 0x1018})}, {0x3A0, store}}, {code, data, {2, 5, 0x3A0, 0x2000, 0, 4, 4, 16}}), 0x210), "A segment that is not loaded was converted"); + requireFailure([&] { (void)convert(leaTable, {{0x300, table(0x3000, {0x1010, 0x1018})}}, {code, {1, 4, 0x300, 0x3000, 0, 0x1000, 0x1000, 16}}); }, "A jump table was read from a segment that exceeds the file"); +} + void converterRosetta() { const auto converter = Codegen::MakeAmd64OnlyConverter(Codegen::Amd64OnlyTarget::Rosetta); Bytes file(0x300, 0xCC); @@ -1637,6 +1674,7 @@ int main() { converterRosetta(); wideStoreSplit(); converterWideStoreWindows(); + converterJumpTableTargets(); converterReciprocal(); converterStrayRex(); rewriterStrayRex(); diff --git a/core/relinker/main.cpp b/core/relinker/main.cpp index 1734ede1b..24151c4e3 100644 --- a/core/relinker/main.cpp +++ b/core/relinker/main.cpp @@ -66,8 +66,8 @@ int Run(const int argc, char* argv[]) { std::vector trampolines; if (args.toIntel) { - const auto codeSegments = Relinker::ElfReader(sourceBytes).ReadCodeSegments(); - auto converted = Codegen::MakeAmd64OnlyConverter(args.toRosetta ? Codegen::Amd64OnlyTarget::Rosetta : Codegen::Amd64OnlyTarget::Intel)->Convert(std::move(sourceBytes), codeSegments); + const auto segments = Relinker::ElfReader(sourceBytes).ReadProgramHeaders(); + auto converted = Codegen::MakeAmd64OnlyConverter(args.toRosetta ? Codegen::Amd64OnlyTarget::Rosetta : Codegen::Amd64OnlyTarget::Intel)->Convert(std::move(sourceBytes), segments); sourceBytes = std::move(converted.Bytes); trampolines = std::move(converted.Trampolines); std::map stubsByName; diff --git a/core/relinker/relinker/include/relinker/domain/IElfReader.hpp b/core/relinker/relinker/include/relinker/domain/IElfReader.hpp index 90ce34de3..f528b3945 100644 --- a/core/relinker/relinker/include/relinker/domain/IElfReader.hpp +++ b/core/relinker/relinker/include/relinker/domain/IElfReader.hpp @@ -18,7 +18,6 @@ public: [[nodiscard]] virtual FileByteOffset TranslateVirtualAddress(VirtualAddress address) const = 0; [[nodiscard]] virtual std::vector ReadSection(const SectionHeader& header) const = 0; [[nodiscard]] virtual std::vector ReadSegment(const ProgramHeader& header) const = 0; - [[nodiscard]] virtual std::vector ReadCodeSegments() const = 0; [[nodiscard]] virtual std::uint64_t GetFileSize() const = 0; [[nodiscard]] virtual const std::vector& GetRawBytes() const = 0; }; diff --git a/core/relinker/relinker/include/relinker/parsing/ElfReader.hpp b/core/relinker/relinker/include/relinker/parsing/ElfReader.hpp index 1ed4ca889..b0cb4d0f7 100644 --- a/core/relinker/relinker/include/relinker/parsing/ElfReader.hpp +++ b/core/relinker/relinker/include/relinker/parsing/ElfReader.hpp @@ -17,7 +17,6 @@ public: FileByteOffset TranslateVirtualAddress(VirtualAddress address) const override; std::vector ReadSection(const SectionHeader& header) const override; std::vector ReadSegment(const ProgramHeader& header) const override; - std::vector ReadCodeSegments() const override; std::uint64_t GetFileSize() const override; const std::vector& GetRawBytes() const override; diff --git a/core/relinker/relinker/src/guest/GuestModuleBuilder.cpp b/core/relinker/relinker/src/guest/GuestModuleBuilder.cpp index 08aec4772..b12b66331 100644 --- a/core/relinker/relinker/src/guest/GuestModuleBuilder.cpp +++ b/core/relinker/relinker/src/guest/GuestModuleBuilder.cpp @@ -342,7 +342,7 @@ std::vector GuestModuleBuilder::Build(const std::filesystem::path std::vector codeHeaders; for (const auto& header : image.Headers) if (header.Type == 1 && (header.Flags & 1) != 0) codeHeaders.push_back(header); if (toIntel) { - auto converted = Codegen::MakeAmd64OnlyConverter(toRosetta ? Codegen::Amd64OnlyTarget::Rosetta : Codegen::Amd64OnlyTarget::Intel)->Convert(std::move(image.Bytes), codeHeaders); + auto converted = Codegen::MakeAmd64OnlyConverter(toRosetta ? Codegen::Amd64OnlyTarget::Rosetta : Codegen::Amd64OnlyTarget::Intel)->Convert(std::move(image.Bytes), image.Headers); image.Trampolines = std::move(converted.Trampolines); image.Bytes = std::move(converted.Bytes); } diff --git a/core/relinker/relinker/src/parsing/ElfReader.cpp b/core/relinker/relinker/src/parsing/ElfReader.cpp index c6af1ec10..7edb52b46 100644 --- a/core/relinker/relinker/src/parsing/ElfReader.cpp +++ b/core/relinker/relinker/src/parsing/ElfReader.cpp @@ -261,18 +261,6 @@ std::vector ElfReader::ReadSegment(const ProgramHeader& header) co _fileBuffer.begin() + header.Offset + header.FileSize); } -std::vector ElfReader::ReadCodeSegments() const { - static constexpr std::uint32_t kPtLoad = 1; - static constexpr std::uint32_t kPfX = 0x1; - std::vector result; - for (const auto& ph : ReadProgramHeaders()) { - if (ph.Type == kPtLoad && (ph.Flags & kPfX)) { - result.push_back(ph); - } - } - return result; -} - std::uint64_t ElfReader::GetFileSize() const { return _fileBuffer.size(); }