Skip to content

Commit

Permalink
Adds SbpfVersion::fix_callx_encoding() to make callx use the src reg …
Browse files Browse the repository at this point in the history
…field instead of the imm field.
  • Loading branch information
Lichtso committed Jul 28, 2023
1 parent 5debf64 commit b84dcbd
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 12 deletions.
8 changes: 7 additions & 1 deletion src/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,13 @@ pub fn assemble<C: ContextObject>(
.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.callx_uses_src_reg() {
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,
Expand Down
2 changes: 1 addition & 1 deletion src/disassembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ pub fn disassemble_instruction<C: ContextObject>(
};
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.callx_uses_src_reg() { insn.src } else { insn.imm as u8 }); },
ebpf::EXIT => { name = "exit"; desc = name.to_string(); },

_ => { name = "unknown"; desc = format!("{} opcode={:#x}", name, insn.opc); },
Expand Down
5 changes: 5 additions & 0 deletions src/elf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ impl SBPFVersion {
self != &SBPFVersion::V1
}

/// Use src reg instead of imm in callx
pub fn callx_uses_src_reg(&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 {
Expand Down
12 changes: 8 additions & 4 deletions src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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().callx_uses_src_reg() {
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;
}
Expand Down
7 changes: 6 additions & 1 deletion src/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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().callx_uses_src_reg() {
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));
Expand Down
14 changes: 10 additions & 4 deletions src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.callx_uses_src_reg() {
insn.src as i64
} else {
insn.imm
};
if !(0..=10).contains(&reg) || (reg == 10 && config.reject_callx_r10) {
return Err(VerifierError::InvalidRegister(adj_insn_ptr(insn_ptr)));
}
Ok(())
Expand Down Expand Up @@ -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 => {},

_ => {
Expand Down
2 changes: 1 addition & 1 deletion tests/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)])
);
}

Expand Down

0 comments on commit b84dcbd

Please sign in to comment.