Jelajahi Sumber

src/interpreter: Remove/explain masks for 32-bit shift operations

We recently fixed the 64-bit arithmetic shift-right operations by adding
a mask to the number of bits (as immediates or from the source register)
by which to shift, as per the eBPF ISA specification. The 32-bit
operations must use the same mask, but .wrapping_shr() already takes
care of it for us. Let's add a comment to make it explicit.

As it turns out, masking is just as well unnecessary for the
non-arithmetic left-right shift operations that we tried to fix
recently. Let's also remove the mask there.

Signed-off-by: Quentin Monnet <qmo@qmon.net>
Quentin Monnet 1 tahun lalu
induk
melakukan
3e80ed720a
1 mengubah file dengan 8 tambahan dan 5 penghapusan
  1. 8 5
      src/interpreter.rs

+ 8 - 5
src/interpreter.rs

@@ -37,7 +37,6 @@ 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_ {
@@ -226,10 +225,12 @@ 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 & 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,
+            // As for the 64-bit version, we should mask the number of bits to shift with
+            // 0x1f, but .wrappping_shr() already takes care of it for us.
+            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::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,
@@ -239,6 +240,8 @@ pub fn execute_program(prog_: Option<&[u8]>, mem: &[u8], mbuff: &[u8], helpers:
             ebpf::XOR32_REG  =>   reg[_dst] = (reg[_dst] as u32             ^ reg[_src] as u32) as u64,
             ebpf::MOV32_IMM  =>   reg[_dst] = insn.imm   as u32                                 as u64,
             ebpf::MOV32_REG  =>   reg[_dst] = (reg[_src] as u32)                                as u64,
+            // As for the 64-bit version, we should mask the number of bits to shift with
+            // 0x1f, but .wrappping_shr() already takes care of it for us.
             ebpf::ARSH32_IMM => { reg[_dst] = (reg[_dst] as i32).wrapping_shr(insn.imm  as u32) as u64; reg[_dst] &= U32MAX; },
             ebpf::ARSH32_REG => { reg[_dst] = (reg[_dst] as i32).wrapping_shr(reg[_src] as u32) as u64; reg[_dst] &= U32MAX; },
             ebpf::LE         => {