Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazily calculate target pc for CALL_IMM #627

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions examples/to_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn to_json(program: &[u8]) -> String {
let analysis = Analysis::from_executable(&executable).unwrap();

let mut json_insns = vec![];
for insn in analysis.instructions.iter() {
for (pc, insn) in analysis.instructions.iter().enumerate() {
json_insns.push(object!(
"opc" => format!("{:#x}", insn.opc), // => insn.opc,
"dst" => format!("{:#x}", insn.dst), // => insn.dst,
Expand All @@ -53,7 +53,8 @@ fn to_json(program: &[u8]) -> String {
// has no effect and the complete value is printed anyway.
"imm" => format!("{:#x}", insn.imm as i32), // => insn.imm,
"desc" => analysis.disassemble_instruction(
insn
insn,
pc
),
));
}
Expand Down
17 changes: 13 additions & 4 deletions src/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,11 @@ pub fn assemble<C: ContextObject>(
insn(opc, 0, 0, resolve_label(insn_ptr, &labels, label)?, 0)
}
(CallImm, [Integer(imm)]) => {
let instr_imm = if sbpf_version.static_syscalls() {
*imm
} else {
*imm + insn_ptr as i64 + 1
};
let target_pc = *imm + insn_ptr as i64 + 1;
let label = format!("function_{}", target_pc as usize);
function_registry
Expand All @@ -426,7 +431,7 @@ pub fn assemble<C: ContextObject>(
target_pc as usize,
)
.map_err(|_| format!("Label hash collision {name}"))?;
insn(opc, 0, 1, 0, target_pc)
insn(opc, 0, 1, 0, instr_imm)
}
(CallReg, [Register(dst)]) => {
if sbpf_version.callx_uses_src_reg() {
Expand Down Expand Up @@ -461,10 +466,14 @@ pub fn assemble<C: ContextObject>(
(Syscall, [Integer(imm)]) => insn(opc, 0, 0, 0, *imm),
(CallImm, [Label(label)]) => {
let label: &str = label;
let target_pc = *labels
let mut target_pc = *labels
.get(label)
.ok_or_else(|| format!("Label not found {label}"))?;
insn(opc, 0, 1, 0, target_pc as i64)
.ok_or_else(|| format!("Label not found {label}"))?
as i64;
if sbpf_version.static_syscalls() {
target_pc = target_pc - insn_ptr as i64 - 1;
}
insn(opc, 0, 1, 0, target_pc)
}
(Endian(size), [Register(dst)]) => insn(opc, *dst, 0, 0, size),
(LoadDwImm, [Register(dst), Integer(imm)]) => {
Expand Down
6 changes: 4 additions & 2 deletions src/disassembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ fn jmp_reg_str(name: &str, insn: &ebpf::Insn, cfg_nodes: &BTreeMap<usize, CfgNod
/// Disassemble an eBPF instruction
#[rustfmt::skip]
pub fn disassemble_instruction<C: ContextObject>(
insn: &ebpf::Insn,
insn: &ebpf::Insn,
pc: usize,
cfg_nodes: &BTreeMap<usize, CfgNode>,
function_registry: &FunctionRegistry<usize>,
loader: &BuiltinProgram<C>,
Expand Down Expand Up @@ -265,7 +266,8 @@ pub fn disassemble_instruction<C: ContextObject>(
ebpf::JSLE_IMM => { name = "jsle"; desc = jmp_imm_str(name, insn, cfg_nodes); },
ebpf::JSLE_REG => { name = "jsle"; desc = jmp_reg_str(name, insn, cfg_nodes); },
ebpf::CALL_IMM => {
let function_name = function_registry.lookup_by_key(insn.imm as u32).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string());
let key = sbpf_version.calculate_call_imm_target_pc(pc, insn.imm);
let function_name = function_registry.lookup_by_key(key).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string());
let function_name = if let Some(function_name) = function_name {
name = "call";
function_name
Expand Down
17 changes: 8 additions & 9 deletions src/elf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,10 +799,7 @@ impl<C: ContextObject> Executable<C> {
.ok_or(ElfError::ValueOutOfBounds)?;
for i in 0..instruction_count {
let insn = ebpf::get_insn(text_bytes, i);
if insn.opc == ebpf::CALL_IMM
&& insn.imm != -1
&& !(sbpf_version.static_syscalls() && insn.src == 0)
{
if insn.opc == ebpf::CALL_IMM && insn.imm != -1 {
let target_pc = (i as isize)
.saturating_add(1)
.saturating_add(insn.imm as isize);
Expand All @@ -820,11 +817,13 @@ impl<C: ContextObject> Executable<C> {
name.as_bytes(),
target_pc as usize,
)?;
let offset = i.saturating_mul(ebpf::INSN_SIZE).saturating_add(4);
let checked_slice = text_bytes
.get_mut(offset..offset.saturating_add(4))
.ok_or(ElfError::ValueOutOfBounds)?;
LittleEndian::write_u32(checked_slice, key);
if !sbpf_version.static_syscalls() {
let offset = i.saturating_mul(ebpf::INSN_SIZE).saturating_add(4);
let checked_slice = text_bytes
.get_mut(offset..offset.saturating_add(4))
.ok_or(ElfError::ValueOutOfBounds)?;
LittleEndian::write_u32(checked_slice, key);
}
}
}

Expand Down
11 changes: 10 additions & 1 deletion src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,16 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> {
}

if internal && !resolved {
if let Some((_function_name, target_pc)) = self.executable.get_function_registry().lookup_by_key(insn.imm as u32) {
if let Some((_function_name, target_pc)) =
self.executable
.get_function_registry()
.lookup_by_key(
self
.executable
.get_sbpf_version()
.calculate_call_imm_target_pc(self.reg[11] as usize, insn.imm)
)
{
resolved = true;

// make BPF to BPF call
Expand Down
15 changes: 12 additions & 3 deletions src/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,18 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
}

if internal {
if let Some((_function_name, target_pc)) = self.executable.get_function_registry().lookup_by_key(insn.imm as u32) {
self.emit_internal_call(Value::Constant64(target_pc as i64, true));
resolved = true;
if let Some((_function_name, target_pc)) =
self.executable
.get_function_registry()
.lookup_by_key(
self
.executable
.get_sbpf_version()
.calculate_call_imm_target_pc(self.pc, insn.imm)
)
{
self.emit_internal_call(Value::Constant64(target_pc as i64, true));
resolved = true;
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ impl SBPFVersion {
self != SBPFVersion::V1
}

/// Calculate the target program counter for a CALL_IMM instruction depending on
/// the SBPF version.
pub fn calculate_call_imm_target_pc(self, pc: usize, imm: i64) -> u32 {
if self.static_syscalls() {
(pc as i64).saturating_add(imm).saturating_add(1) as u32
} else {
imm as u32
}
}

/// Move opcodes of memory instructions into ALU instruction classes
pub fn move_memory_instruction_classes(self) -> bool {
self != SBPFVersion::V1
Expand Down
13 changes: 7 additions & 6 deletions src/static_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,10 @@ impl<'a> Analysis<'a> {
}

/// Generates assembler code for a single instruction
pub fn disassemble_instruction(&self, insn: &ebpf::Insn) -> String {
pub fn disassemble_instruction(&self, insn: &ebpf::Insn, pc: usize) -> String {
disassemble_instruction(
insn,
pc,
&self.cfg_nodes,
self.executable.get_function_registry(),
self.executable.get_loader(),
Expand All @@ -455,14 +456,14 @@ impl<'a> Analysis<'a> {
/// Generates assembler code for the analyzed executable
pub fn disassemble<W: std::io::Write>(&self, output: &mut W) -> std::io::Result<()> {
let mut last_basic_block = usize::MAX;
for insn in self.instructions.iter() {
for (pc, insn) in self.instructions.iter().enumerate() {
self.disassemble_label(
output,
Some(insn) == self.instructions.first(),
insn.ptr,
&mut last_basic_block,
)?;
writeln!(output, " {}", self.disassemble_instruction(insn))?;
writeln!(output, " {}", self.disassemble_instruction(insn, pc))?;
}
Ok(())
}
Expand Down Expand Up @@ -493,7 +494,7 @@ impl<'a> Analysis<'a> {
index,
&entry[0..11],
pc,
self.disassemble_instruction(insn),
self.disassemble_instruction(insn, pc),
)?;
}
Ok(())
Expand Down Expand Up @@ -545,9 +546,9 @@ impl<'a> Analysis<'a> {
writeln!(output, " lbb_{} [label=<<table border=\"0\" cellborder=\"0\" cellpadding=\"3\">{}</table>>];",
cfg_node_start,
analysis.instructions[cfg_node.instructions.clone()].iter()
.map(|insn| {
.enumerate().map(|(pc, insn)| {
let desc = analysis.disassemble_instruction(
insn
insn, pc
);
if let Some(split_index) = desc.find(' ') {
let mut rest = desc[split_index+1..].to_string();
Expand Down
5 changes: 3 additions & 2 deletions src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,11 @@ impl Verifier for RequisiteVerifier {
ebpf::JSLE_IMM => { check_jmp_offset(prog, insn_ptr, &function_range)?; },
ebpf::JSLE_REG => { check_jmp_offset(prog, insn_ptr, &function_range)?; },
ebpf::CALL_IMM if sbpf_version.static_syscalls() => {
let target_pc = sbpf_version.calculate_call_imm_target_pc(insn_ptr, insn.imm);
check_call_target(
insn.imm as u32,
target_pc,
function_registry,
VerifierError::InvalidFunction(insn.imm as usize)
VerifierError::InvalidFunction(target_pc as usize)
)?;
},
ebpf::CALL_IMM => {},
Expand Down
2 changes: 1 addition & 1 deletion tests/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn test_call_reg() {
fn test_call_imm() {
assert_eq!(
asm("call 299"),
Ok(vec![insn(0, ebpf::CALL_IMM, 0, 1, 0, 300)])
Ok(vec![insn(0, ebpf::CALL_IMM, 0, 1, 0, 299)])
);
}

Expand Down
3 changes: 2 additions & 1 deletion tests/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2962,8 +2962,9 @@ fn test_err_exit_capped() {
);
test_interpreter_and_jit_asm!(
"
call 1
call function_foo
exit
function_foo:
mov r0, r0
exit
",
Expand Down
2 changes: 1 addition & 1 deletion tests/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ fn test_verifier_err_unknown_opcode() {
}

#[test]
#[should_panic(expected = "InvalidFunction(1811268606)")]
#[should_panic(expected = "InvalidFunction(1811268607)")]
fn test_verifier_unknown_sycall() {
let prog = &[
0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe
Expand Down