solana-labs / rbpf

Rust virtual machine and JIT compiler for eBPF programs
Apache License 2.0
277 stars 169 forks source link

Removing check SHIFT immediate number from verifier #561

Closed shenghaoyuan closed 3 months ago

shenghaoyuan commented 5 months ago

Hi, could we remove all check_imm_shift from the verifier, or do this checking during JIT's emit_shift?

ebpf::LSH32_IMM  => { check_imm_shift(&insn, insn_ptr, 32)?; },

For the interpreter, the wrapping_shl has ensured imm%64 or imm%32.

ebpf::LSH32_IMM  => self.reg[dst] = (self.reg[dst] as u32).wrapping_shl(insn.imm as u32)      as u64,
ebpf::LSH32_REG  => self.reg[dst] = (self.reg[dst] as u32).wrapping_shl(self.reg[src] as u32) as u64,

BTW, I don't find how the JIT emit_shift does src%32 in the X64 code for rbpf shl_reg instruction (sorry, I am not familiar with x86 ISA if this is a stupid question).

Lichtso commented 5 months ago

Is there a specific need for this? Generally we can't just remove verification steps and add them to the JIT only, because then the Interpreter and JIT could disagree.

shenghaoyuan commented 5 months ago

Hello Lichtso, The interpreter has a wrapping semantics, e.g. LSH32_IMM does imm%32, so rbpf could do the same wrapping operation imm%32 during the JIT process. In this case, the shift verification step could be removed. The current way is OK: someone may confuse that the shift verification step in verifier and the wrapping semantics in interpreter are overlapping. Best, Shenghao