From bbbe89c6acc682c98ad6ae4ed8c70f4815eac01c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 28 Jul 2023 12:24:44 +0200 Subject: [PATCH] Adds SbpfVersion::fix_callx_encoding() to make callx use the src reg field instead of the imm field. --- src/assembler.rs | 8 +++++++- src/disassembler.rs | 2 +- src/elf.rs | 5 +++++ src/interpreter.rs | 12 ++++++++---- src/jit.rs | 7 ++++++- src/verifier.rs | 14 ++++++++++---- tests/assembler.rs | 2 +- 7 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/assembler.rs b/src/assembler.rs index 9f3212cc..0ebe3843 100644 --- a/src/assembler.rs +++ b/src/assembler.rs @@ -304,7 +304,13 @@ pub fn assemble( .map_err(|_| format!("Label hash collision {name}"))?; insn(opc, 0, 1, 0, target_pc) } - (CallReg, [Register(dst)]) => insn(opc, 0, 0, 0, *dst), + (CallReg, [Register(dst)]) => { + if sbpf_version.fix_callx_encoding() { + insn(opc, 0, *dst, 0, 0) + } else { + insn(opc, 0, 0, 0, *dst) + } + } (JumpConditional, [Register(dst), Register(src), Label(label)]) => insn( opc | ebpf::BPF_X, *dst, diff --git a/src/disassembler.rs b/src/disassembler.rs index 7a19b43c..b3dd2c36 100644 --- a/src/disassembler.rs +++ b/src/disassembler.rs @@ -269,7 +269,7 @@ pub fn disassemble_instruction( }; desc = format!("{name} {function_name}"); }, - ebpf::CALL_REG => { name = "callx"; desc = format!("{} r{}", name, insn.imm); }, + ebpf::CALL_REG => { name = "callx"; desc = format!("{} r{}", name, if sbpf_version.fix_callx_encoding() { insn.src } else { insn.imm as u8 }); }, ebpf::EXIT => { name = "exit"; desc = name.to_string(); }, _ => { name = "unknown"; desc = format!("{} opcode={:#x}", name, insn.opc); }, diff --git a/src/elf.rs b/src/elf.rs index c3c6d936..05f9a7ee 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -230,6 +230,11 @@ impl SBPFVersion { self != &SBPFVersion::V1 } + /// Use src reg instead of imm in callx + pub fn fix_callx_encoding(&self) -> bool { + self != &SBPFVersion::V1 + } + /// Ensure that rodata sections don't exceed their maximum allowed size and /// overlap with the stack pub fn reject_rodata_stack_overlap(&self) -> bool { diff --git a/src/interpreter.rs b/src/interpreter.rs index 62c87f9e..9699344b 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -411,14 +411,18 @@ impl<'a, 'b, V: Verifier, C: ContextObject> Interpreter<'a, 'b, V, C> { ebpf::JSLE_REG => if (self.reg[dst] as i64) <= self.reg[src] as i64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, ebpf::CALL_REG => { - let target_address = self.reg[insn.imm as usize]; + let target_pc = if self.executable.get_sbpf_version().fix_callx_encoding() { + self.reg[src] + } else { + self.reg[insn.imm as usize] + }; if !self.push_frame(config) { return false; } - if target_address < self.program_vm_addr { - throw_error!(self, EbpfError::CallOutsideTextSegment(pc + ebpf::ELF_INSN_DUMP_OFFSET, target_address / ebpf::INSN_SIZE as u64 * ebpf::INSN_SIZE as u64)); + if target_pc < self.program_vm_addr { + throw_error!(self, EbpfError::CallOutsideTextSegment(pc + ebpf::ELF_INSN_DUMP_OFFSET, target_pc / ebpf::INSN_SIZE as u64 * ebpf::INSN_SIZE as u64)); } - self.pc = (target_address - self.program_vm_addr) as usize / ebpf::INSN_SIZE; + self.pc = (target_pc - self.program_vm_addr) as usize / ebpf::INSN_SIZE; if !self.check_pc(pc) { return false; } diff --git a/src/jit.rs b/src/jit.rs index 6201364d..c80677ff 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -621,7 +621,12 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> { } }, ebpf::CALL_REG => { - self.emit_internal_call(Value::Register(REGISTER_MAP[insn.imm as usize])); + let target_pc = if self.executable.get_sbpf_version().fix_callx_encoding() { + src + } else { + REGISTER_MAP[insn.imm as usize] + }; + self.emit_internal_call(Value::Register(target_pc)); }, ebpf::EXIT => { let call_depth_access = X86IndirectAccess::Offset(self.slot_on_environment_stack(RuntimeEnvironmentSlot::CallDepth)); diff --git a/src/verifier.rs b/src/verifier.rs index 67aca944..3d4f5ca0 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -203,13 +203,19 @@ fn check_imm_shift(insn: &ebpf::Insn, insn_ptr: usize, imm_bits: u64) -> Result< Ok(()) } -/// Check that the imm is a valid register number -fn check_imm_register( +/// Check that callx has a valid register number +fn check_callx_register( insn: &ebpf::Insn, insn_ptr: usize, config: &Config, + sbpf_version: &SBPFVersion, ) -> Result<(), VerifierError> { - if insn.imm < 0 || insn.imm > 10 || (insn.imm == 10 && config.reject_callx_r10) { + let reg = if sbpf_version.fix_callx_encoding() { + insn.src as i64 + } else { + insn.imm + }; + if reg < 0 || reg > 10 || (reg == 10 && config.reject_callx_r10) { return Err(VerifierError::InvalidRegister(adj_insn_ptr(insn_ptr))); } Ok(()) @@ -355,7 +361,7 @@ impl Verifier for RequisiteVerifier { ebpf::JSLE_REG => { check_jmp_offset(prog, insn_ptr, &function_range)?; }, ebpf::CALL_IMM if sbpf_version.static_syscalls() && insn.src != 0 => { check_jmp_offset(prog, insn_ptr, &program_range)?; }, ebpf::CALL_IMM => {}, - ebpf::CALL_REG => { check_imm_register(&insn, insn_ptr, config)?; }, + ebpf::CALL_REG => { check_callx_register(&insn, insn_ptr, config, sbpf_version)?; }, ebpf::EXIT => {}, _ => { diff --git a/tests/assembler.rs b/tests/assembler.rs index bbf402ee..453885ca 100644 --- a/tests/assembler.rs +++ b/tests/assembler.rs @@ -116,7 +116,7 @@ fn test_jeq() { fn test_call_reg() { assert_eq!( asm("callx r3"), - Ok(vec![insn(0, ebpf::CALL_REG, 0, 0, 0, 3)]) + Ok(vec![insn(0, ebpf::CALL_REG, 0, 3, 0, 0)]) ); }