Add new syscall instruction to the verifier

src/disassembler.rs

@@ -271,7 +271,7 @@ pub fn disassemble_instruction<C: ContextObject>(
                 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}");
         },

src/elf.rs

@@ -315,7 +315,7 @@ impl<C: ContextObject> Executable<C> {
             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<C: ContextObject> Executable<C> {
                             .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(),

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;

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)));

src/program.rs

@@ -153,7 +153,11 @@ impl<T: Copy + PartialEq> FunctionRegistry<T> {
             } 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<C> = fn(*mut EbpfVm<C>, u64, u64, u64, u64, u64);
 pub struct BuiltinProgram<C: ContextObject> {
     /// Holds the Config if this is a loader program
     config: Option<Box<Config>>,
-    /// Function pointers by symbol
-    functions: FunctionRegistry<BuiltinFunction<C>>,
+    /// Function pointers by symbol with sparse indexing
+    sparse_registry: FunctionRegistry<BuiltinFunction<C>>,
+    /// Function pointers by symbol with dense indexing
+    dense_registry: FunctionRegistry<BuiltinFunction<C>>,
 }
 
 impl<C: ContextObject> PartialEq for BuiltinProgram<C> {
     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,23 +251,36 @@ impl<C: ContextObject> BuiltinProgram<C> {
     pub fn new_loader(config: Config, functions: FunctionRegistry<BuiltinFunction<C>>) -> Self {
         Self {
             config: Some(Box::new(config)),
-            functions,
+            sparse_registry: functions,
+            dense_registry: FunctionRegistry::default(),
         }
     }
 
     /// Constructs a built-in program
     pub fn new_builtin(functions: FunctionRegistry<BuiltinFunction<C>>) -> Self {
         Self {
             config: None,
-            functions,
+            sparse_registry: functions,
+            dense_registry: FunctionRegistry::default(),
         }
     }
 
     /// Constructs a mock loader built-in program
     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<C: ContextObject> BuiltinProgram<C> {
         self.config.as_ref().unwrap()
     }
 
-    /// Get the function registry
-    pub fn get_function_registry(&self) -> &FunctionRegistry<BuiltinFunction<C>> {
-        &self.functions
+    /// Get the function registry depending on the SBPF version
+    pub fn get_function_registry(
+        &self,
+        sbpf_version: SBPFVersion,
+    ) -> &FunctionRegistry<BuiltinFunction<C>> {
+        if sbpf_version.static_syscalls() {
+            &self.dense_registry
+        } else {
+            &self.sparse_registry
+        }
     }
 
     /// Calculate memory size
@@ -281,18 +309,40 @@ impl<C: ContextObject> BuiltinProgram<C> {
             } 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<C>,
+    ) -> Result<(), ElfError> {
+        self.sparse_registry.register_function_hashed(name, value)?;
+        self.dense_registry
+            .register_function(dense_key, name, value)
     }
 }
 
 impl<C: ContextObject> std::fmt::Debug for BuiltinProgram<C> {
     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<BuiltinFunction<C>>, &FunctionRegistry<usize>>(
-                &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<BuiltinFunction<C>>,
+                    &FunctionRegistry<usize>,
+                >(&self.sparse_registry,),
+                std::mem::transmute::<
+                    &FunctionRegistry<BuiltinFunction<C>>,
+                    &FunctionRegistry<usize>,
+                >(&self.dense_registry,)
+            )?;
+        }
         Ok(())
     }
 }

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" {

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<T>(
     key: u32,
     function_registry: &FunctionRegistry<T>,
+    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));