From 7fc5d3e640d9e7ec46d3df6a990b3c2151611519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 30 Sep 2023 08:39:18 +0200 Subject: [PATCH 1/7] Replaces all context parameters of BuiltinFunction with a reference to the vm. --- src/interpreter.rs | 4 +--- src/jit.rs | 4 +--- src/program.rs | 21 ++++++++------------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index b953a701..e84524e1 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -481,14 +481,12 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { } self.due_insn_count = 0; function( - self.vm.context_object_pointer, + &mut self.vm, self.reg[1], self.reg[2], self.reg[3], self.reg[4], self.reg[5], - &mut self.vm.memory_mapping, - &mut self.vm.program_result, ); self.reg[0] = match &self.vm.program_result { ProgramResult::Ok(value) => *value, diff --git a/src/jit.rs b/src/jit.rs index b9dc648f..a831203b 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -1407,14 +1407,12 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { ], None); } self.emit_rust_call(Value::Register(REGISTER_SCRATCH), &[ - Argument { index: 7, value: Value::RegisterPlusConstant32(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::ProgramResult), false) }, - Argument { index: 6, value: Value::RegisterPlusConstant32(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::MemoryMapping), false) }, Argument { index: 5, value: Value::Register(ARGUMENT_REGISTERS[5]) }, Argument { index: 4, value: Value::Register(ARGUMENT_REGISTERS[4]) }, Argument { index: 3, value: Value::Register(ARGUMENT_REGISTERS[3]) }, Argument { index: 2, value: Value::Register(ARGUMENT_REGISTERS[2]) }, Argument { index: 1, value: Value::Register(ARGUMENT_REGISTERS[1]) }, - Argument { index: 0, value: Value::RegisterIndirect(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::ContextObjectPointer), false) }, + Argument { index: 0, value: Value::RegisterPlusConstant32(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::HostStackPointer), false) }, ], None); if self.config.enable_instruction_meter { self.emit_rust_call(Value::Constant64(C::get_remaining as *const u8 as i64, false), &[ diff --git a/src/program.rs b/src/program.rs index c2c343e1..f85788dc 100644 --- a/src/program.rs +++ b/src/program.rs @@ -3,8 +3,7 @@ use { crate::{ ebpf, elf::ElfError, - memory_region::MemoryMapping, - vm::{Config, ContextObject, ProgramResult}, + vm::{Config, ContextObject, EbpfVm}, }, std::collections::{btree_map::Entry, BTreeMap}, }; @@ -210,8 +209,7 @@ impl FunctionRegistry { } /// Syscall function without context -pub type BuiltinFunction = - fn(&mut C, u64, u64, u64, u64, u64, &mut MemoryMapping, &mut ProgramResult); +pub type BuiltinFunction = fn(&mut EbpfVm, u64, u64, u64, u64, u64); /// Represents the interface to a fixed functionality program #[derive(Eq)] @@ -279,10 +277,9 @@ 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) + std::mem::transmute::<&FunctionRegistry>, &FunctionRegistry>( + &self.functions, + ) })?; Ok(()) } @@ -300,19 +297,17 @@ macro_rules! declare_builtin_function { /// VM interface #[allow(clippy::too_many_arguments)] pub fn vm( - context_object: &mut TestContextObject, + vm: &mut $crate::vm::EbpfVm, arg_a: u64, arg_b: u64, arg_c: u64, arg_d: u64, arg_e: u64, - memory_mapping: &mut $crate::memory_region::MemoryMapping, - program_result: &mut $crate::vm::ProgramResult, ) { let converted_result: $crate::vm::ProgramResult = Self::rust( - context_object, arg_a, arg_b, arg_c, arg_d, arg_e, memory_mapping, + vm.context_object_pointer, arg_a, arg_b, arg_c, arg_d, arg_e, &mut vm.memory_mapping, ).into(); - *program_result = converted_result; + vm.program_result = converted_result; } } }; From 59d3a7923b3f2d01bda6b87f4576ad92f0102106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 30 Sep 2023 13:44:10 +0200 Subject: [PATCH 2/7] Deduplicates instruction metering of interpreter and JIT by moving it into declare_builtin_function. --- src/interpreter.rs | 7 +------ src/jit.rs | 13 ++----------- src/program.rs | 8 ++++++++ 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index e84524e1..6abadb9e 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -476,9 +476,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry().lookup_by_key(insn.imm as u32) { resolved = true; - if config.enable_instruction_meter { - self.vm.context_object_pointer.consume(self.due_insn_count); - } + self.vm.previous_instruction_meter = self.due_insn_count; self.due_insn_count = 0; function( &mut self.vm, @@ -492,9 +490,6 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { ProgramResult::Ok(value) => *value, ProgramResult::Err(_err) => return false, }; - if config.enable_instruction_meter { - self.vm.previous_instruction_meter = self.vm.context_object_pointer.get_remaining(); - } } } diff --git a/src/jit.rs b/src/jit.rs index a831203b..0cffcc8a 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -1398,13 +1398,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { self.set_anchor(ANCHOR_EXTERNAL_FUNCTION_CALL); self.emit_ins(X86Instruction::push_immediate(OperandSize::S64, -1)); // Used as PC value in error case, acts as stack padding otherwise if self.config.enable_instruction_meter { - // REGISTER_INSTRUCTION_METER = *PreviousInstructionMeter - REGISTER_INSTRUCTION_METER; - self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x2B, REGISTER_INSTRUCTION_METER, REGISTER_PTR_TO_VM, 0, Some(X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::PreviousInstructionMeter))))); // REGISTER_INSTRUCTION_METER -= *PreviousInstructionMeter; - self.emit_ins(X86Instruction::alu(OperandSize::S64, 0xf7, 3, REGISTER_INSTRUCTION_METER, 0, None)); // REGISTER_INSTRUCTION_METER = -REGISTER_INSTRUCTION_METER; - self.emit_rust_call(Value::Constant64(C::consume as *const u8 as i64, false), &[ - Argument { index: 1, value: Value::Register(REGISTER_INSTRUCTION_METER) }, - Argument { index: 0, value: Value::RegisterIndirect(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::ContextObjectPointer), false) }, - ], None); + self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x29, REGISTER_INSTRUCTION_METER, REGISTER_PTR_TO_VM, 0, Some(X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::PreviousInstructionMeter))))); // *PreviousInstructionMeter -= REGISTER_INSTRUCTION_METER; } self.emit_rust_call(Value::Register(REGISTER_SCRATCH), &[ Argument { index: 5, value: Value::Register(ARGUMENT_REGISTERS[5]) }, @@ -1415,10 +1409,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { Argument { index: 0, value: Value::RegisterPlusConstant32(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::HostStackPointer), false) }, ], None); if self.config.enable_instruction_meter { - self.emit_rust_call(Value::Constant64(C::get_remaining as *const u8 as i64, false), &[ - Argument { index: 0, value: Value::RegisterIndirect(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::ContextObjectPointer), false) }, - ], Some(REGISTER_INSTRUCTION_METER)); - self.emit_ins(X86Instruction::store(OperandSize::S64, REGISTER_INSTRUCTION_METER, REGISTER_PTR_TO_VM, X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::PreviousInstructionMeter)))); // *PreviousInstructionMeter = REGISTER_INSTRUCTION_METER; + self.emit_ins(X86Instruction::load(OperandSize::S64, REGISTER_PTR_TO_VM, REGISTER_INSTRUCTION_METER, X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::PreviousInstructionMeter)))); // REGISTER_INSTRUCTION_METER = *PreviousInstructionMeter; } // Test if result indicates that an error occured diff --git a/src/program.rs b/src/program.rs index f85788dc..cb3972e1 100644 --- a/src/program.rs +++ b/src/program.rs @@ -304,10 +304,18 @@ macro_rules! declare_builtin_function { arg_d: u64, arg_e: u64, ) { + use $crate::vm::ContextObject; + let config = vm.loader.get_config(); + if config.enable_instruction_meter { + vm.context_object_pointer.consume(vm.previous_instruction_meter); + } let converted_result: $crate::vm::ProgramResult = Self::rust( vm.context_object_pointer, arg_a, arg_b, arg_c, arg_d, arg_e, &mut vm.memory_mapping, ).into(); vm.program_result = converted_result; + if config.enable_instruction_meter { + vm.previous_instruction_meter = vm.context_object_pointer.get_remaining(); + } } } }; From 2bb84b6f4b54a72b020888da2c84b668cb2ef437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 30 Sep 2023 08:41:23 +0200 Subject: [PATCH 3/7] Moves get_runtime_environment_key() from JIT into VM. --- fuzz/fuzz_targets/common.rs | 4 ---- src/jit.rs | 18 +++--------------- src/vm.rs | 14 +++++++++++--- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/fuzz/fuzz_targets/common.rs b/fuzz/fuzz_targets/common.rs index 7e14bee1..f2f3e7e9 100644 --- a/fuzz/fuzz_targets/common.rs +++ b/fuzz/fuzz_targets/common.rs @@ -12,7 +12,6 @@ pub struct ConfigTemplate { enable_stack_frame_gaps: bool, enable_symbol_and_section_labels: bool, sanitize_user_provided_values: bool, - encrypt_runtime_environment: bool, reject_callx_r10: bool, optimize_rodata: bool, } @@ -27,7 +26,6 @@ impl<'a> Arbitrary<'a> for ConfigTemplate { enable_stack_frame_gaps: bools & (1 << 0) != 0, enable_symbol_and_section_labels: bools & (1 << 1) != 0, sanitize_user_provided_values: bools & (1 << 3) != 0, - encrypt_runtime_environment: bools & (1 << 4) != 0, reject_callx_r10: bools & (1 << 6) != 0, optimize_rodata: bools & (1 << 9) != 0, }) @@ -51,7 +49,6 @@ impl From for Config { enable_stack_frame_gaps, enable_symbol_and_section_labels, sanitize_user_provided_values, - encrypt_runtime_environment, reject_callx_r10, optimize_rodata, } => Config { @@ -61,7 +58,6 @@ impl From for Config { enable_symbol_and_section_labels, noop_instruction_rate, sanitize_user_provided_values, - encrypt_runtime_environment, reject_callx_r10, optimize_rodata, ..Default::default() diff --git a/src/jit.rs b/src/jit.rs index 0cffcc8a..f4bd641a 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -21,20 +21,14 @@ use crate::{ allocate_pages, free_pages, get_system_page_size, protect_pages, round_to_page_size, }, memory_region::{AccessType, MemoryMapping}, - vm::{Config, ContextObject, EbpfVm, ProgramResult}, + vm::{get_runtime_environment_key, Config, ContextObject, EbpfVm, ProgramResult}, x86::*, }; -/// Shift the RUNTIME_ENVIRONMENT_KEY by this many bits to the LSB -/// -/// 3 bits for 8 Byte alignment, and 1 bit to have encoding space for the RuntimeEnvironment. -const PROGRAM_ENVIRONMENT_KEY_SHIFT: u32 = 4; const MAX_EMPTY_PROGRAM_MACHINE_CODE_LENGTH: usize = 4096; const MAX_MACHINE_CODE_LENGTH_PER_INSTRUCTION: usize = 110; const MACHINE_CODE_PER_INSTRUCTION_METER_CHECKPOINT: usize = 13; -static RUNTIME_ENVIRONMENT_KEY: std::sync::OnceLock = std::sync::OnceLock::::new(); - pub struct JitProgram { /// OS page size in bytes and the alignment of the sections page_size: usize, @@ -129,7 +123,7 @@ impl JitProgram { "pop rbx", host_stack_pointer = in(reg) &mut vm.host_stack_pointer, inlateout("rax") instruction_meter, - inlateout("rdi") (vm as *mut _ as *mut u64).offset(*RUNTIME_ENVIRONMENT_KEY.get().unwrap() as isize) => _, + inlateout("rdi") (vm as *mut _ as *mut u64).offset(get_runtime_environment_key() as isize) => _, inlateout("r10") self.pc_section[registers[11] as usize] => _, inlateout("r11") ®isters => _, lateout("rsi") _, lateout("rdx") _, lateout("rcx") _, lateout("r8") _, @@ -357,13 +351,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { code_length_estimate += pc / config.instruction_meter_checkpoint_distance * MACHINE_CODE_PER_INSTRUCTION_METER_CHECKPOINT; } - let runtime_environment_key = *RUNTIME_ENVIRONMENT_KEY.get_or_init(|| { - if config.encrypt_runtime_environment { - rand::thread_rng().gen::() >> PROGRAM_ENVIRONMENT_KEY_SHIFT - } else { - 0 - } - }); + let runtime_environment_key = get_runtime_environment_key(); let mut diversification_rng = SmallRng::from_rng(rand::thread_rng()).map_err(|_| EbpfError::JitNotCompiled)?; Ok(Self { diff --git a/src/vm.rs b/src/vm.rs index 365239a2..b59e2eaf 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -21,8 +21,19 @@ use crate::{ program::{BuiltinProgram, FunctionRegistry, SBPFVersion}, static_analysis::{Analysis, TraceLogEntry}, }; +use rand::Rng; use std::{collections::BTreeMap, fmt::Debug, sync::Arc}; +/// Shift the RUNTIME_ENVIRONMENT_KEY by this many bits to the LSB +/// +/// 3 bits for 8 Byte alignment, and 1 bit to have encoding space for the RuntimeEnvironment. +const PROGRAM_ENVIRONMENT_KEY_SHIFT: u32 = 4; +static RUNTIME_ENVIRONMENT_KEY: std::sync::OnceLock = std::sync::OnceLock::::new(); +pub(crate) fn get_runtime_environment_key() -> i32 { + *RUNTIME_ENVIRONMENT_KEY + .get_or_init(|| rand::thread_rng().gen::() >> PROGRAM_ENVIRONMENT_KEY_SHIFT) +} + /// Same as `Result` but provides a stable memory layout #[derive(Debug)] #[repr(C, u64)] @@ -125,8 +136,6 @@ pub struct Config { pub noop_instruction_rate: u32, /// Enable disinfection of immediate values and offsets provided by the user in JIT pub sanitize_user_provided_values: bool, - /// Encrypt the runtime environment in JIT - pub encrypt_runtime_environment: bool, /// Throw ElfError::SymbolHashCollision when a BPF function collides with a registered syscall pub external_internal_function_hash_collision: bool, /// Have the verifier reject "callx r10" @@ -164,7 +173,6 @@ impl Default for Config { reject_broken_elfs: false, noop_instruction_rate: 256, sanitize_user_provided_values: true, - encrypt_runtime_environment: true, external_internal_function_hash_collision: true, reject_callx_r10: true, optimize_rodata: true, From d9e33a1e0a57916fdd141ab917929057d8099750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 30 Sep 2023 16:20:04 +0200 Subject: [PATCH 4/7] Passes the encrypted VM pointer to BuiltinFunctions. --- src/interpreter.rs | 4 ++-- src/jit.rs | 2 +- src/program.rs | 7 +++++-- src/vm.rs | 4 +++- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index 6abadb9e..93b2b10d 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -16,7 +16,7 @@ use crate::{ ebpf::{self, STACK_PTR_REG}, elf::Executable, error::EbpfError, - vm::{Config, ContextObject, EbpfVm, ProgramResult}, + vm::{get_runtime_environment_key, Config, ContextObject, EbpfVm, ProgramResult}, }; /// Virtual memory operation helper. @@ -479,7 +479,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { self.vm.previous_instruction_meter = self.due_insn_count; self.due_insn_count = 0; function( - &mut self.vm, + unsafe { (self.vm as *mut _ as *mut u64).offset(get_runtime_environment_key() as isize) as *mut _ }, self.reg[1], self.reg[2], self.reg[3], diff --git a/src/jit.rs b/src/jit.rs index f4bd641a..31331509 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -1394,7 +1394,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { Argument { index: 3, value: Value::Register(ARGUMENT_REGISTERS[3]) }, Argument { index: 2, value: Value::Register(ARGUMENT_REGISTERS[2]) }, Argument { index: 1, value: Value::Register(ARGUMENT_REGISTERS[1]) }, - Argument { index: 0, value: Value::RegisterPlusConstant32(REGISTER_PTR_TO_VM, self.slot_in_vm(RuntimeEnvironmentSlot::HostStackPointer), false) }, + Argument { index: 0, value: Value::Register(REGISTER_PTR_TO_VM) }, ], None); if self.config.enable_instruction_meter { self.emit_ins(X86Instruction::load(OperandSize::S64, REGISTER_PTR_TO_VM, REGISTER_INSTRUCTION_METER, X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::PreviousInstructionMeter)))); // REGISTER_INSTRUCTION_METER = *PreviousInstructionMeter; diff --git a/src/program.rs b/src/program.rs index cb3972e1..350587fe 100644 --- a/src/program.rs +++ b/src/program.rs @@ -209,7 +209,7 @@ impl FunctionRegistry { } /// Syscall function without context -pub type BuiltinFunction = fn(&mut EbpfVm, u64, u64, u64, u64, u64); +pub type BuiltinFunction = fn(*mut EbpfVm, u64, u64, u64, u64, u64); /// Represents the interface to a fixed functionality program #[derive(Eq)] @@ -297,7 +297,7 @@ macro_rules! declare_builtin_function { /// VM interface #[allow(clippy::too_many_arguments)] pub fn vm( - vm: &mut $crate::vm::EbpfVm, + vm: *mut $crate::vm::EbpfVm, arg_a: u64, arg_b: u64, arg_c: u64, @@ -305,6 +305,9 @@ macro_rules! declare_builtin_function { arg_e: u64, ) { use $crate::vm::ContextObject; + let vm = unsafe { + &mut *((vm as *mut u64).offset(-($crate::vm::get_runtime_environment_key() as isize)) as *mut $crate::vm::EbpfVm) + }; let config = vm.loader.get_config(); if config.enable_instruction_meter { vm.context_object_pointer.consume(vm.previous_instruction_meter); diff --git a/src/vm.rs b/src/vm.rs index b59e2eaf..88f67cad 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -29,7 +29,9 @@ use std::{collections::BTreeMap, fmt::Debug, sync::Arc}; /// 3 bits for 8 Byte alignment, and 1 bit to have encoding space for the RuntimeEnvironment. const PROGRAM_ENVIRONMENT_KEY_SHIFT: u32 = 4; static RUNTIME_ENVIRONMENT_KEY: std::sync::OnceLock = std::sync::OnceLock::::new(); -pub(crate) fn get_runtime_environment_key() -> i32 { + +/// Returns (and if not done before generates) the encryption key for the VM pointer +pub fn get_runtime_environment_key() -> i32 { *RUNTIME_ENVIRONMENT_KEY .get_or_init(|| rand::thread_rng().gen::() >> PROGRAM_ENVIRONMENT_KEY_SHIFT) } From 9b1d33fcdb867b1b68e6dcb903e9f6a40578a382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 30 Sep 2023 17:23:19 +0200 Subject: [PATCH 5/7] Puts due_insn_count into the VM. --- src/interpreter.rs | 14 ++++++-------- src/jit.rs | 14 ++++++++------ src/program.rs | 2 +- src/vm.rs | 14 +++++++++----- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index 93b2b10d..2d485ba0 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -91,7 +91,6 @@ pub struct Interpreter<'a, 'b, C: ContextObject> { pub(crate) executable: &'a Executable, pub(crate) program: &'a [u8], pub(crate) program_vm_addr: u64, - pub(crate) due_insn_count: u64, /// General purpose registers and pc pub reg: [u64; 12], @@ -115,7 +114,6 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { executable, program, program_vm_addr, - due_insn_count: 0, reg: registers, #[cfg(feature = "debugger")] debug_state: DebugState::Continue, @@ -161,7 +159,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { pub fn step(&mut self) -> bool { let config = &self.executable.get_config(); - self.due_insn_count += 1; + self.vm.due_insn_count += 1; let mut next_pc = self.reg[11] + 1; if next_pc as usize * ebpf::INSN_SIZE > self.program.len() { throw_error!(self, EbpfError::ExecutionOverrun); @@ -456,7 +454,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { } check_pc!(self, next_pc, (target_pc - self.program_vm_addr) / ebpf::INSN_SIZE as u64); if self.executable.get_sbpf_version().static_syscalls() && self.executable.get_function_registry().lookup_by_key(next_pc as u32).is_none() { - self.due_insn_count += 1; + self.vm.due_insn_count += 1; self.reg[11] = next_pc; throw_error!(self, EbpfError::UnsupportedInstruction); } @@ -476,8 +474,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry().lookup_by_key(insn.imm as u32) { resolved = true; - self.vm.previous_instruction_meter = self.due_insn_count; - self.due_insn_count = 0; + self.vm.due_insn_count = self.vm.previous_instruction_meter - self.vm.due_insn_count; function( unsafe { (self.vm as *mut _ as *mut u64).offset(get_runtime_environment_key() as isize) as *mut _ }, self.reg[1], @@ -486,6 +483,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { self.reg[4], self.reg[5], ); + self.vm.due_insn_count = 0; self.reg[0] = match &self.vm.program_result { ProgramResult::Ok(value) => *value, ProgramResult::Err(_err) => return false, @@ -512,7 +510,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { ebpf::EXIT => { if self.vm.call_depth == 0 { - if config.enable_instruction_meter && self.due_insn_count > self.vm.previous_instruction_meter { + if config.enable_instruction_meter && self.vm.due_insn_count > self.vm.previous_instruction_meter { throw_error!(self, EbpfError::ExceededMaxInstructions); } self.vm.program_result = ProgramResult::Ok(self.reg[0]); @@ -535,7 +533,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { _ => throw_error!(self, EbpfError::UnsupportedInstruction), } - if config.enable_instruction_meter && self.due_insn_count >= self.vm.previous_instruction_meter { + if config.enable_instruction_meter && self.vm.due_insn_count >= self.vm.previous_instruction_meter { self.reg[11] += 1; throw_error!(self, EbpfError::ExceededMaxInstructions); } diff --git a/src/jit.rs b/src/jit.rs index 31331509..3e26a934 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -249,11 +249,12 @@ enum RuntimeEnvironmentSlot { StackPointer = 2, ContextObjectPointer = 3, PreviousInstructionMeter = 4, - StopwatchNumerator = 5, - StopwatchDenominator = 6, - Registers = 7, - ProgramResult = 19, - MemoryMapping = 27, + DueInsnCount = 5, + StopwatchNumerator = 6, + StopwatchDenominator = 7, + Registers = 8, + ProgramResult = 20, + MemoryMapping = 28, } /* Explaination of the Instruction Meter @@ -1386,7 +1387,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { self.set_anchor(ANCHOR_EXTERNAL_FUNCTION_CALL); self.emit_ins(X86Instruction::push_immediate(OperandSize::S64, -1)); // Used as PC value in error case, acts as stack padding otherwise if self.config.enable_instruction_meter { - self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x29, REGISTER_INSTRUCTION_METER, REGISTER_PTR_TO_VM, 0, Some(X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::PreviousInstructionMeter))))); // *PreviousInstructionMeter -= REGISTER_INSTRUCTION_METER; + self.emit_ins(X86Instruction::store(OperandSize::S64, REGISTER_INSTRUCTION_METER, REGISTER_PTR_TO_VM, X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::DueInsnCount)))); // *DueInsnCount = REGISTER_INSTRUCTION_METER; } self.emit_rust_call(Value::Register(REGISTER_SCRATCH), &[ Argument { index: 5, value: Value::Register(ARGUMENT_REGISTERS[5]) }, @@ -1627,6 +1628,7 @@ mod tests { check_slot!(env, stack_pointer, StackPointer); check_slot!(env, context_object_pointer, ContextObjectPointer); check_slot!(env, previous_instruction_meter, PreviousInstructionMeter); + check_slot!(env, due_insn_count, DueInsnCount); check_slot!(env, stopwatch_numerator, StopwatchNumerator); check_slot!(env, stopwatch_denominator, StopwatchDenominator); check_slot!(env, registers, Registers); diff --git a/src/program.rs b/src/program.rs index 350587fe..add04970 100644 --- a/src/program.rs +++ b/src/program.rs @@ -310,7 +310,7 @@ macro_rules! declare_builtin_function { }; let config = vm.loader.get_config(); if config.enable_instruction_meter { - vm.context_object_pointer.consume(vm.previous_instruction_meter); + vm.context_object_pointer.consume(vm.previous_instruction_meter - vm.due_insn_count); } let converted_result: $crate::vm::ProgramResult = Self::rust( vm.context_object_pointer, arg_a, arg_b, arg_c, arg_d, arg_e, &mut vm.memory_mapping, diff --git a/src/vm.rs b/src/vm.rs index 88f67cad..73a1250b 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -376,6 +376,8 @@ pub struct EbpfVm<'a, C: ContextObject> { pub context_object_pointer: &'a mut C, /// Last return value of instruction_meter.get_remaining() pub previous_instruction_meter: u64, + /// Outstanding value to instruction_meter.consume() + pub due_insn_count: u64, /// CPU cycles accumulated by the stop watch pub stopwatch_numerator: u64, /// Number of times the stop watch was used @@ -422,6 +424,7 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { stack_pointer, context_object_pointer: context_object, previous_instruction_meter: 0, + due_insn_count: 0, stopwatch_numerator: 0, stopwatch_denominator: 0, registers: [0u64; 12], @@ -454,8 +457,9 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { 0 }; self.previous_instruction_meter = initial_insn_count; + self.due_insn_count = 0; self.program_result = ProgramResult::Ok(0); - let due_insn_count = if interpreted { + if interpreted { #[cfg(feature = "debugger")] let debug_port = self.debug_port.clone(); let mut interpreter = Interpreter::new(self, executable, self.registers); @@ -467,7 +471,6 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { } #[cfg(not(feature = "debugger"))] while interpreter.step() {} - interpreter.due_insn_count } else { #[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))] { @@ -480,9 +483,10 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { }; let instruction_meter_final = compiled_program.invoke(config, self, self.registers).max(0) as u64; - self.context_object_pointer + self.due_insn_count = self + .context_object_pointer .get_remaining() - .saturating_sub(instruction_meter_final) + .saturating_sub(instruction_meter_final); } #[cfg(not(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64")))] { @@ -490,7 +494,7 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { } }; let instruction_count = if config.enable_instruction_meter { - self.context_object_pointer.consume(due_insn_count); + self.context_object_pointer.consume(self.due_insn_count); initial_insn_count.saturating_sub(self.context_object_pointer.get_remaining()) } else { 0 From 0097061c278a086c3e131893add415e5bed84f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 30 Sep 2023 18:27:31 +0200 Subject: [PATCH 6/7] Moves the calculation of due_insn_count from the VM into the JIT epilogue. --- src/jit.rs | 12 ++++++------ src/vm.rs | 7 +------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/jit.rs b/src/jit.rs index 3e26a934..76d11646 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -94,10 +94,8 @@ impl JitProgram { _config: &Config, vm: &mut EbpfVm, registers: [u64; 12], - ) -> i64 { + ) { unsafe { - let mut instruction_meter = - (vm.previous_instruction_meter as i64).wrapping_add(registers[11] as i64); std::arch::asm!( // RBP and RBX must be saved and restored manually in the current version of rustc and llvm. "push rbx", @@ -118,19 +116,17 @@ impl JitProgram { "mov rbp, [r11 + 0x50]", "mov r11, [r11 + 0x58]", "call r10", - "mov rax, rbx", "pop rbp", "pop rbx", host_stack_pointer = in(reg) &mut vm.host_stack_pointer, - inlateout("rax") instruction_meter, inlateout("rdi") (vm as *mut _ as *mut u64).offset(get_runtime_environment_key() as isize) => _, + inlateout("rax") (vm.previous_instruction_meter as i64).wrapping_add(registers[11] as i64) => _, inlateout("r10") self.pc_section[registers[11] as usize] => _, inlateout("r11") ®isters => _, lateout("rsi") _, lateout("rdx") _, lateout("rcx") _, lateout("r8") _, lateout("r9") _, lateout("r12") _, lateout("r13") _, lateout("r14") _, lateout("r15") _, // lateout("rbp") _, lateout("rbx") _, ); - instruction_meter } } @@ -1315,6 +1311,10 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { if self.config.enable_instruction_meter { self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x81, 5, REGISTER_INSTRUCTION_METER, 1, None)); // REGISTER_INSTRUCTION_METER -= 1; self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x29, REGISTER_SCRATCH, REGISTER_INSTRUCTION_METER, 0, None)); // REGISTER_INSTRUCTION_METER -= pc; + // *DueInsnCount = *PreviousInstructionMeter - REGISTER_INSTRUCTION_METER; + self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x2B, REGISTER_INSTRUCTION_METER, REGISTER_PTR_TO_VM, 0, Some(X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::PreviousInstructionMeter))))); // REGISTER_INSTRUCTION_METER -= *PreviousInstructionMeter; + self.emit_ins(X86Instruction::alu(OperandSize::S64, 0xf7, 3, REGISTER_INSTRUCTION_METER, 0, None)); // REGISTER_INSTRUCTION_METER = -REGISTER_INSTRUCTION_METER; + self.emit_ins(X86Instruction::store(OperandSize::S64, REGISTER_INSTRUCTION_METER, REGISTER_PTR_TO_VM, X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::DueInsnCount)))); // *DueInsnCount = REGISTER_INSTRUCTION_METER; } // Print stop watch value fn stopwatch_result(numerator: u64, denominator: u64) { diff --git a/src/vm.rs b/src/vm.rs index 73a1250b..47219c8f 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -481,12 +481,7 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { Ok(compiled_program) => compiled_program, Err(error) => return (0, ProgramResult::Err(error)), }; - let instruction_meter_final = - compiled_program.invoke(config, self, self.registers).max(0) as u64; - self.due_insn_count = self - .context_object_pointer - .get_remaining() - .saturating_sub(instruction_meter_final); + compiled_program.invoke(config, self, self.registers); } #[cfg(not(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64")))] { From b4391ac8bd379f75508eb3c9a40cebc3ca87e254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sun, 1 Oct 2023 14:49:21 +0200 Subject: [PATCH 7/7] Makes $ContextObject generic in declare_builtin_function!(). --- src/program.rs | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/program.rs b/src/program.rs index add04970..05bc0a68 100644 --- a/src/program.rs +++ b/src/program.rs @@ -288,32 +288,50 @@ impl std::fmt::Debug for BuiltinProgram { /// Generates an adapter for a BuiltinFunction between the Rust and the VM interface #[macro_export] macro_rules! declare_builtin_function { - ($(#[$attr:meta])* $name:ident, $rust:item) => { + ($(#[$attr:meta])* $name:ident, fn rust( + $vm:ident : &mut $ContextObject:ty, + $arg_a:ident : u64, + $arg_b:ident : u64, + $arg_c:ident : u64, + $arg_d:ident : u64, + $arg_e:ident : u64, + $memory_mapping:ident : &mut $MemoryMapping:ty, + ) -> Result $rust:tt) => { $(#[$attr])* pub struct $name {} impl $name { /// Rust interface - $rust + pub fn rust( + $vm: &mut $ContextObject, + $arg_a: u64, + $arg_b: u64, + $arg_c: u64, + $arg_d: u64, + $arg_e: u64, + $memory_mapping: &mut $MemoryMapping, + ) -> Result { + $rust + } /// VM interface #[allow(clippy::too_many_arguments)] pub fn vm( - vm: *mut $crate::vm::EbpfVm, - arg_a: u64, - arg_b: u64, - arg_c: u64, - arg_d: u64, - arg_e: u64, + $vm: *mut $crate::vm::EbpfVm<$ContextObject>, + $arg_a: u64, + $arg_b: u64, + $arg_c: u64, + $arg_d: u64, + $arg_e: u64, ) { use $crate::vm::ContextObject; let vm = unsafe { - &mut *((vm as *mut u64).offset(-($crate::vm::get_runtime_environment_key() as isize)) as *mut $crate::vm::EbpfVm) + &mut *(($vm as *mut u64).offset(-($crate::vm::get_runtime_environment_key() as isize)) as *mut $crate::vm::EbpfVm<$ContextObject>) }; let config = vm.loader.get_config(); if config.enable_instruction_meter { vm.context_object_pointer.consume(vm.previous_instruction_meter - vm.due_insn_count); } let converted_result: $crate::vm::ProgramResult = Self::rust( - vm.context_object_pointer, arg_a, arg_b, arg_c, arg_d, arg_e, &mut vm.memory_mapping, + vm.context_object_pointer, $arg_a, $arg_b, $arg_c, $arg_d, $arg_e, &mut vm.memory_mapping, ).into(); vm.program_result = converted_result; if config.enable_instruction_meter {