Skip to content

Commit

Permalink
Removes Config::static_syscalls.
Browse files Browse the repository at this point in the history
  • Loading branch information
@Lichtso
Lichtso committed Jun 20, 2023
1 parent 9abf2ee commit 091a6d8
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/disassembler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub fn disassemble_instruction<C: ContextObject>(
cfg_nodes: &BTreeMap<usize, CfgNode>,
function_registry: &FunctionRegistry,
loader: &BuiltinProgram<C>,
_capabilities: &ExecutableCapabilities,
capabilities: &ExecutableCapabilities,
) -> String {
let name;
let desc;
Expand Down Expand Up @@ -252,7 +252,7 @@ pub fn disassemble_instruction<C: ContextObject>(
ebpf::JSLE_REG => { name = "jsle"; desc = jmp_reg_str(name, insn, cfg_nodes); },
ebpf::CALL_IMM => {
let mut function_name = None;
if loader.get_config().static_syscalls {
if capabilities != &ExecutableCapabilities::SBPFv1 {
if insn.src != 0 {
function_name = Some(resolve_label(cfg_nodes, insn.imm as usize));
}
Expand Down
35 changes: 9 additions & 26 deletions src/elf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,13 @@ pub fn hash_internal_function(pc: usize, name: &[u8]) -> u32 {
pub fn register_internal_function<C: ContextObject>(
function_registry: &mut FunctionRegistry,
loader: &BuiltinProgram<C>,
_capabilities: &ExecutableCapabilities,
capabilities: &ExecutableCapabilities,
pc: usize,
name: &[u8],
) -> Result<u32, ElfError> {
let config = loader.get_config();
let key = if config.static_syscalls {
// With static_syscalls normal function calls and syscalls are differentiated in the ISA.
let key = if capabilities != &ExecutableCapabilities::SBPFv1 {
// In SBPFv2 normal function calls and syscalls are differentiated in the ISA.
// Thus, we don't need to hash them here anymore and collisions are gone as well.
pc as u32
} else {
Expand Down Expand Up @@ -529,7 +529,7 @@ impl<V: Verifier, C: ContextObject> Executable<V, C> {
return Err(ElfError::InvalidEntrypoint);
}
let entry_pc = if let Some(entry_pc) = (offset as usize).checked_div(ebpf::INSN_SIZE) {
if !config.static_syscalls {
if capabilities == ExecutableCapabilities::SBPFv1 {
function_registry.remove(&ebpf::hash_symbol_name(b"entrypoint"));
}
register_internal_function(
Expand Down Expand Up @@ -619,7 +619,7 @@ impl<V: Verifier, C: ContextObject> Executable<V, C> {
let mut insn = ebpf::get_insn(elf_bytes, i);
if insn.opc == ebpf::CALL_IMM
&& insn.imm != -1
&& !(config.static_syscalls && insn.src == 0)
&& !(capabilities != &ExecutableCapabilities::SBPFv1 && insn.src == 0)
{
let target_pc = (i as isize)
.saturating_add(1)
Expand Down Expand Up @@ -1462,7 +1462,6 @@ mod test {
fn test_fixup_relative_calls_back() {
let mut function_registry = FunctionRegistry::default();
let loader = BuiltinProgram::new_loader(Config {
static_syscalls: false,
enable_symbol_and_section_labels: true,
..Config::default()
});
Expand All @@ -1480,7 +1479,7 @@ mod test {
ElfExecutable::fixup_relative_calls(
&mut function_registry,
&loader,
&ExecutableCapabilities::SBPFv2,
&ExecutableCapabilities::SBPFv1,
&mut prog,
)
.unwrap();
Expand All @@ -1502,7 +1501,7 @@ mod test {
ElfExecutable::fixup_relative_calls(
&mut function_registry,
&loader,
&ExecutableCapabilities::SBPFv2,
&ExecutableCapabilities::SBPFv1,
&mut prog,
)
.unwrap();
Expand All @@ -1523,7 +1522,6 @@ mod test {
fn test_fixup_relative_calls_forward() {
let mut function_registry = FunctionRegistry::default();
let loader = BuiltinProgram::new_loader(Config {
static_syscalls: false,
enable_symbol_and_section_labels: true,
..Config::default()
});
Expand All @@ -1541,7 +1539,7 @@ mod test {
ElfExecutable::fixup_relative_calls(
&mut function_registry,
&loader,
&ExecutableCapabilities::SBPFv2,
&ExecutableCapabilities::SBPFv1,
&mut prog,
)
.unwrap();
Expand All @@ -1563,7 +1561,7 @@ mod test {
ElfExecutable::fixup_relative_calls(
&mut function_registry,
&loader,
&ExecutableCapabilities::SBPFv2,
&ExecutableCapabilities::SBPFv1,
&mut prog,
)
.unwrap();
Expand Down Expand Up @@ -2274,21 +2272,6 @@ mod test {
ElfExecutable::load(&elf_bytes, loader()).expect("validation failed");
}

#[test]
#[should_panic(expected = r#"validation failed: RelativeJumpOutOfBounds(29)"#)]
fn test_static_syscall_disabled() {
let loader = BuiltinProgram::new_loader(Config {
static_syscalls: false,
..Config::default()
});
let elf_bytes =
std::fs::read("tests/elfs/syscall_static_unknown.so").expect("failed to read elf file");

// when config.static_syscalls=false, all CALL_IMMs are treated as relative
// calls for backwards compatibility
ElfExecutable::load(&elf_bytes, Arc::new(loader)).expect("validation failed");
}

#[test]
#[should_panic(expected = "validation failed: InvalidProgramHeader")]
fn test_program_headers_overflow() {
Expand Down
8 changes: 4 additions & 4 deletions src/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
//! Interpreter for eBPF programs.

use crate::{
ebpf,
ebpf::STACK_PTR_REG,
ebpf::{self, STACK_PTR_REG},
elf::ExecutableCapabilities,
error::EbpfError,
verifier::Verifier,
vm::{Config, ContextObject, EbpfVm, ProgramResult},
Expand Down Expand Up @@ -420,7 +420,7 @@ impl<'a, 'b, V: Verifier, C: ContextObject> Interpreter<'a, 'b, V, C> {
if !self.check_pc(pc) {
return false;
}
if config.static_syscalls && self.vm.executable.lookup_internal_function(self.pc as u32).is_none() {
if self.vm.executable.get_capabilities() != &ExecutableCapabilities::SBPFv1 && self.vm.executable.lookup_internal_function(self.pc as u32).is_none() {
self.due_insn_count += 1;
throw_error!(self, EbpfError::UnsupportedInstruction(self.pc + ebpf::ELF_INSN_DUMP_OFFSET));
}
Expand All @@ -430,7 +430,7 @@ impl<'a, 'b, V: Verifier, C: ContextObject> Interpreter<'a, 'b, V, C> {
// changed after the program has been verified.
ebpf::CALL_IMM => {
let mut resolved = false;
let (external, internal) = if config.static_syscalls {
let (external, internal) = if self.vm.executable.get_capabilities() != &ExecutableCapabilities::SBPFv1 {
(insn.src == 0, insn.src != 0)
} else {
(true, true)
Expand Down
7 changes: 3 additions & 4 deletions src/jit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::{fmt::Debug, mem, ptr};

use crate::{
ebpf::{self, FIRST_SCRATCH_REG, FRAME_PTR_REG, INSN_SIZE, SCRATCH_REGS, STACK_PTR_REG},
elf::Executable,
elf::{Executable, ExecutableCapabilities},
error::EbpfError,
memory_management::{
allocate_pages, free_pages, get_system_page_size, protect_pages, round_to_page_size,
Expand Down Expand Up @@ -592,7 +592,7 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> {
// For JIT, external functions MUST be registered at compile time.

let mut resolved = false;
let (external, internal) = if self.config.static_syscalls {
let (external, internal) = if self.executable.get_capabilities() != &ExecutableCapabilities::SBPFv1 {
(insn.src == 0, insn.src != 0)
} else {
(true, true)
Expand Down Expand Up @@ -1529,7 +1529,7 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> {
}
// There is no `VerifierError::JumpToMiddleOfLDDW` for `call imm` so patch it here
let call_unsupported_instruction = self.anchors[ANCHOR_CALL_UNSUPPORTED_INSTRUCTION] as usize;
if self.config.static_syscalls {
if self.executable.get_capabilities() != &ExecutableCapabilities::SBPFv1 {
let mut prev_pc = 0;
for current_pc in self.executable.get_function_registry().keys() {
if *current_pc as usize >= self.result.pc_section.len() {
Expand All @@ -1551,7 +1551,6 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> {
mod tests {
use super::*;
use crate::{
elf::ExecutableCapabilities,
syscalls,
verifier::TautologyVerifier,
vm::{BuiltinProgram, FunctionRegistry, TestContextObject},
Expand Down
4 changes: 2 additions & 2 deletions src/verifier.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ impl Verifier for RequisiteVerifier {
let insn = ebpf::get_insn(prog, insn_ptr);
let mut store = false;

if config.static_syscalls && function_iter.peek() == Some(&insn_ptr) {
if capabilities != &ExecutableCapabilities::SBPFv1 && function_iter.peek() == Some(&insn_ptr) {
function_range.start = function_iter.next().unwrap_or(0);
function_range.end = *function_iter.peek().unwrap_or(&program_range.end);
if insn.opc == 0 {
Expand Down Expand Up @@ -361,7 +361,7 @@ 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 config.static_syscalls && insn.src != 0 => { check_jmp_offset(prog, insn_ptr, &program_range)?; },
ebpf::CALL_IMM if capabilities != &ExecutableCapabilities::SBPFv1 && insn.src != 0 => { check_jmp_offset(prog, insn_ptr, &program_range)?; },
ebpf::CALL_IMM => {},
ebpf::CALL_REG => { check_imm_register(&insn, insn_ptr, config)?; },
ebpf::EXIT => {},
Expand Down
3 changes: 0 additions & 3 deletions src/vm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,6 @@ pub struct Config {
pub dynamic_stack_frames: bool,
/// Avoid copying read only sections when possible
pub optimize_rodata: bool,
/// Support syscalls via pseudo calls (insn.src = 0)
pub static_syscalls: bool,
/// Use the new ELF parser
pub new_elf_parser: bool,
/// Use aligned memory mapping
Expand Down Expand Up @@ -271,7 +269,6 @@ impl Default for Config {
reject_callx_r10: true,
dynamic_stack_frames: true,
optimize_rodata: true,
static_syscalls: true,
new_elf_parser: true,
aligned_memory_mapping: true,
enable_sbpf_v1: true,
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 @@ -2749,7 +2749,7 @@ fn test_err_static_jmp_lddw() {
#[test]
fn test_err_dynamic_jmp_lddw() {
let config = Config {
static_syscalls: false,
enable_sbpf_v2: false,
..Config::default()
};
test_interpreter_and_jit_asm!(
Expand Down

0 comments on commit 091a6d8

Please sign in to comment.