diff --git a/src/disassembler.rs b/src/disassembler.rs index 97edc391..fbfe2c58 100644 --- a/src/disassembler.rs +++ b/src/disassembler.rs @@ -271,7 +271,7 @@ pub fn disassemble_instruction( function_name } else { name = "syscall"; - loader.get_function_registry().lookup_by_key(insn.imm as u32).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string()).unwrap_or_else(|| "[invalid]".to_string()) + loader.get_function_registry(sbpf_version).lookup_by_key(insn.imm as u32).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string()).unwrap_or_else(|| "[invalid]".to_string()) }; desc = format!("{name} {function_name}"); }, diff --git a/src/elf.rs b/src/elf.rs index aecc47f4..640073c3 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -315,7 +315,7 @@ impl Executable { self.get_config(), self.get_sbpf_version(), self.get_function_registry(), - self.loader.get_function_registry(), + self.loader.get_function_registry(self.get_sbpf_version()), )?; Ok(()) } @@ -1071,7 +1071,10 @@ impl Executable { .entry(symbol.st_name) .or_insert_with(|| ebpf::hash_symbol_name(name)); if config.reject_broken_elfs - && loader.get_function_registry().lookup_by_key(hash).is_none() + && loader + .get_function_registry(SBPFVersion::V1) + .lookup_by_key(hash) + .is_none() { return Err(ElfError::UnresolvedSymbol( String::from_utf8_lossy(name).to_string(), diff --git a/src/interpreter.rs b/src/interpreter.rs index 34ea6dd8..9a257608 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -536,7 +536,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { }; if external { - if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry().lookup_by_key(insn.imm as u32) { + if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry(self.executable.get_sbpf_version()).lookup_by_key(insn.imm as u32) { resolved = true; self.vm.due_insn_count = self.vm.previous_instruction_meter - self.vm.due_insn_count; diff --git a/src/jit.rs b/src/jit.rs index 1f48034e..d0a17180 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -717,7 +717,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { }; if external { - if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry().lookup_by_key(insn.imm as u32) { + if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry(self.executable.get_sbpf_version()).lookup_by_key(insn.imm as u32) { self.emit_validate_and_profile_instruction_count(false, Some(0)); self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, REGISTER_SCRATCH, function as usize as i64)); self.emit_ins(X86Instruction::call_immediate(self.relative_to_anchor(ANCHOR_EXTERNAL_FUNCTION_CALL, 5))); diff --git a/src/program.rs b/src/program.rs index 470a3830..e40998a5 100644 --- a/src/program.rs +++ b/src/program.rs @@ -153,7 +153,11 @@ impl FunctionRegistry { } else { ebpf::hash_symbol_name(&usize::from(value).to_le_bytes()) }; - if loader.get_function_registry().lookup_by_key(hash).is_some() { + if loader + .get_function_registry(SBPFVersion::V1) + .lookup_by_key(hash) + .is_some() + { return Err(ElfError::SymbolHashCollision(hash)); } hash @@ -228,13 +232,17 @@ pub type BuiltinFunction = fn(*mut EbpfVm, u64, u64, u64, u64, u64); pub struct BuiltinProgram { /// Holds the Config if this is a loader program config: Option>, - /// Function pointers by symbol - functions: FunctionRegistry>, + /// Function pointers by symbol with sparse indexing + sparse_registry: FunctionRegistry>, + /// Function pointers by symbol with dense indexing + dense_registry: FunctionRegistry>, } impl PartialEq for BuiltinProgram { fn eq(&self, other: &Self) -> bool { - self.config.eq(&other.config) && self.functions.eq(&other.functions) + self.config.eq(&other.config) + && self.sparse_registry.eq(&other.sparse_registry) + && self.dense_registry.eq(&other.dense_registry) } } @@ -243,7 +251,8 @@ impl BuiltinProgram { pub fn new_loader(config: Config, functions: FunctionRegistry>) -> Self { Self { config: Some(Box::new(config)), - functions, + sparse_registry: functions, + dense_registry: FunctionRegistry::default(), } } @@ -251,7 +260,8 @@ impl BuiltinProgram { pub fn new_builtin(functions: FunctionRegistry>) -> Self { Self { config: None, - functions, + sparse_registry: functions, + dense_registry: FunctionRegistry::default(), } } @@ -259,7 +269,18 @@ impl BuiltinProgram { pub fn new_mock() -> Self { Self { config: Some(Box::default()), - functions: FunctionRegistry::default(), + sparse_registry: FunctionRegistry::default(), + dense_registry: FunctionRegistry::default(), + } + } + + /// Create a new loader with both dense and sparse function indexation + /// Use `BuiltinProgram::register_function` for registrations. + pub fn new_loader_with_dense_registration(config: Config) -> Self { + Self { + config: Some(Box::new(config)), + sparse_registry: FunctionRegistry::default(), + dense_registry: FunctionRegistry::default(), } } @@ -268,9 +289,16 @@ impl BuiltinProgram { self.config.as_ref().unwrap() } - /// Get the function registry - pub fn get_function_registry(&self) -> &FunctionRegistry> { - &self.functions + /// Get the function registry depending on the SBPF version + pub fn get_function_registry( + &self, + sbpf_version: SBPFVersion, + ) -> &FunctionRegistry> { + if sbpf_version.static_syscalls() { + &self.dense_registry + } else { + &self.sparse_registry + } } /// Calculate memory size @@ -281,18 +309,40 @@ impl BuiltinProgram { } else { 0 }) - .saturating_add(self.functions.mem_size()) + .saturating_add(self.sparse_registry.mem_size()) + .saturating_add(self.dense_registry.mem_size()) + } + + /// Register a function both in the sparse and dense registries + pub fn register_function( + &mut self, + name: &str, + dense_key: u32, + value: BuiltinFunction, + ) -> Result<(), ElfError> { + self.sparse_registry.register_function_hashed(name, value)?; + self.dense_registry + .register_function(dense_key, name, value) } } impl std::fmt::Debug for BuiltinProgram { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - writeln!(f, "{:?}", unsafe { - // `derive(Debug)` does not know that `C: ContextObject` does not need to implement `Debug` - std::mem::transmute::<&FunctionRegistry>, &FunctionRegistry>( - &self.functions, - ) - })?; + unsafe { + writeln!( + f, + "sparse: {:?}\n dense: {:?}", + // `derive(Debug)` does not know that `C: ContextObject` does not need to implement `Debug` + std::mem::transmute::< + &FunctionRegistry>, + &FunctionRegistry, + >(&self.sparse_registry,), + std::mem::transmute::< + &FunctionRegistry>, + &FunctionRegistry, + >(&self.dense_registry,) + )?; + } Ok(()) } } diff --git a/src/static_analysis.rs b/src/static_analysis.rs index aa5d0ae3..77dde3b1 100644 --- a/src/static_analysis.rs +++ b/src/static_analysis.rs @@ -6,6 +6,7 @@ use crate::{ ebpf, elf::Executable, error::EbpfError, + program::SBPFVersion, vm::{ContextObject, DynamicAnalysis, TestContextObject}, }; use rustc_demangle::demangle; @@ -192,7 +193,7 @@ impl<'a> Analysis<'a> { dfg_forward_edges: BTreeMap::new(), dfg_reverse_edges: BTreeMap::new(), }; - result.split_into_basic_blocks(false); + result.split_into_basic_blocks(false, executable.get_sbpf_version()); result.control_flow_graph_tarjan(); result.control_flow_graph_dominance_hierarchy(); result.label_basic_blocks(); @@ -223,7 +224,7 @@ impl<'a> Analysis<'a> { /// Splits the sequence of instructions into basic blocks /// /// Also links the control-flow graph edges between the basic blocks. - pub fn split_into_basic_blocks(&mut self, flatten_call_graph: bool) { + pub fn split_into_basic_blocks(&mut self, flatten_call_graph: bool, sbpf_version: SBPFVersion) { self.cfg_nodes.insert(0, CfgNode::default()); for pc in self.functions.keys() { self.cfg_nodes.entry(*pc).or_default(); @@ -236,7 +237,7 @@ impl<'a> Analysis<'a> { if let Some((function_name, _function)) = self .executable .get_loader() - .get_function_registry() + .get_function_registry(sbpf_version) .lookup_by_key(insn.imm as u32) { if function_name == b"abort" { diff --git a/src/verifier.rs b/src/verifier.rs index 3e0e6151..3cbee58b 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -73,6 +73,9 @@ pub enum VerifierError { /// Invalid function #[error("Invalid function at instruction {0}")] InvalidFunction(usize), + /// Invalid syscall + #[error("Invalid syscall code {0}")] + InvalidSyscall(u32), } /// eBPF Verifier @@ -157,6 +160,7 @@ fn check_jmp_offset( fn check_call_target( key: u32, function_registry: &FunctionRegistry, + error: VerifierError, ) -> Result<(), VerifierError> where T: Copy, @@ -165,7 +169,7 @@ where function_registry .lookup_by_key(key) .map(|_| ()) - .ok_or(VerifierError::InvalidFunction(key as usize)) + .ok_or(error) } fn check_registers( @@ -386,13 +390,23 @@ impl Verifier for RequisiteVerifier { ebpf::JSLT_REG => { check_jmp_offset(prog, insn_ptr, &function_range)?; }, 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() && insn.src != 0 => { check_call_target(insn.imm as u32, function_registry)?; }, - ebpf::CALL_IMM if sbpf_version.static_syscalls() && insn.src == 0 => { check_call_target(insn.imm as u32, syscall_registry)?; }, + ebpf::CALL_IMM if sbpf_version.static_syscalls() => { + check_call_target( + insn.imm as u32, + function_registry, + VerifierError::InvalidFunction(insn.imm as usize) + )?; + }, ebpf::CALL_IMM => {}, ebpf::CALL_REG => { check_callx_register(&insn, insn_ptr, sbpf_version)?; }, ebpf::EXIT if !sbpf_version.static_syscalls() => {}, ebpf::RETURN if sbpf_version.static_syscalls() => {}, - ebpf::SYSCALL if sbpf_version.static_syscalls() => {}, + ebpf::SYSCALL if sbpf_version.static_syscalls() => { + check_call_target( + insn.imm as u32, + syscall_registry, + VerifierError::InvalidSyscall(insn.imm as u32))?; + }, _ => { return Err(VerifierError::UnknownOpCode(insn.opc, insn_ptr)); diff --git a/tests/execution.rs b/tests/execution.rs index 1548af71..54c1be01 100644 --- a/tests/execution.rs +++ b/tests/execution.rs @@ -1999,6 +1999,10 @@ fn test_stack1() { #[test] fn test_stack2() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " stb [r10-4], 0x01 @@ -2017,6 +2021,7 @@ fn test_stack2() { syscall bpf_gather_bytes xor r0, 0x2a2a2a2a exit", + config, [], ( "bpf_mem_frob" => syscalls::SyscallMemFrob::vm, @@ -2029,6 +2034,10 @@ fn test_stack2() { #[test] fn test_string_stack() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov r1, 0x78636261 @@ -2059,6 +2068,7 @@ fn test_string_stack() { jeq r1, r6, +1 mov r0, 0x0 exit", + config, [], ( "bpf_str_cmp" => syscalls::SyscallStrCmp::vm, @@ -2367,6 +2377,10 @@ fn test_bpf_to_bpf_scratch_registers() { #[test] fn test_syscall_parameter_on_stack() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov64 r1, r10 @@ -2375,6 +2389,7 @@ fn test_syscall_parameter_on_stack() { syscall bpf_syscall_string mov64 r0, 0x0 exit", + config, [], ( "bpf_syscall_string" => syscalls::SyscallString::vm, @@ -2559,12 +2574,17 @@ fn test_call_save() { #[test] fn test_err_syscall_string() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov64 r1, 0x0 syscall bpf_syscall_string mov64 r0, 0x0 exit", + config, [72, 101, 108, 108, 111], ( "bpf_syscall_string" => syscalls::SyscallString::vm, @@ -2576,12 +2596,17 @@ fn test_err_syscall_string() { #[test] fn test_syscall_string() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov64 r2, 0x5 syscall bpf_syscall_string mov64 r0, 0x0 exit", + config, [72, 101, 108, 108, 111], ( "bpf_syscall_string" => syscalls::SyscallString::vm, @@ -2593,6 +2618,10 @@ fn test_syscall_string() { #[test] fn test_syscall() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov64 r1, 0xAA @@ -2603,6 +2632,7 @@ fn test_syscall() { syscall bpf_syscall_u64 mov64 r0, 0x0 exit", + config, [], ( "bpf_syscall_u64" => syscalls::SyscallU64::vm, @@ -2614,6 +2644,10 @@ fn test_syscall() { #[test] fn test_call_gather_bytes() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov r1, 1 @@ -2623,6 +2657,7 @@ fn test_call_gather_bytes() { mov r5, 5 syscall bpf_gather_bytes exit", + config, [], ( "bpf_gather_bytes" => syscalls::SyscallGatherBytes::vm, @@ -2634,6 +2669,10 @@ fn test_call_gather_bytes() { #[test] fn test_call_memfrob() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov r6, r1 @@ -2643,6 +2682,7 @@ fn test_call_memfrob() { ldxdw r0, [r6] be64 r0 exit", + config, [ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, // ], @@ -2684,7 +2724,11 @@ declare_builtin_function!( function_registry .register_function_hashed(*b"nested_vm_syscall", SyscallNestedVm::vm) .unwrap(); - let loader = BuiltinProgram::new_loader(Config::default(), function_registry); + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; + let loader = BuiltinProgram::new_loader(config, function_registry); let mem = [depth as u8 - 1, throw as u8]; let mut executable = assemble::( " @@ -2780,12 +2824,17 @@ fn test_tight_infinite_recursion_callx() { #[test] fn test_instruction_count_syscall() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov64 r2, 0x5 syscall bpf_syscall_string mov64 r0, 0x0 exit", + config, [72, 101, 108, 108, 111], ( "bpf_syscall_string" => syscalls::SyscallString::vm, @@ -2797,12 +2846,17 @@ fn test_instruction_count_syscall() { #[test] fn test_err_instruction_count_syscall_capped() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov64 r2, 0x5 syscall bpf_syscall_string mov64 r0, 0x0 exit", + config, [72, 101, 108, 108, 111], ( "bpf_syscall_string" => syscalls::SyscallString::vm, @@ -2822,14 +2876,11 @@ fn test_err_non_terminate_capped() { mov64 r3, 0x0 mov64 r4, 0x0 mov64 r5, r6 - syscall bpf_trace_printf add64 r6, 0x1 ja -0x8 exit", [], - ( - "bpf_trace_printf" => syscalls::SyscallTracePrintf::vm, - ), + (), TestContextObject::new(7), ProgramResult::Err(EbpfError::ExceededMaxInstructions), ); @@ -2841,14 +2892,11 @@ fn test_err_non_terminate_capped() { mov64 r3, 0x0 mov64 r4, 0x0 mov64 r5, r6 - syscall bpf_trace_printf add64 r6, 0x1 ja -0x8 exit", [], - ( - "bpf_trace_printf" => syscalls::SyscallTracePrintf::vm, - ), + (), TestContextObject::new(1000), ProgramResult::Err(EbpfError::ExceededMaxInstructions), ); @@ -2952,6 +3000,10 @@ fn test_far_jumps() { #[test] fn test_symbol_relocation() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; test_interpreter_and_jit_asm!( " mov64 r1, r10 @@ -2960,6 +3012,7 @@ fn test_symbol_relocation() { syscall bpf_syscall_string mov64 r0, 0x0 exit", + config, [72, 101, 108, 108, 111], ( "bpf_syscall_string" => syscalls::SyscallString::vm, diff --git a/tests/verifier.rs b/tests/verifier.rs index f0f54635..dc846c58 100644 --- a/tests/verifier.rs +++ b/tests/verifier.rs @@ -367,19 +367,17 @@ fn test_verifier_unknown_sycall() { #[test] fn test_verifier_known_syscall() { let prog = &[ - 0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe + 0x95, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, // syscall 2 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // return ]; - let mut function_registry = FunctionRegistry::>::default(); - function_registry - .register_function(0x6bf5c3fe, b"my_syscall", syscalls::SyscallString::vm) + + let mut loader = BuiltinProgram::new_loader_with_dense_registration(Config::default()); + loader + .register_function("my_syscall", 2, syscalls::SyscallString::vm) .unwrap(); let executable = Executable::::from_text_bytes( prog, - Arc::new(BuiltinProgram::new_loader( - Config::default(), - function_registry, - )), + Arc::new(loader), SBPFVersion::V2, FunctionRegistry::default(), ) @@ -489,7 +487,7 @@ fn return_instr() { .unwrap(); let result = executable.verify::(); if sbpf_version == SBPFVersion::V2 { - assert!(result.is_ok()); + assert_error!(result, "VerifierError(InvalidSyscall(0))"); } else { assert_error!(result, "VerifierError(UnknownOpCode(157, 2))"); }