[llvm] eec7588 - [BPF] fix an asan issue when disassemble an illegal instruction

[Yonghong Song via llvm-commits]

Author: Yonghong Song
Date: 2020-05-18T22:33:34-07:00
New Revision: eec758825d2a85f229bd4914a322eab8e7d57b23

URL: https://github.com/llvm/llvm-project/commit/eec758825d2a85f229bd4914a322eab8e7d57b23
DIFF: https://github.com/llvm/llvm-project/commit/eec758825d2a85f229bd4914a322eab8e7d57b23.diff

LOG: [BPF] fix an asan issue when disassemble an illegal instruction

Commit 8e8f1bd75a9a ("[BPF] Return fail if disassembled insn registers
out of range") tried to fix a segfault when an illegal instruction
is decoded. A test case is added to emulate such an illegal instruction.

The llvm buildbot reported an asan issue with this test case.
  ERROR: AddressSanitizer: global-buffer-overflow on address ...
  decodeMemoryOpValue(llvm::MCInst&, unsigned int, ...)
  llvm::MCDisassembler::DecodeStatus llvm::decodeToMCInst<unsigned long>(...)
  llvm::MCDisassembler::DecodeStatus llvm::decodeInstruction<unsigned long>(...)
  in (anonymous namespace)::BPFDisassembler::getInstruction(...)
  ...

Basically, the fix in Commit 8e8f1bd75a9a is too later to prevent
the asan. The fix in this patch moved the register number check earlier
during decodeInstruction(). It will return fail for decodeInstruction()
if the register number is out of range.

Note that DecodeGPRRegisterClass() and DecodeGPR32RegisterClass()
already have register number checking, so here we only check
decodeMemoryOpValue().

Added: 
    

Modified: 
    llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp b/llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
index ca394fe97ec2..4d98dc7341d0 100644
--- a/llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
+++ b/llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
@@ -126,6 +126,9 @@ static DecodeStatus DecodeGPR32RegisterClass(MCInst &Inst, unsigned RegNo,
 static DecodeStatus decodeMemoryOpValue(MCInst &Inst, unsigned Insn,
                                         uint64_t Address, const void *Decoder) {
   unsigned Register = (Insn >> 16) & 0xf;
+  if (Register > 11)
+    return MCDisassembler::Fail;
+
   Inst.addOperand(MCOperand::createReg(GPRDecoderTable[Register]));
   unsigned Offset = (Insn & 0xffff);
   Inst.addOperand(MCOperand::createImm(SignExtend32<16>(Offset)));
@@ -183,14 +186,6 @@ DecodeStatus BPFDisassembler::getInstruction(MCInst &Instr, uint64_t &Size,
 
   if (Result == MCDisassembler::Fail) return MCDisassembler::Fail;
 
-  /* to ensure registers in range */
-  for (unsigned i = 0, e = Instr.getNumOperands(); i != e; ++i) {
-    const MCOperand &MO = Instr.getOperand(i);
-    if (MO.isReg() &&
-        (MO.getReg() <= BPF::NoRegister || MO.getReg() >= BPF::NUM_TARGET_REGS))
-      return MCDisassembler::Fail;
-  }
-
   switch (Instr.getOpcode()) {
   case BPF::LD_imm64:
   case BPF::LD_pseudo: {