From f7ad168586f117473f6f01715c57271a79db452c Mon Sep 17 00:00:00 2001 From: hsqStephenZhang Date: Sat, 11 Jan 2025 19:09:38 +0800 Subject: [PATCH 1/2] fix: clarify signed & unsigned for jump insts Signed-off-by: hsqStephenZhang --- src/interpreter.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index 1c79bf6a9..8a21171d5 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -90,6 +90,12 @@ pub fn execute_program( insn_ptr = (insn_ptr as i16 + insn.off) as usize; }; + macro_rules! unsigned_u64 { + ($imm:expr) => { + ($imm as u32) as u64 + }; + } + match insn.opc { // BPF_LD class @@ -298,20 +304,22 @@ pub fn execute_program( // BPF_JMP class // TODO: check this actually works as expected for signed / unsigned ops + // J-EQ, J-NE, J-GT, J-GE, J-LT, J-LE: unsigned + // JS-GT, JS-GE, JS-LT, JS-LE: signed ebpf::JA => do_jump(), - ebpf::JEQ_IMM => if reg[_dst] == insn.imm as u64 { do_jump(); }, + ebpf::JEQ_IMM => if reg[_dst] == unsigned_u64!(insn.imm) { do_jump(); }, ebpf::JEQ_REG => if reg[_dst] == reg[_src] { do_jump(); }, - ebpf::JGT_IMM => if reg[_dst] > insn.imm as u64 { do_jump(); }, + ebpf::JGT_IMM => if reg[_dst] > unsigned_u64!(insn.imm) { do_jump(); }, ebpf::JGT_REG => if reg[_dst] > reg[_src] { do_jump(); }, - ebpf::JGE_IMM => if reg[_dst] >= insn.imm as u64 { do_jump(); }, + ebpf::JGE_IMM => if reg[_dst] >= unsigned_u64!(insn.imm) { do_jump(); }, ebpf::JGE_REG => if reg[_dst] >= reg[_src] { do_jump(); }, - ebpf::JLT_IMM => if reg[_dst] < insn.imm as u64 { do_jump(); }, + ebpf::JLT_IMM => if reg[_dst] < unsigned_u64!(insn.imm) { do_jump(); }, ebpf::JLT_REG => if reg[_dst] < reg[_src] { do_jump(); }, - ebpf::JLE_IMM => if reg[_dst] <= insn.imm as u64 { do_jump(); }, + ebpf::JLE_IMM => if reg[_dst] <= unsigned_u64!(insn.imm) { do_jump(); }, ebpf::JLE_REG => if reg[_dst] <= reg[_src] { do_jump(); }, ebpf::JSET_IMM => if reg[_dst] & insn.imm as u64 != 0 { do_jump(); }, ebpf::JSET_REG => if reg[_dst] & reg[_src] != 0 { do_jump(); }, - ebpf::JNE_IMM => if reg[_dst] != insn.imm as u64 { do_jump(); }, + ebpf::JNE_IMM => if reg[_dst] != unsigned_u64!(insn.imm) { do_jump(); }, ebpf::JNE_REG => if reg[_dst] != reg[_src] { do_jump(); }, ebpf::JSGT_IMM => if reg[_dst] as i64 > insn.imm as i64 { do_jump(); }, ebpf::JSGT_REG => if reg[_dst] as i64 > reg[_src] as i64 { do_jump(); }, From 8098c816d98c7da5d464cc3bcf1de27dcb4eb45a Mon Sep 17 00:00:00 2001 From: hsqStephenZhang Date: Wed, 15 Jan 2025 13:12:19 +0800 Subject: [PATCH 2/2] fix: add jmp insn unsigned extend unittest Signed-off-by: hsqStephenZhang --- tests/ubpf_vm.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/ubpf_vm.rs b/tests/ubpf_vm.rs index 8918e52b9..9f3a2eb90 100644 --- a/tests/ubpf_vm.rs +++ b/tests/ubpf_vm.rs @@ -769,6 +769,38 @@ fn test_vm_jset_imm() { assert_eq!(vm.execute_program().unwrap(), 0x1); } +#[test] +fn test_vm_jmp_unsigned_extend() { + use rbpf::ebpf::{Insn, BE, EXIT, JEQ_IMM, MOV32_IMM, LD_W_REG}; + + // the insn `jeq r2, 0x80000000, +2` will be rejected + assert!(assemble("jeq r2, 0x80000000, +2").is_err()); + // the prog is as follows: + // ldxw r2, [r1] + // be32 r2 + // jeq r2, 0x80000000, +2 # 0x80000000 should be interpreted as 0x0000000080000000 (unsigned) + // mov32 r0, 2 + // exit + // mov32 r0, 1 + // exit + // we build it manually to bypass the verifier in `assemble(...)` + let insns = vec![ + Insn { opc: LD_W_REG, dst: 2, src: 1, off: 0, imm: 0 }, + Insn { opc: BE, dst: 2, src: 0, off: 0, imm: 32 }, + Insn { opc: JEQ_IMM, dst: 2, src: 0, off: 2, imm: 0x80000000u32 as i32 }, + Insn { opc: MOV32_IMM, dst: 0, src: 0, off: 0, imm: 2 }, + Insn { opc: EXIT, dst: 0, src: 0, off: 0, imm: 0 }, + Insn { opc: MOV32_IMM, dst: 0, src: 0, off: 0, imm: 1 }, + Insn { opc: EXIT, dst: 0, src: 0, off: 0, imm: 0 } + ]; + let prog = insns.iter().map(|x| x.to_array()).flatten().collect::>(); + let vm = rbpf::EbpfVmRaw::new(Some(&prog)).unwrap(); + let mut data = vec![ + 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + ]; + assert_eq!(vm.execute_program(&mut data).unwrap(), 1); +} + #[test] fn test_vm_jset_reg() { let prog = assemble("