From 3cdefa5aa31580c388d834a67cb496280889b173 Mon Sep 17 00:00:00 2001 From: SAKIBUL HUDA ONNAY Date: Wed, 28 Oct 2026 23:20:21 +0600 Subject: [PATCH] fix(relinker): decode UD0 ModRM operands before trapping --- .../codegen/src/x86/X64InstructionDecoder.cpp | 6 ++--- .../codegen/tests/DecoderLengthTests.cpp | 23 +++++++++++++++++++ .../codegen/tests/DecoderMetadataCases.hpp | 21 ++++++++++++++++- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp b/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp index 432ecb4b64..b72a10d143 100644 --- a/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp +++ b/core/relinker/codegen/src/x86/X64InstructionDecoder.cpp @@ -254,7 +254,7 @@ std::size_t X64InstructionDecoder::Decode(const std::uint8_t* data, std::size_t hasModRm = true; immediateSize = ImmSize8; } else if (opcode == TwoByteShldCl || opcode == TwoByteShrdCl || - opcode == TwoBytePopcnt || opcode == TwoByteUd1 || opcode == TwoByteBtc || + opcode == TwoBytePopcnt || opcode == TwoByteUd1 || opcode == 0xFF || opcode == TwoByteBtc || opcode == TwoBytePrefetchw || opcode == TwoByteXaddRm8 || opcode == TwoByteMovnti || opcode == TwoByteGrp6 || opcode == TwoByteLar || opcode == TwoByteLsl || @@ -531,11 +531,11 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction( } } } else { - if (op == 0x0B || op == 0xFF) { + if (op == 0x0B) { info.FlowKind = ControlFlowKind::Trap; return info; } - if (op == TwoByteUd1) { + if (op == TwoByteUd1 || op == 0xFF) { if (pos < info.Length) readModRm(pos, false); info.FlowKind = ControlFlowKind::Trap; diff --git a/core/relinker/codegen/tests/DecoderLengthTests.cpp b/core/relinker/codegen/tests/DecoderLengthTests.cpp index a3f0ade694..e1d39cae30 100644 --- a/core/relinker/codegen/tests/DecoderLengthTests.cpp +++ b/core/relinker/codegen/tests/DecoderLengthTests.cpp @@ -78,6 +78,29 @@ int main() { } catch (const Codegen::CodegenException&) { } } + for (const auto* testCase : kDecoderMissingModRmCases) { + const auto bytes = parse(testCase); + require(truncationRejected(decoder, bytes, bytes.size()), "A truncated " + std::string(testCase) + " was accepted"); + try { + (void)decoder.DecodeInstruction(bytes.data(), bytes.size()); + throw std::runtime_error("Instruction metadata accepted a missing ModRM for " + std::string(testCase)); + } catch (const Codegen::CodegenException&) { + } + } + for (const auto& testCase : kDecoderTrapModRmCases) { + const std::string name = testCase.Bytes; + auto bytes = parse(testCase.Bytes); + require(bytes.size() == testCase.Length, "Trap case " + name + " does not hold its own length"); + require(truncationRejected(decoder, bytes, bytes.size() - 1), "A truncated " + name + " was accepted"); + bytes.insert(bytes.end(), 15, 0x90); + const auto info = decoder.DecodeInstruction(bytes.data(), bytes.size()); + require(info.Length == testCase.Length, "Wrong length for " + name); + require(info.FlowKind == Codegen::ControlFlowKind::Trap && !info.HasBranchTarget, "Wrong trap metadata for " + name); + require(info.HasModRm && info.ModRmByte == testCase.ModRmByte && info.ModRmOffset == 2, "Wrong ModRM metadata for " + name); + require(info.HasMemoryOperand == testCase.HasMemoryOperand, "Wrong memory operand presence for " + name); + require(info.HasRipRelativeDisp == testCase.HasRipRelativeDisp, "Wrong RIP-relative operand presence for " + name); + require(info.RipRelativeDispOffset == testCase.DisplacementOffset, "Wrong RIP-relative displacement offset for " + name); + } for (const auto& testCase : kDecoderRipRelativeCases) { const std::string name = testCase.Bytes; auto bytes = parse(testCase.Bytes); diff --git a/core/relinker/codegen/tests/DecoderMetadataCases.hpp b/core/relinker/codegen/tests/DecoderMetadataCases.hpp index 5e475260c6..78b31ab91a 100644 --- a/core/relinker/codegen/tests/DecoderMetadataCases.hpp +++ b/core/relinker/codegen/tests/DecoderMetadataCases.hpp @@ -723,7 +723,6 @@ inline constexpr DecoderRipRelativeCase kDecoderRipRelativeCases[] = { inline constexpr DecoderBranchCase kDecoderBranchCases[] = { {"0f0b", 2, Codegen::ControlFlowKind::Trap, false, 0}, - {"0fff", 2, Codegen::ControlFlowKind::Trap, false, 0}, {"0f80c0111111", 6, Codegen::ControlFlowKind::ConditionalBranch, true, 286331328}, {"0f81c0111111", 6, Codegen::ControlFlowKind::ConditionalBranch, true, 286331328}, {"0f82c0111111", 6, Codegen::ControlFlowKind::ConditionalBranch, true, 286331328}, @@ -795,6 +794,26 @@ inline constexpr DecoderBranchCase kDecoderBranchCases[] = { {"664875c0", 4, Codegen::ControlFlowKind::ConditionalBranch, true, -64}, }; +struct DecoderTrapModRmCase { + const char* Bytes; + std::size_t Length; + std::uint8_t ModRmByte; + bool HasMemoryOperand; + bool HasRipRelativeDisp; + std::size_t DisplacementOffset; +}; + +inline constexpr DecoderTrapModRmCase kDecoderTrapModRmCases[] = { + {"0fff0511111111", 7, 0x05, true, true, 3}, + {"0fffc0", 3, 0xc0, false, false, 0}, + {"0fff042511111111", 8, 0x04, true, false, 0}, +}; + +inline constexpr const char* kDecoderMissingModRmCases[] = { + "0fb9", + "0fff", +}; + struct DecoderTruncatedBranchCase { const char* Bytes; std::size_t Length;