From e66d78c626f1415af0b618d1254d3be563944fcc Mon Sep 17 00:00:00 2001 From: overlordxrz-source Date: Wed, 7 Oct 2026 23:37:31 +0000 Subject: [PATCH] fix(relinker): decode 0F A8-AA without a ModRM and MOV CR/DR without a memory operand PUSH GS, POP GS and RSM (0F A8, 0F A9, 0F AA) have no ModRM byte, but fell in the 0F A3-AB ModRM range, so the decoder read the next byte as a ModRM and sized its displacement. MOV to and from control and debug registers (0F 20-23) ignore the ModRM mod field and always name a register, but were sized and reported as memory forms, including a RIP-relative disp32 for mod 00 r/m 101. --- .../codegen/x86/X64OpcodeConstants.hpp | 3 +- .../codegen/src/x86/X64InstructionDecoder.cpp | 13 +++++---- .../codegen/tests/Amd64OnlyConverterTests.cpp | 28 +++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp b/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp index 022721a23..567fa5134 100644 --- a/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp +++ b/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp @@ -117,7 +117,7 @@ inline constexpr std::uint8_t TwoByteMovImm8ModRm = 0xBA; inline constexpr std::uint8_t TwoByteCmovRangeMin = 0x40; inline constexpr std::uint8_t TwoByteCmovRangeMax = 0x4F; inline constexpr std::uint8_t TwoByteModRmRangeAMin = 0xA3; -inline constexpr std::uint8_t TwoByteModRmRangeAMax = 0xAB; +inline constexpr std::uint8_t TwoByteModRmRangeAMax = 0xA7; inline constexpr std::uint8_t TwoByteModRmRangeBMin = 0xB0; inline constexpr std::uint8_t TwoByteModRmRangeBMax = 0xB7; inline constexpr std::uint8_t TwoByteModRmRangeCMin = 0xBC; @@ -172,6 +172,7 @@ inline constexpr std::uint8_t TwoByteMovCrDrMin = 0x20; inline constexpr std::uint8_t TwoByteMovCrDrMax = 0x23; inline constexpr std::uint8_t TwoBytePopcnt = 0xB8; inline constexpr std::uint8_t TwoByteUd1 = 0xB9; +inline constexpr std::uint8_t TwoByteBts = 0xAB; inline constexpr std::uint8_t TwoByteBtc = 0xBB; inline constexpr std::uint8_t TwoByteXaddRm8 = 0xC0; inline constexpr std::uint8_t TwoByteMovnti = 0xC3; diff --git a/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp b/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp index 065c7f9dd..0133844f0 100644 --- a/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp +++ b/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp @@ -237,6 +237,7 @@ std::size_t X64InstructionDecoder::Decode(const std::uint8_t* data, std::size_t hasModRm = true; } else if ((opcode >= TwoByteCmovRangeMin && opcode <= TwoByteCmovRangeMax) || (opcode >= TwoByteModRmRangeAMin && opcode <= TwoByteModRmRangeAMax) || + opcode == TwoByteBts || (opcode >= TwoByteModRmRangeBMin && opcode <= TwoByteModRmRangeBMax) || (opcode >= TwoByteModRmRangeCMin && opcode <= TwoByteModRmRangeCMax) || (opcode >= TwoByteModRmRangeDMin && opcode <= TwoByteModRmRangeDMax) || @@ -270,7 +271,8 @@ std::size_t X64InstructionDecoder::Decode(const std::uint8_t* data, std::size_t const std::uint8_t modrm = data[pos]; pos += 1; - const auto mod = static_cast((modrm >> ModRmModShift) & ModRmModMask); + const bool registerOnlyModRm = twoByteOpcode && !threeByteEscape && opcode >= TwoByteMovCrDrMin && opcode <= TwoByteMovCrDrMax; + const auto mod = registerOnlyModRm ? ModRmModRegister : static_cast((modrm >> ModRmModShift) & ModRmModMask); const auto reg = static_cast((modrm >> ModRmRegShift) & ModRmRegMask); const auto rm = static_cast(modrm & ModRmRmMask); @@ -352,12 +354,12 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction( if (pos >= info.Length) return info; - const auto readModRm = [&](const std::size_t modrmPos) { + const auto readModRm = [&](const std::size_t modrmPos, const bool registerOnly) { const std::uint8_t modrm = data[modrmPos]; info.HasModRm = true; info.ModRmByte = modrm; info.ModRmRegField = (modrm >> ModRmRegShift) & ModRmRegMask; - const std::uint8_t mod = (modrm >> ModRmModShift) & ModRmModMask; + const std::uint8_t mod = registerOnly ? ModRmModRegister : (modrm >> ModRmModShift) & ModRmModMask; const std::uint8_t rm = modrm & ModRmRmMask; if (mod == ModRmModIndirect && rm == ModRmRmRipRelative) { info.HasRipRelativeDisp = true; @@ -371,7 +373,7 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction( if (op == OneByteVex2 || op == OneByteVex3 || op == EvexPrefix) { pos += (op == OneByteVex2 ? 1 : (op == OneByteVex3 ? 2 : EvexPrefixLength - 1)) + 1; if (pos < info.Length) - readModRm(pos); + readModRm(pos, false); info.FlowKind = ControlFlowKind::Sequential; return info; } @@ -507,6 +509,7 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction( } else { if ((op >= TwoByteCmovRangeMin && op <= TwoByteCmovRangeMax) || (op >= TwoByteModRmRangeAMin && op <= TwoByteModRmRangeAMax) || + op == TwoByteBts || (op >= TwoByteModRmRangeBMin && op <= TwoByteModRmRangeBMax) || (op >= TwoByteModRmRangeCMin && op <= TwoByteModRmRangeCMax) || (op >= TwoByteModRmRangeDMin && op <= TwoByteModRmRangeDMax) || @@ -535,7 +538,7 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction( } if (hasModRm && pos < info.Length) - readModRm(pos); + readModRm(pos, twoByteOpcode && op >= TwoByteMovCrDrMin && op <= TwoByteMovCrDrMax); info.FlowKind = ControlFlowKind::Sequential; return info; diff --git a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp index c4fab89f4..d3a8d9881 100644 --- a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp +++ b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp @@ -162,6 +162,33 @@ void decoderRipRelative() { require(!decoder.DecodeInstruction(instruction.data(), instruction.size()).HasRipRelativeDisp, "Vector instruction without a RIP-relative operand was reported as RIP-relative"); } +void decoderTwoByteOpcodeLengths() { + const Codegen::X64InstructionDecoder decoder; + const std::vector instructions = { + {0x0F, 0xA8}, {0x0F, 0xA9}, {0x0F, 0xAA}, {0x66, 0x0F, 0xA8}, {0x41, 0x0F, 0xA9}, + {0x0F, 0xAB, 0xC8}, {0x0F, 0xAB, 0x05, 0x10, 0x00, 0x00, 0x00}, {0x48, 0x0F, 0xAB, 0x44, 0x24, 0x08}, + {0x0F, 0x20, 0xC0}, {0x0F, 0x20, 0x05}, {0x0F, 0x22, 0x04}, {0x0F, 0x21, 0x45}, {0x0F, 0x23, 0x85}, + {0x66, 0x0F, 0x38, 0x20, 0x05, 0x10, 0x00, 0x00, 0x00}, {0x66, 0x0F, 0x38, 0x23, 0x44, 0x24, 0x08}}; + Bytes padded; + for (const auto& instruction : instructions) { + padded = instruction; + padded.insert(padded.end(), 8, 0x90); + require(decoder.Decode(padded.data(), padded.size()) == instruction.size(), "Two-byte opcode was decoded with operand bytes it does not have"); + require(decoder.DecodeInstruction(padded.data(), padded.size()).Length == instruction.size(), "Two-byte opcode was described with operand bytes it does not have"); + } + const Bytes controlRegister = {0x0F, 0x20, 0x05, 0x90, 0x90, 0x90, 0x90}; + const auto control = decoder.DecodeInstruction(controlRegister.data(), controlRegister.size()); + require(control.Length == 3 && control.HasModRm && control.ModRmByte == 0x05 && !control.HasRipRelativeDisp, "MOV from a control register was reported as RIP-relative"); + const Bytes debugRegister = {0x0F, 0x23, 0x05, 0x90, 0x90, 0x90, 0x90}; + require(!decoder.DecodeInstruction(debugRegister.data(), debugRegister.size()).HasRipRelativeDisp, "MOV to a debug register was reported as RIP-relative"); + const Bytes extend = {0x66, 0x0F, 0x38, 0x20, 0x05, 0x10, 0x00, 0x00, 0x00}; + const auto extended = decoder.DecodeInstruction(extend.data(), extend.size()); + require(extended.HasRipRelativeDisp && read(extend, extended.RipRelativeDispOffset) == 0x10, "PMOVSXBW lost its RIP-relative operand"); + const Bytes bitTest = {0x0F, 0xAB, 0x05, 0x10, 0x00, 0x00, 0x00}; + const auto bts = decoder.DecodeInstruction(bitTest.data(), bitTest.size()); + require(bts.HasRipRelativeDisp && read(bitTest, bts.RipRelativeDispOffset) == 0x10, "BTS lost its RIP-relative operand"); +} + void sse4aOperands() { const auto check = [](const Bytes& site, const bool insertq, const int dst, const int src, const int length, const int index) { const auto operands = Codegen::DecodeSse4a(site.data(), site.size()); @@ -1113,6 +1140,7 @@ int main() { try { decoderLengths(); decoderRipRelative(); + decoderTwoByteOpcodeLengths(); sse4aOperands(); sha256Operands(); sha1Operands();