[llvm] 8e8f1bd - [BPF] Return fail if disassembled insn registers out of range

[Tom Stellard via llvm-commits]

On 05/18/2020 06:53 PM, Yonghong Song via llvm-commits wrote:
> 
> Author: Yonghong Song
> Date: 2020-05-18T18:53:23-07:00
> New Revision: 8e8f1bd75a9abd04b1f85ba92990e373fa6f5624
> 

Should we backport this to the release/10.x branch?

-Tom

> URL: https://github.com/llvm/llvm-project/commit/8e8f1bd75a9abd04b1f85ba92990e373fa6f5624
> DIFF: https://github.com/llvm/llvm-project/commit/8e8f1bd75a9abd04b1f85ba92990e373fa6f5624.diff
> 
> LOG: [BPF] Return fail if disassembled insn registers out of range
> 
> Daniel reported a llvm-objdump segfault like below:
>   $ llvm-objdump -D bpf_xdp.o
>   ...
>   0000000000000000 <.strtab>:
>        0:       00 63 69 6c 69 75 6d 5f <unknown>
>        1:       6c 62 36 5f 61 66 66 69 w2 <<= w6
>   ...
>   (llvm-objdump: lib/Target/BPF/BPFGenAsmWriter.inc:1087: static const char*
>    llvm::BPFInstPrinter::getRegisterName(unsigned int): Assertion
>    `RegNo && RegNo < 25 && "Invalid register number!"' failed.
>    Stack dump:
>    0.      Program arguments: llvm-objdump -D bpf_xdp.o
>     ...
>     abort
>     ...
>     llvm::BPFInstPrinter::getRegisterName(unsigned int)
>     llvm::BPFInstPrinter::printMemOperand(llvm::MCInst const*,
>                           int, llvm::raw_ostream&, char const*)
>     llvm::BPFInstPrinter::printInstruction(llvm::MCInst const*,
>                           unsigned long, llvm::raw_ostream&)
>     llvm::BPFInstPrinter::printInst(llvm::MCInst const*,
>                           unsigned long, llvm::StringRef, llvm::MCSubtargetInfo const&,
>                           llvm::raw_ostream&)
>    ...
> 
> Basically, since -D enables disassembly for all sections, .strtab is also disassembled,
> but some strings are decoded as legal instructions but with illegal register numbers.
> When llvm-objdump tries to print register name for these illegal register numbers,
> assertion and segfault happens.
> 
> The patch fixed the issue by returning fail for a disassembled insn if
> that insn contains a reg operand with illegal reg number.
> The insn will be printed as "<unknown>" instead of causing an assertion.
> 
> Added: 
>     llvm/test/CodeGen/BPF/objdump_dis_all.ll
> 
> 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 75f963b5448a..ca394fe97ec2 100644
> --- a/llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
> +++ b/llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
> @@ -183,6 +183,14 @@ 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: {
> 
> diff  --git a/llvm/test/CodeGen/BPF/objdump_dis_all.ll b/llvm/test/CodeGen/BPF/objdump_dis_all.ll
> new file mode 100644
> index 000000000000..4c65d0528de9
> --- /dev/null
> +++ b/llvm/test/CodeGen/BPF/objdump_dis_all.ll
> @@ -0,0 +1,26 @@
> +; RUN: llc -march=bpfel -filetype=obj -o - %s | llvm-objdump -D - | FileCheck %s
> +;
> +; Source:
> +;   /* *(u64 *)(r10 - 16) = r1 */
> +;   unsigned long long g = 0x00000000fff01a7bULL;
> +;   /* *(u64 *)(r15 - 16) = r1 */
> +;   unsigned long long h = 0x00000000fff01f7bULL;
> +;   int test() {
> +;     return 0;
> +;   }
> +; Compilation flag:
> +;  clang -target bpf -O2 -S -emit-llvm t.c
> +
> + at g = dso_local local_unnamed_addr global i64 4293925499, align 8
> + at h = dso_local local_unnamed_addr global i64 4293926779, align 8
> +
> +; Function Attrs: norecurse nounwind readnone
> +define dso_local i32 @test() local_unnamed_addr {
> +entry:
> +  ret i32 0
> +}
> +; CHECK-LABEL: section .data
> +; CHECK-LABEL: g
> +; CHECK:       *(u64 *)(r10 - 16) = r1
> +; CHECK-LABEL: h
> +; CHECK:       <unknown>
> 
> 
>         
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>