Просмотр исходного кода

src/interpreter.rs: Fix left/right shifts implementation (mask offset)

The current version of the BPF Instruction Set Specification specifies
that "Shift operations use a mask of 0x3F (63) for 64-bit operations and
0x1F (31) for 32-bit operations" [0].

The current implementation is not compliant, and Rust complains if we
overflow the number of bits we have when trying to shift. Let's fix it,
and let's complete the test suite regarding left and right shift
operations.

Note: The JITs are not updated at this stage. The standard does not
mention any difference between JIT and interpreter, but the kernel
clearly considers the masking for the interpreter only. JIT-compilers
are supposed to handle the overflow case in an implementation-defined
(architecture-dependant) fashion. Kernel also use masking for shifts
with registers only (not immediates) and handles overflows with
immediates in the verifier by rejecting the programs.

[0]: https://www.ietf.org/archive/id/draft-thaler-bpf-isa-02.html#section-3.1-18

Reported-by: HAPPY <pcy190@126.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Quentin Monnet 1 год назад
Родитель
Сommit
7bebff57be
2 измененных файлов с 84 добавлено и 9 удалено
  1. 10 8
      src/interpreter.rs
  2. 74 1
      tests/ubpf_vm.rs

+ 10 - 8
src/interpreter.rs

@@ -37,6 +37,8 @@ fn check_mem(addr: u64, len: usize, access_type: &str, insn_ptr: usize,
 #[allow(cyclomatic_complexity)]
 pub fn execute_program(prog_: Option<&[u8]>, mem: &[u8], mbuff: &[u8], helpers: &HashMap<u32, ebpf::Helper>) -> Result<u64, Error> {
     const U32MAX: u64 = u32::MAX as u64;
+    const SHIFT_MASK_32: u32 = 0x1f;
+    const SHIFT_MASK_64: u64 = 0x3f;
 
     let prog = match prog_ {
         Some(prog) => prog,
@@ -224,10 +226,10 @@ pub fn execute_program(prog_: Option<&[u8]>, mem: &[u8], mbuff: &[u8], helpers:
             ebpf::OR32_REG   =>   reg[_dst] = (reg[_dst] as u32             | reg[_src] as u32) as u64,
             ebpf::AND32_IMM  =>   reg[_dst] = (reg[_dst] as u32             & insn.imm  as u32) as u64,
             ebpf::AND32_REG  =>   reg[_dst] = (reg[_dst] as u32             & reg[_src] as u32) as u64,
-            ebpf::LSH32_IMM  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shl(insn.imm  as u32) as u64,
-            ebpf::LSH32_REG  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shl(reg[_src] as u32) as u64,
-            ebpf::RSH32_IMM  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shr(insn.imm  as u32) as u64,
-            ebpf::RSH32_REG  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shr(reg[_src] as u32) as u64,
+            ebpf::LSH32_IMM  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shl(insn.imm  as u32 & SHIFT_MASK_32) as u64,
+            ebpf::LSH32_REG  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shl(reg[_src] as u32 & SHIFT_MASK_32) as u64,
+            ebpf::RSH32_IMM  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shr(insn.imm  as u32 & SHIFT_MASK_32) as u64,
+            ebpf::RSH32_REG  =>   reg[_dst] = (reg[_dst] as u32).wrapping_shr(reg[_src] as u32 & SHIFT_MASK_32) as u64,
             ebpf::NEG32      => { reg[_dst] = (reg[_dst] as i32).wrapping_neg()                 as u64; reg[_dst] &= U32MAX; },
             ebpf::MOD32_IMM if insn.imm as u32 == 0 => (),
             ebpf::MOD32_IMM  =>   reg[_dst] = (reg[_dst] as u32             % insn.imm  as u32) as u64,
@@ -271,10 +273,10 @@ pub fn execute_program(prog_: Option<&[u8]>, mem: &[u8], mbuff: &[u8], helpers:
             ebpf::OR64_REG   => reg[_dst] |=  reg[_src],
             ebpf::AND64_IMM  => reg[_dst] &=  insn.imm as u64,
             ebpf::AND64_REG  => reg[_dst] &=  reg[_src],
-            ebpf::LSH64_IMM  => reg[_dst] <<= insn.imm as u64,
-            ebpf::LSH64_REG  => reg[_dst] <<= reg[_src],
-            ebpf::RSH64_IMM  => reg[_dst] >>= insn.imm as u64,
-            ebpf::RSH64_REG  => reg[_dst] >>= reg[_src],
+            ebpf::LSH64_IMM  => reg[_dst] <<= insn.imm as u64 & SHIFT_MASK_64,
+            ebpf::LSH64_REG  => reg[_dst] <<= reg[_src] & SHIFT_MASK_64,
+            ebpf::RSH64_IMM  => reg[_dst] >>= insn.imm as u64 & SHIFT_MASK_64,
+            ebpf::RSH64_REG  => reg[_dst] >>= reg[_src] & SHIFT_MASK_64,
             ebpf::NEG64      => reg[_dst] = -(reg[_dst] as i64) as u64,
             ebpf::MOD64_IMM if insn.imm == 0 => (),
             ebpf::MOD64_IMM  => reg[_dst] %=  insn.imm as u64,

+ 74 - 1
tests/ubpf_vm.rs

@@ -1598,10 +1598,20 @@ fn test_vm_le64() {
     assert_eq!(vm.execute_program(mem).unwrap(), 0x1122334455667788);
 }
 
+#[test]
+fn test_vm_lsh_imm() {
+    let prog = assemble("
+        mov r0, 1
+        lsh r0, 4
+        exit").unwrap();
+    let vm = rbpf::EbpfVmNoData::new(Some(&prog)).unwrap();
+    assert_eq!(vm.execute_program().unwrap(), 0x10);
+}
+
 #[test]
 fn test_vm_lsh_reg() {
     let prog = assemble("
-        mov r0, 0x1
+        mov r0, 1
         mov r7, 4
         lsh r0, r7
         exit").unwrap();
@@ -1609,6 +1619,69 @@ fn test_vm_lsh_reg() {
     assert_eq!(vm.execute_program().unwrap(), 0x10);
 }
 
+#[test]
+fn test_vm_lsh32_imm() {
+    let prog = assemble("
+        mov32 r0, 1
+        lsh32 r0, 4
+        exit").unwrap();
+    let vm = rbpf::EbpfVmNoData::new(Some(&prog)).unwrap();
+    assert_eq!(vm.execute_program().unwrap(), 0x10);
+}
+
+#[test]
+fn test_vm_lsh32_reg() {
+    let prog = assemble("
+        mov32 r0, 1
+        mov32 r7, 4
+        lsh32 r0, r7
+        exit").unwrap();
+    let vm = rbpf::EbpfVmNoData::new(Some(&prog)).unwrap();
+    assert_eq!(vm.execute_program().unwrap(), 0x10);
+}
+
+#[test]
+fn test_vm_lsh_imm_overflow() {
+    let prog = assemble("
+        mov r0, 1
+        lsh r0, 64
+        exit").unwrap();
+    let vm = rbpf::EbpfVmNoData::new(Some(&prog)).unwrap();
+    assert_eq!(vm.execute_program().unwrap(), 0x1);
+}
+
+#[test]
+fn test_vm_lsh_reg_overflow() {
+    let prog = assemble("
+        mov r0, 1
+        mov r7, 64
+        lsh r0, r7
+        exit").unwrap();
+    let vm = rbpf::EbpfVmNoData::new(Some(&prog)).unwrap();
+    assert_eq!(vm.execute_program().unwrap(), 0x1);
+}
+
+#[test]
+fn test_vm_lsh32_imm_overflow() {
+    let prog = assemble("
+        mov32 r0, 1
+        lsh32 r0, 32
+        exit").unwrap();
+    let vm = rbpf::EbpfVmNoData::new(Some(&prog)).unwrap();
+    assert_eq!(vm.execute_program().unwrap(), 0x1);
+}
+
+#[test]
+fn test_vm_lsh32_reg_overflow() {
+    let prog = assemble("
+        mov32 r0, 1
+        mov32 r7, 32
+        lsh32 r0, r7
+        exit").unwrap();
+    let vm = rbpf::EbpfVmNoData::new(Some(&prog)).unwrap();
+    assert_eq!(vm.execute_program().unwrap(), 0x1);
+}
+
 #[test]
 fn test_vm_mod() {
     let prog = assemble("