diff --git a/src/elf.rs b/src/elf.rs index 9a4380b6..c45dc965 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -632,56 +632,6 @@ impl Executable { // Functions exposed for tests - /// Fix-ups relative calls - pub fn fixup_relative_calls( - function_registry: &mut FunctionRegistry, - loader: &BuiltinProgram, - sbpf_version: &SBPFVersion, - elf_bytes: &mut [u8], - ) -> Result<(), ElfError> { - let config = loader.get_config(); - let instruction_count = elf_bytes - .len() - .checked_div(ebpf::INSN_SIZE) - .ok_or(ElfError::ValueOutOfBounds)?; - for i in 0..instruction_count { - let mut insn = ebpf::get_insn(elf_bytes, i); - if insn.opc == ebpf::CALL_IMM - && insn.imm != -1 - && !(sbpf_version.static_syscalls() && insn.src == 0) - { - let target_pc = (i as isize) - .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), - )); - } - let name = if config.enable_symbol_and_section_labels { - format!("function_{target_pc}") - } else { - String::default() - }; - - let key = register_internal_function( - function_registry, - loader, - sbpf_version, - target_pc as usize, - name.as_bytes(), - )?; - insn.imm = key as i64; - let offset = i.saturating_mul(ebpf::INSN_SIZE); - let checked_slice = elf_bytes - .get_mut(offset..offset.saturating_add(ebpf::INSN_SIZE)) - .ok_or(ElfError::ValueOutOfBounds)?; - checked_slice.copy_from_slice(&insn.to_array()); - } - } - Ok(()) - } - /// Validates the ELF pub fn validate<'a, P: ElfParser<'a>>( config: &Config, @@ -990,16 +940,48 @@ impl Executable { }; // Fixup all program counter relative call instructions - Self::fixup_relative_calls( - function_registry, - loader, - &sbpf_version, - elf_bytes - .get_mut(text_section.file_range().unwrap_or_default()) - .ok_or(ElfError::ValueOutOfBounds)?, - )?; - let config = loader.get_config(); + let text_bytes = elf_bytes + .get_mut(text_section.file_range().unwrap_or_default()) + .ok_or(ElfError::ValueOutOfBounds)?; + let instruction_count = text_bytes + .len() + .checked_div(ebpf::INSN_SIZE) + .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) + { + let target_pc = (i as isize) + .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), + )); + } + let name = if config.enable_symbol_and_section_labels { + format!("function_{target_pc}") + } else { + String::default() + }; + let key = register_internal_function( + function_registry, + loader, + &sbpf_version, + target_pc as usize, + name.as_bytes(), + )?; + 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); + } + } + let mut program_header: Option<&

>::ProgramHeader> = None; // Fixup all the relocations in the relocation section if exists @@ -1505,202 +1487,6 @@ mod test { assert_eq!(0, executable.get_entrypoint_instruction_offset()); } - #[test] - fn test_fixup_relative_calls_back() { - let mut function_registry = FunctionRegistry::default(); - let loader = BuiltinProgram::new_loader(Config { - enable_symbol_and_section_labels: true, - ..Config::default() - }); - - // call -2 - #[rustfmt::skip] - let mut prog = vec![ - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x85, 0x10, 0x00, 0x00, 0xfe, 0xff, 0xff, 0xff]; - - ElfExecutable::fixup_relative_calls( - &mut function_registry, - &loader, - &SBPFVersion::V1, - &mut prog, - ) - .unwrap(); - let name = "function_4".to_string(); - let hash = hash_internal_function(4, name.as_bytes()); - let insn = ebpf::Insn { - opc: 0x85, - dst: 0, - src: 1, - imm: hash as i64, - ..ebpf::Insn::default() - }; - assert_eq!(insn.to_array(), prog[40..]); - assert_eq!(*function_registry.get(&hash).unwrap(), (4, name)); - - // call +6 - let mut function_registry = FunctionRegistry::default(); - prog.splice(44.., vec![0xfa, 0xff, 0xff, 0xff]); - ElfExecutable::fixup_relative_calls( - &mut function_registry, - &loader, - &SBPFVersion::V1, - &mut prog, - ) - .unwrap(); - let name = "function_0".to_string(); - let hash = hash_internal_function(0, name.as_bytes()); - let insn = ebpf::Insn { - opc: 0x85, - dst: 0, - src: 1, - imm: hash as i64, - ..ebpf::Insn::default() - }; - assert_eq!(insn.to_array(), prog[40..]); - assert_eq!(*function_registry.get(&hash).unwrap(), (0, name)); - } - - #[test] - fn test_fixup_relative_calls_forward() { - let mut function_registry = FunctionRegistry::default(); - let loader = BuiltinProgram::new_loader(Config { - enable_symbol_and_section_labels: true, - ..Config::default() - }); - - // call +0 - #[rustfmt::skip] - let mut prog = vec![ - 0x85, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; - - ElfExecutable::fixup_relative_calls( - &mut function_registry, - &loader, - &SBPFVersion::V1, - &mut prog, - ) - .unwrap(); - let name = "function_1".to_string(); - let hash = hash_internal_function(1, name.as_bytes()); - let insn = ebpf::Insn { - opc: 0x85, - dst: 0, - src: 1, - imm: hash as i64, - ..ebpf::Insn::default() - }; - assert_eq!(insn.to_array(), prog[..8]); - assert_eq!(*function_registry.get(&hash).unwrap(), (1, name)); - - // call +4 - let mut function_registry = FunctionRegistry::default(); - prog.splice(4..8, vec![0x04, 0x00, 0x00, 0x00]); - ElfExecutable::fixup_relative_calls( - &mut function_registry, - &loader, - &SBPFVersion::V1, - &mut prog, - ) - .unwrap(); - let name = "function_5".to_string(); - let hash = hash_internal_function(5, name.as_bytes()); - let insn = ebpf::Insn { - opc: 0x85, - dst: 0, - src: 1, - imm: hash as i64, - ..ebpf::Insn::default() - }; - assert_eq!(insn.to_array(), prog[..8]); - assert_eq!(*function_registry.get(&hash).unwrap(), (5, name)); - } - - #[test] - #[should_panic( - expected = "called `Result::unwrap()` on an `Err` value: RelativeJumpOutOfBounds(29)" - )] - fn test_fixup_relative_calls_out_of_bounds_forward() { - let mut function_registry = FunctionRegistry::default(); - let loader = loader(); - - // call +5 - #[rustfmt::skip] - let mut prog = vec![ - 0x85, 0x10, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; - - ElfExecutable::fixup_relative_calls( - &mut function_registry, - &loader, - &SBPFVersion::V2, - &mut prog, - ) - .unwrap(); - let name = "function_1".to_string(); - let hash = hash_internal_function(1, name.as_bytes()); - let insn = ebpf::Insn { - opc: 0x85, - dst: 0, - src: 1, - imm: hash as i64, - ..ebpf::Insn::default() - }; - assert_eq!(insn.to_array(), prog[..8]); - assert_eq!(*function_registry.get(&hash).unwrap(), (1, name)); - } - - #[test] - #[should_panic( - expected = "called `Result::unwrap()` on an `Err` value: RelativeJumpOutOfBounds(34)" - )] - fn test_fixup_relative_calls_out_of_bounds_back() { - let mut function_registry = FunctionRegistry::default(); - let loader = loader(); - - // call -7 - #[rustfmt::skip] - let mut prog = vec![ - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x85, 0x10, 0x00, 0x00, 0xf9, 0xff, 0xff, 0xff]; - - ElfExecutable::fixup_relative_calls( - &mut function_registry, - &loader, - &SBPFVersion::V2, - &mut prog, - ) - .unwrap(); - let name = "function_4".to_string(); - let hash = hash_internal_function(4, name.as_bytes()); - let insn = ebpf::Insn { - opc: 0x85, - dst: 0, - src: 1, - imm: hash as i64, - ..ebpf::Insn::default() - }; - assert_eq!(insn.to_array(), prog[40..]); - assert_eq!(*function_registry.get(&hash).unwrap(), (4, name)); - } - #[test] #[ignore] fn test_fuzz_load() { @@ -2280,4 +2066,22 @@ mod test { .expect("failed to read elf file"); ElfExecutable::load(&elf_bytes, loader()).expect("validation failed"); } + + #[test] + #[should_panic(expected = "validation failed: RelativeJumpOutOfBounds(43)")] + fn test_relative_call_oob_forward() { + let mut elf_bytes = + std::fs::read("tests/elfs/relative_call_sbpfv1.so").expect("failed to read elf file"); + LittleEndian::write_i32(&mut elf_bytes[0x1074..0x1078], 11); + ElfExecutable::load(&elf_bytes, loader()).expect("validation failed"); + } + + #[test] + #[should_panic(expected = "validation failed: RelativeJumpOutOfBounds(43)")] + fn test_relative_call_oob_backward() { + let mut elf_bytes = + std::fs::read("tests/elfs/relative_call_sbpfv1.so").expect("failed to read elf file"); + LittleEndian::write_i32(&mut elf_bytes[0x1074..0x1078], -16i32); + ElfExecutable::load(&elf_bytes, loader()).expect("validation failed"); + } } diff --git a/tests/elfs/elfs.sh b/tests/elfs/elfs.sh index 0acfe204..b03d7c27 100755 --- a/tests/elfs/elfs.sh +++ b/tests/elfs/elfs.sh @@ -33,10 +33,6 @@ rm unresolved_syscall.o rm entrypoint.o rm multiple_file.o -"$LLVM_DIR"clang $CC_FLAGS -o relative_call.o -c relative_call.c -"$LLVM_DIR"ld.lld $LD_FLAGS -o relative_call.so relative_call.o -rm relative_call.o - "$LLVM_DIR"clang $CC_FLAGS_V1 -o reloc_64_64.o -c reloc_64_64.c "$LLVM_DIR"ld.lld $LD_FLAGS_V1 -o reloc_64_64_sbpfv1.so reloc_64_64.o rm reloc_64_64.o diff --git a/tests/elfs/relative_call.c b/tests/elfs/relative_call.c index 4e47e3c2..d495557b 100644 --- a/tests/elfs/relative_call.c +++ b/tests/elfs/relative_call.c @@ -2,8 +2,11 @@ * @brief test program that generates BPF PC relative call instructions */ -#include "syscalls.h" +typedef unsigned char uint8_t; +typedef unsigned long int uint64_t; +extern void log(const char*, uint64_t); +uint64_t function_bar(uint64_t x); uint64_t __attribute__ ((noinline)) function_foo(uint64_t x) { log(__func__, sizeof(__func__)); return x + 1; @@ -13,6 +16,11 @@ extern uint64_t entrypoint(const uint8_t *input) { uint64_t x = (uint64_t)*input; log(__func__, sizeof(__func__)); x = function_foo(x); + x = function_bar(x); return x; } +uint64_t __attribute__ ((noinline)) function_bar(uint64_t x) { + log(__func__, sizeof(__func__)); + return x + 2; +} diff --git a/tests/elfs/relative_call.so b/tests/elfs/relative_call.so deleted file mode 100755 index 9adff4bb..00000000 Binary files a/tests/elfs/relative_call.so and /dev/null differ diff --git a/tests/elfs/relative_call_sbpfv1.so b/tests/elfs/relative_call_sbpfv1.so new file mode 100755 index 00000000..f9a46758 Binary files /dev/null and b/tests/elfs/relative_call_sbpfv1.so differ diff --git a/tests/execution.rs b/tests/execution.rs index 607bc893..75181098 100644 --- a/tests/execution.rs +++ b/tests/execution.rs @@ -2592,13 +2592,13 @@ fn test_err_mem_access_out_of_bound() { #[test] fn test_relative_call() { test_interpreter_and_jit_elf!( - "tests/elfs/relative_call.so", + "tests/elfs/relative_call_sbpfv1.so", [1], ( "log" => syscalls::bpf_syscall_string, ), - TestContextObject::new(14), - ProgramResult::Ok(2), + TestContextObject::new(23), + ProgramResult::Ok(4), ); }