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.

Sign up for GitHub

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

Cleanup - Removes ELF_INSN_DUMP_OFFSET #541

Merged
merged 1 commit into from
Oct 10, 2023
Merged

Cleanup - Removes ELF_INSN_DUMP_OFFSET #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions src/ebpf.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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