Skip to content

Commit

Permalink
src/interpreter: Remove/explain masks for 32-bit shift operations
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
qmonnet committed Mar 15, 2024
1 parent 7e9f2e1 commit 19a2be8
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_ {
Expand Down Expand Up @@ -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,
Expand All @@ -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 => {
Expand Down

0 comments on commit 19a2be8

Please sign in to comment.