Skip to content

Commit

Permalink
Removes ELF_INSN_DUMP_OFFSET. (#541)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Lichtso
Lichtso authored Oct 10, 2023
1 parent 06e22d7 commit ae90876
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 64 deletions.
4 changes: 0 additions & 4 deletions src/ebpf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ pub const STACK_PTR_REG: usize = 11;
pub const FIRST_SCRATCH_REG: usize = 6;
/// Number of scratch registers
pub const SCRATCH_REGS: usize = 4;
/// ELF dump instruction offset
/// Instruction numbers typically start at 29 in the ELF dump, use this offset
/// when reporting so that trace aligns with the dump.
pub const ELF_INSN_DUMP_OFFSET: usize = 29;
/// Alignment of the memory regions in host address space in bytes
pub const HOST_ALIGN: usize = 16;
/// Upper half of a pointer is the region index, lower half the virtual address inside that region.
Expand Down
15 changes: 4 additions & 11 deletions src/elf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -834,9 +834,7 @@ impl<C: ContextObject> Executable<C> {
.saturating_add(1)
.saturating_add(insn.imm as isize);
if target_pc < 0 || target_pc >= instruction_count as isize {
return Err(ElfError::RelativeJumpOutOfBounds(
i.saturating_add(ebpf::ELF_INSN_DUMP_OFFSET),
));
return Err(ElfError::RelativeJumpOutOfBounds(i));
}
let name = if config.enable_symbol_and_section_labels {
format!("function_{target_pc}")
Expand Down Expand Up @@ -1101,12 +1099,7 @@ impl<C: ContextObject> Executable<C> {
{
return Err(ElfError::UnresolvedSymbol(
String::from_utf8_lossy(name).to_string(),
r_offset
.checked_div(ebpf::INSN_SIZE)
.and_then(|offset| {
offset.checked_add(ebpf::ELF_INSN_DUMP_OFFSET)
})
.unwrap_or(ebpf::ELF_INSN_DUMP_OFFSET),
r_offset.checked_div(ebpf::INSN_SIZE).unwrap_or(0),
r_offset,
));
}
Expand Down Expand Up @@ -1951,7 +1944,7 @@ mod test {
}

#[test]
#[should_panic(expected = "validation failed: RelativeJumpOutOfBounds(38)")]
#[should_panic(expected = "validation failed: RelativeJumpOutOfBounds(9)")]
fn test_relative_call_oob_backward() {
let mut elf_bytes =
std::fs::read("tests/elfs/relative_call.so").expect("failed to read elf file");
Expand All @@ -1960,7 +1953,7 @@ mod test {
}

#[test]
#[should_panic(expected = "validation failed: RelativeJumpOutOfBounds(41)")]
#[should_panic(expected = "validation failed: RelativeJumpOutOfBounds(12)")]
fn test_relative_call_oob_forward() {
let mut elf_bytes =
std::fs::read("tests/elfs/relative_call.so").expect("failed to read elf file");
Expand Down
2 changes: 1 addition & 1 deletion src/static_analysis.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ impl<'a> Analysis<'a> {
"{:5?} {:016X?} {:5?}: {}",
index,
&entry[0..11],
pc + ebpf::ELF_INSN_DUMP_OFFSET,
pc,
self.disassemble_instruction(insn),
)?;
}
Expand Down
34 changes: 12 additions & 22 deletions src/verifier.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ pub trait Verifier {
) -> Result<(), VerifierError>;
}

fn adj_insn_ptr(insn_ptr: usize) -> usize {
insn_ptr + ebpf::ELF_INSN_DUMP_OFFSET
}

fn check_prog_len(prog: &[u8]) -> Result<(), VerifierError> {
if prog.len() % ebpf::INSN_SIZE != 0 {
return Err(VerifierError::ProgramLengthNotMultiple);
Expand All @@ -120,17 +116,15 @@ fn check_prog_len(prog: &[u8]) -> Result<(), VerifierError> {

fn check_imm_nonzero(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierError> {
if insn.imm == 0 {
return Err(VerifierError::DivisionByZero(adj_insn_ptr(insn_ptr)));
return Err(VerifierError::DivisionByZero(insn_ptr));
}
Ok(())
}

fn check_imm_endian(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierError> {
match insn.imm {
16 | 32 | 64 => Ok(()),
_ => Err(VerifierError::UnsupportedLEBEArgument(adj_insn_ptr(
insn_ptr,
))),
_ => Err(VerifierError::UnsupportedLEBEArgument(insn_ptr)),
}
}

Expand All @@ -141,7 +135,7 @@ fn check_load_dw(prog: &[u8], insn_ptr: usize) -> Result<(), VerifierError> {
}
let next_insn = ebpf::get_insn(prog, insn_ptr + 1);
if next_insn.opc != 0 {
return Err(VerifierError::IncompleteLDDW(adj_insn_ptr(insn_ptr)));
return Err(VerifierError::IncompleteLDDW(insn_ptr));
}
Ok(())
}
Expand All @@ -157,14 +151,14 @@ fn check_jmp_offset(
if dst_insn_ptr < 0 || !function_range.contains(&(dst_insn_ptr as usize)) {
return Err(VerifierError::JumpOutOfCode(
dst_insn_ptr as usize,
adj_insn_ptr(insn_ptr),
insn_ptr,
));
}
let dst_insn = ebpf::get_insn(prog, dst_insn_ptr as usize);
if dst_insn.opc == 0 {
return Err(VerifierError::JumpToMiddleOfLDDW(
dst_insn_ptr as usize,
adj_insn_ptr(insn_ptr),
insn_ptr,
));
}
Ok(())
Expand All @@ -177,16 +171,14 @@ fn check_registers(
sbpf_version: &SBPFVersion,
) -> Result<(), VerifierError> {
if insn.src > 10 {
return Err(VerifierError::InvalidSourceRegister(adj_insn_ptr(insn_ptr)));
return Err(VerifierError::InvalidSourceRegister(insn_ptr));
}

match (insn.dst, store) {
(0..=9, _) | (10, true) => Ok(()),
(11, _) if sbpf_version.dynamic_stack_frames() && insn.opc == ebpf::ADD64_IMM => Ok(()),
(10, false) => Err(VerifierError::CannotWriteR10(adj_insn_ptr(insn_ptr))),
(_, _) => Err(VerifierError::InvalidDestinationRegister(adj_insn_ptr(
insn_ptr,
))),
(10, false) => Err(VerifierError::CannotWriteR10(insn_ptr)),
(_, _) => Err(VerifierError::InvalidDestinationRegister(insn_ptr)),
}
}

Expand All @@ -195,9 +187,7 @@ fn check_imm_shift(insn: &ebpf::Insn, insn_ptr: usize, imm_bits: u64) -> Result<
let shift_by = insn.imm as u64;
if insn.imm < 0 || shift_by >= imm_bits {
return Err(VerifierError::ShiftWithOverflow(
shift_by,
imm_bits,
adj_insn_ptr(insn_ptr),
shift_by, imm_bits, insn_ptr,
));
}
Ok(())
Expand All @@ -216,7 +206,7 @@ fn check_callx_register(
insn.imm
};
if !(0..=10).contains(&reg) || (reg == 10 && config.reject_callx_r10) {
return Err(VerifierError::InvalidRegister(adj_insn_ptr(insn_ptr)));
return Err(VerifierError::InvalidRegister(insn_ptr));
}
Ok(())
}
Expand Down Expand Up @@ -387,7 +377,7 @@ impl Verifier for RequisiteVerifier {
ebpf::EXIT => {},

_ => {
return Err(VerifierError::UnknownOpCode(insn.opc, adj_insn_ptr(insn_ptr)));
return Err(VerifierError::UnknownOpCode(insn.opc, insn_ptr));
}
}

Expand All @@ -398,7 +388,7 @@ impl Verifier for RequisiteVerifier {

// insn_ptr should now be equal to number of instructions.
if insn_ptr != prog.len() / ebpf::INSN_SIZE {
return Err(VerifierError::JumpOutOfCode(adj_insn_ptr(insn_ptr), adj_insn_ptr(insn_ptr)));
return Err(VerifierError::JumpOutOfCode(insn_ptr, insn_ptr));
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion tests/execution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2881,7 +2881,7 @@ fn test_err_unresolved_syscall_reloc_64_32() {
file.read_to_end(&mut elf).unwrap();
assert_error!(
Executable::<TestContextObject>::from_elf(&elf, Arc::new(loader)),
"UnresolvedSymbol(\"log\", 68, 312)"
"UnresolvedSymbol(\"log\", 39, 312)"
);
}

Expand Down
45 changes: 20 additions & 25 deletions tests/verifier.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn test_verifier_fail() {
}

#[test]
#[should_panic(expected = "DivisionByZero(30)")]
#[should_panic(expected = "DivisionByZero(1)")]
fn test_verifier_err_div_by_zero_imm() {
let executable = assemble::<TestContextObject>(
"
Expand All @@ -114,7 +114,7 @@ fn test_verifier_err_div_by_zero_imm() {
}

#[test]
#[should_panic(expected = "UnsupportedLEBEArgument(29)")]
#[should_panic(expected = "UnsupportedLEBEArgument(0)")]
fn test_verifier_err_endian_size() {
let prog = &[
0xdc, 0x01, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, //
Expand All @@ -132,7 +132,7 @@ fn test_verifier_err_endian_size() {
}

#[test]
#[should_panic(expected = "IncompleteLDDW(29)")]
#[should_panic(expected = "IncompleteLDDW(0)")]
fn test_verifier_err_incomplete_lddw() {
// Note: ubpf has test-err-incomplete-lddw2, which is the same
let prog = &[
Expand Down Expand Up @@ -168,7 +168,7 @@ fn test_verifier_err_invalid_reg_dst() {
)
.unwrap();
let result = executable.verify::<RequisiteVerifier>();
assert_error!(result, "VerifierError(InvalidDestinationRegister(29))");
assert_error!(result, "VerifierError(InvalidDestinationRegister(0))");
}
}

Expand All @@ -191,7 +191,7 @@ fn test_verifier_err_invalid_reg_src() {
)
.unwrap();
let result = executable.verify::<RequisiteVerifier>();
assert_error!(result, "VerifierError(InvalidSourceRegister(29))");
assert_error!(result, "VerifierError(InvalidSourceRegister(0))");
}
}

Expand All @@ -215,7 +215,7 @@ fn test_verifier_resize_stack_ptr_success() {
}

#[test]
#[should_panic(expected = "JumpToMiddleOfLDDW(2, 29)")]
#[should_panic(expected = "JumpToMiddleOfLDDW(2, 0)")]
fn test_verifier_err_jmp_lddw() {
let executable = assemble::<TestContextObject>(
"
Expand Down Expand Up @@ -257,7 +257,7 @@ fn test_verifier_err_function_fallthrough() {
}

#[test]
#[should_panic(expected = "JumpOutOfCode(3, 29)")]
#[should_panic(expected = "JumpOutOfCode(3, 0)")]
fn test_verifier_err_jmp_out() {
let executable = assemble::<TestContextObject>(
"
Expand All @@ -270,7 +270,7 @@ fn test_verifier_err_jmp_out() {
}

#[test]
#[should_panic(expected = "JumpOutOfCode(18446744073709551615, 29)")]
#[should_panic(expected = "JumpOutOfCode(18446744073709551615, 0)")]
fn test_verifier_err_jmp_out_start() {
let executable = assemble::<TestContextObject>(
"
Expand All @@ -283,7 +283,7 @@ fn test_verifier_err_jmp_out_start() {
}

#[test]
#[should_panic(expected = "UnknownOpCode(6, 29)")]
#[should_panic(expected = "UnknownOpCode(6, 0)")]
fn test_verifier_err_unknown_opcode() {
let prog = &[
0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
Expand All @@ -300,7 +300,7 @@ fn test_verifier_err_unknown_opcode() {
}

#[test]
#[should_panic(expected = "CannotWriteR10(29)")]
#[should_panic(expected = "CannotWriteR10(0)")]
fn test_verifier_err_write_r10() {
let executable = assemble::<TestContextObject>(
"
Expand All @@ -317,25 +317,25 @@ fn test_verifier_err_all_shift_overflows() {
let testcases = [
// lsh32_imm
("lsh32 r0, 16", Ok(())),
("lsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 29)")),
("lsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 29)")),
("lsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 0)")),
("lsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 0)")),
// rsh32_imm
("rsh32 r0, 16", Ok(())),
("rsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 29)")),
("rsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 29)")),
("rsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 0)")),
("rsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 0)")),
// arsh32_imm
("arsh32 r0, 16", Ok(())),
("arsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 29)")),
("arsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 29)")),
("arsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 0)")),
("arsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 0)")),
// lsh64_imm
("lsh64 r0, 32", Ok(())),
("lsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 29)")),
("lsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 0)")),
// rsh64_imm
("rsh64 r0, 32", Ok(())),
("rsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 29)")),
("rsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 0)")),
// arsh64_imm
("arsh64 r0, 32", Ok(())),
("arsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 29)")),
("arsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 0)")),
];

for (overflowing_instruction, expected) in testcases {
Expand Down Expand Up @@ -377,12 +377,7 @@ fn test_sdiv_disabled() {
if enable_sbpf_v2 {
assert!(result.is_ok());
} else {
assert_error!(
result,
"VerifierError(UnknownOpCode({}, {}))",
opc,
ebpf::ELF_INSN_DUMP_OFFSET
);
assert_error!(result, "VerifierError(UnknownOpCode({}, {}))", opc, 0);
}
}
}
Expand Down

0 comments on commit ae90876

Please sign in to comment.