From b3a3495c2257945d0e3e09603cb8aa40396c0fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 29 Sep 2023 14:34:22 +0200 Subject: [PATCH] Refactor - Store exception pc back in vm (#529) * Enables tracing in all execution tests. * Stores exception IP/PC in vm.registers[11]. * Assert final pc values do match. * Moves pc in registers. --- src/debugger.rs | 4 +- src/interpreter.rs | 126 ++++++++++++++++++++------------------------- src/jit.rs | 7 ++- src/vm.rs | 16 +++--- tests/execution.rs | 19 ++++--- 5 files changed, 84 insertions(+), 88 deletions(-) diff --git a/src/debugger.rs b/src/debugger.rs index 23f971ee..432cf8b7 100644 --- a/src/debugger.rs +++ b/src/debugger.rs @@ -181,7 +181,7 @@ impl<'a, 'b, C: ContextObject> SingleThreadBase for Interpreter<'a, 'b, C> { self.reg[i] = regs.r[i]; } self.reg[ebpf::FRAME_PTR_REG] = regs.sp; - self.pc = regs.pc as usize; + self.reg[11] = regs.pc; Ok(()) } @@ -252,7 +252,7 @@ impl<'a, 'b, C: ContextObject> target::ext::base::single_register_access::Single match reg_id { BpfRegId::Gpr(i) => self.reg[i as usize] = r, BpfRegId::Sp => self.reg[ebpf::FRAME_PTR_REG] = r, - BpfRegId::Pc => self.pc = r as usize, + BpfRegId::Pc => self.reg[11] = r, BpfRegId::InstructionCountRemaining => (), } Ok(()) diff --git a/src/interpreter.rs b/src/interpreter.rs index 529d59b5..b953a701 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -18,7 +18,6 @@ use crate::{ error::EbpfError, vm::{Config, ContextObject, EbpfVm, ProgramResult}, }; -use std::convert::TryInto; /// Virtual memory operation helper. macro_rules! translate_memory_access { @@ -29,8 +28,7 @@ macro_rules! translate_memory_access { ) { ProgramResult::Ok(v) => v, ProgramResult::Err(err) => { - $self.vm.program_result = ProgramResult::Err(err); - return false; + throw_error!($self, err); }, } }; @@ -48,6 +46,7 @@ macro_rules! translate_memory_access { macro_rules! throw_error { ($self:expr, $err:expr) => {{ + $self.vm.registers[11] = $self.reg[11]; $self.vm.program_result = ProgramResult::Err($err); return false; }}; @@ -63,6 +62,20 @@ macro_rules! throw_error { }; } +macro_rules! check_pc { + ($self:expr, $next_pc:ident, $target_pc:expr) => { + if ($target_pc as usize) + .checked_mul(ebpf::INSN_SIZE) + .and_then(|offset| $self.program.get(offset..offset + ebpf::INSN_SIZE)) + .is_some() + { + $next_pc = $target_pc; + } else { + throw_error!($self, EbpfError::CallOutsideTextSegment); + } + }; +} + /// State of the interpreter during a debugging session #[cfg(feature = "debugger")] pub enum DebugState { @@ -80,10 +93,8 @@ pub struct Interpreter<'a, 'b, C: ContextObject> { pub(crate) program_vm_addr: u64, pub(crate) due_insn_count: u64, - /// General purpose self.registers - pub reg: [u64; 11], - /// Program counter / instruction pointer - pub pc: usize, + /// General purpose registers and pc + pub reg: [u64; 12], #[cfg(feature = "debugger")] pub(crate) debug_state: DebugState, @@ -105,8 +116,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { program, program_vm_addr, due_insn_count: 0, - reg: registers[0..11].try_into().unwrap(), - pc: registers[11] as usize, + reg: registers, #[cfg(feature = "debugger")] debug_state: DebugState::Continue, #[cfg(feature = "debugger")] @@ -114,23 +124,10 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { } } - fn check_pc(&mut self) -> bool { - if self - .pc - .checked_mul(ebpf::INSN_SIZE) - .and_then(|offset| self.program.get(offset..offset + ebpf::INSN_SIZE)) - .is_some() - { - true - } else { - throw_error!(self, EbpfError::CallOutsideTextSegment); - } - } - /// Translate between the virtual machines' pc value and the pc value used by the debugger #[cfg(feature = "debugger")] pub fn get_dbg_pc(&self) -> u64 { - ((self.pc * ebpf::INSN_SIZE) as u64) + self.executable.get_text_section_offset() + (self.reg[11] * ebpf::INSN_SIZE as u64) + self.executable.get_text_section_offset() } fn push_frame(&mut self, config: &Config) -> bool { @@ -139,7 +136,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { &self.reg[ebpf::FIRST_SCRATCH_REG..ebpf::FIRST_SCRATCH_REG + ebpf::SCRATCH_REGS], ); frame.frame_pointer = self.reg[ebpf::FRAME_PTR_REG]; - frame.target_pc = self.pc; + frame.target_pc = self.reg[11] + 1; self.vm.call_depth += 1; if self.vm.call_depth as usize == config.max_call_depth { @@ -165,20 +162,16 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { let config = &self.executable.get_config(); self.due_insn_count += 1; - let pc = self.pc; - self.pc += 1; - if self.pc * ebpf::INSN_SIZE > self.program.len() { + let mut next_pc = self.reg[11] + 1; + if next_pc as usize * ebpf::INSN_SIZE > self.program.len() { throw_error!(self, EbpfError::ExecutionOverrun); } - let mut insn = ebpf::get_insn_unchecked(self.program, pc); + let mut insn = ebpf::get_insn_unchecked(self.program, self.reg[11] as usize); let dst = insn.dst as usize; let src = insn.src as usize; if config.enable_instruction_tracing { - let mut state = [0u64; 12]; - state[0..11].copy_from_slice(&self.reg); - state[11] = pc as u64; - self.vm.context_object_pointer.trace(state); + self.vm.context_object_pointer.trace(self.reg); } match insn.opc { @@ -194,8 +187,9 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { ebpf::LD_DW_IMM => { ebpf::augment_lddw_unchecked(self.program, &mut insn); - self.pc += 1; self.reg[dst] = insn.imm as u64; + self.reg[11] += 1; + next_pc += 1; }, // BPF_LDX class @@ -424,29 +418,29 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { }, // BPF_JMP class - ebpf::JA => { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JEQ_IMM => if self.reg[dst] == insn.imm as u64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JEQ_REG => if self.reg[dst] == self.reg[src] { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JGT_IMM => if self.reg[dst] > insn.imm as u64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JGT_REG => if self.reg[dst] > self.reg[src] { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JGE_IMM => if self.reg[dst] >= insn.imm as u64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JGE_REG => if self.reg[dst] >= self.reg[src] { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JLT_IMM => if self.reg[dst] < insn.imm as u64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JLT_REG => if self.reg[dst] < self.reg[src] { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JLE_IMM => if self.reg[dst] <= insn.imm as u64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JLE_REG => if self.reg[dst] <= self.reg[src] { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSET_IMM => if self.reg[dst] & insn.imm as u64 != 0 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSET_REG => if self.reg[dst] & self.reg[src] != 0 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JNE_IMM => if self.reg[dst] != insn.imm as u64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JNE_REG => if self.reg[dst] != self.reg[src] { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSGT_IMM => if (self.reg[dst] as i64) > insn.imm { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSGT_REG => if (self.reg[dst] as i64) > self.reg[src] as i64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSGE_IMM => if (self.reg[dst] as i64) >= insn.imm { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSGE_REG => if (self.reg[dst] as i64) >= self.reg[src] as i64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSLT_IMM => if (self.reg[dst] as i64) < insn.imm { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSLT_REG => if (self.reg[dst] as i64) < self.reg[src] as i64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSLE_IMM => if (self.reg[dst] as i64) <= insn.imm { self.pc = (self.pc as isize + insn.off as isize) as usize; }, - ebpf::JSLE_REG => if (self.reg[dst] as i64) <= self.reg[src] as i64 { self.pc = (self.pc as isize + insn.off as isize) as usize; }, + ebpf::JA => { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JEQ_IMM => if self.reg[dst] == insn.imm as u64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JEQ_REG => if self.reg[dst] == self.reg[src] { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JGT_IMM => if self.reg[dst] > insn.imm as u64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JGT_REG => if self.reg[dst] > self.reg[src] { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JGE_IMM => if self.reg[dst] >= insn.imm as u64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JGE_REG => if self.reg[dst] >= self.reg[src] { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JLT_IMM => if self.reg[dst] < insn.imm as u64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JLT_REG => if self.reg[dst] < self.reg[src] { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JLE_IMM => if self.reg[dst] <= insn.imm as u64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JLE_REG => if self.reg[dst] <= self.reg[src] { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSET_IMM => if self.reg[dst] & insn.imm as u64 != 0 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSET_REG => if self.reg[dst] & self.reg[src] != 0 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JNE_IMM => if self.reg[dst] != insn.imm as u64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JNE_REG => if self.reg[dst] != self.reg[src] { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSGT_IMM => if (self.reg[dst] as i64) > insn.imm { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSGT_REG => if (self.reg[dst] as i64) > self.reg[src] as i64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSGE_IMM => if (self.reg[dst] as i64) >= insn.imm { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSGE_REG => if (self.reg[dst] as i64) >= self.reg[src] as i64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSLT_IMM => if (self.reg[dst] as i64) < insn.imm { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSLT_REG => if (self.reg[dst] as i64) < self.reg[src] as i64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSLE_IMM => if (self.reg[dst] as i64) <= insn.imm { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, + ebpf::JSLE_REG => if (self.reg[dst] as i64) <= self.reg[src] as i64 { next_pc = (next_pc as i64 + insn.off as i64) as u64; }, ebpf::CALL_REG => { let target_pc = if self.executable.get_sbpf_version().callx_uses_src_reg() { @@ -460,12 +454,10 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { if target_pc < self.program_vm_addr { throw_error!(self, EbpfError::CallOutsideTextSegment); } - self.pc = (target_pc - self.program_vm_addr) as usize / ebpf::INSN_SIZE; - if !self.check_pc() { - return false; - } - if self.executable.get_sbpf_version().static_syscalls() && self.executable.get_function_registry().lookup_by_key(self.pc as u32).is_none() { + 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.reg[11] = next_pc; throw_error!(self, EbpfError::UnsupportedInstruction); } }, @@ -516,10 +508,7 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { if !self.push_frame(config) { return false; } - self.pc = target_pc; - if !self.check_pc() { - return false; - } + check_pc!(self, next_pc, target_pc as u64); } } @@ -539,7 +528,6 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { // Return from BPF to BPF call self.vm.call_depth -= 1; let frame = &self.vm.call_frames[self.vm.call_depth as usize]; - self.pc = frame.target_pc; self.reg[ebpf::FRAME_PTR_REG] = frame.frame_pointer; self.reg[ebpf::FIRST_SCRATCH_REG ..ebpf::FIRST_SCRATCH_REG + ebpf::SCRATCH_REGS] @@ -549,17 +537,17 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { config.stack_frame_size * if config.enable_stack_frame_gaps { 2 } else { 1 }; self.vm.stack_pointer -= stack_frame_size as u64; } - if !self.check_pc() { - return false; - } + check_pc!(self, next_pc, frame.target_pc); } _ => throw_error!(self, EbpfError::UnsupportedInstruction), } if config.enable_instruction_meter && self.due_insn_count >= self.vm.previous_instruction_meter { + self.reg[11] += 1; throw_error!(self, EbpfError::ExceededMaxInstructions); } + self.reg[11] = next_pc; true } } diff --git a/src/jit.rs b/src/jit.rs index c91e1e5f..b10f87eb 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -254,8 +254,9 @@ enum RuntimeEnvironmentSlot { PreviousInstructionMeter = 4, StopwatchNumerator = 5, StopwatchDenominator = 6, - ProgramResult = 7, - MemoryMapping = 15, + Registers = 7, + ProgramResult = 19, + MemoryMapping = 27, } /* Explaination of the Instruction Meter @@ -1347,6 +1348,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { // Epilogue for errors self.set_anchor(ANCHOR_THROW_EXCEPTION_UNCHECKED); + self.emit_ins(X86Instruction::store(OperandSize::S64, R11, RBP, X86IndirectAccess::Offset(self.slot_on_environment_stack(RuntimeEnvironmentSlot::Registers) + 11 * std::mem::size_of::() as i32))); // registers[11] = pc; self.emit_ins(X86Instruction::jump_immediate(self.relative_to_anchor(ANCHOR_EPILOGUE, 5))); // Quit gracefully @@ -1649,6 +1651,7 @@ mod tests { check_slot!(env, previous_instruction_meter, PreviousInstructionMeter); check_slot!(env, stopwatch_numerator, StopwatchNumerator); check_slot!(env, stopwatch_denominator, StopwatchDenominator); + check_slot!(env, registers, Registers); check_slot!(env, program_result, ProgramResult); check_slot!(env, memory_mapping, MemoryMapping); } diff --git a/src/vm.rs b/src/vm.rs index 89174bb4..2684cc1d 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -292,7 +292,7 @@ pub struct CallFrame { /// The callers frame pointer pub frame_pointer: u64, /// The target_pc of the exit instruction which returns back to the caller - pub target_pc: usize, + pub target_pc: u64, } /// A virtual machine to run eBPF programs. @@ -371,6 +371,8 @@ pub struct EbpfVm<'a, C: ContextObject> { pub stopwatch_numerator: u64, /// Number of times the stop watch was used pub stopwatch_denominator: u64, + /// Registers inlined + pub registers: [u64; 12], /// ProgramResult inlined pub program_result: ProgramResult, /// MemoryMapping inlined @@ -410,6 +412,7 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { previous_instruction_meter: 0, stopwatch_numerator: 0, stopwatch_denominator: 0, + registers: [0u64; 12], program_result: ProgramResult::Ok(0), memory_mapping, call_frames: vec![CallFrame::default(); config.max_call_depth], @@ -426,11 +429,10 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { executable: &Executable, interpreted: bool, ) -> (u64, ProgramResult) { - let mut registers = [0u64; 12]; // R1 points to beginning of input memory, R10 to the stack of the first frame, R11 is the pc (hidden) - registers[1] = ebpf::MM_INPUT_START; - registers[ebpf::FRAME_PTR_REG] = self.stack_pointer; - registers[11] = executable.get_entrypoint_instruction_offset() as u64; + self.registers[1] = ebpf::MM_INPUT_START; + self.registers[ebpf::FRAME_PTR_REG] = self.stack_pointer; + self.registers[11] = executable.get_entrypoint_instruction_offset() as u64; let config = executable.get_config(); let initial_insn_count = if config.enable_instruction_meter { self.context_object_pointer.get_remaining() @@ -442,7 +444,7 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { let due_insn_count = if interpreted { #[cfg(feature = "debugger")] let debug_port = self.debug_port.clone(); - let mut interpreter = Interpreter::new(self, executable, registers); + let mut interpreter = Interpreter::new(self, executable, self.registers); #[cfg(feature = "debugger")] if let Some(debug_port) = debug_port { crate::debugger::execute(&mut interpreter, debug_port); @@ -463,7 +465,7 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> { Err(error) => return (0, ProgramResult::Err(error)), }; let instruction_meter_final = - compiled_program.invoke(config, self, registers).max(0) as u64; + compiled_program.invoke(config, self, self.registers).max(0) as u64; self.context_object_pointer .get_remaining() .saturating_sub(instruction_meter_final) diff --git a/tests/execution.rs b/tests/execution.rs index 33c4ba87..0cf7f8c4 100644 --- a/tests/execution.rs +++ b/tests/execution.rs @@ -49,7 +49,7 @@ macro_rules! test_interpreter_and_jit { context_object.remaining = INSTRUCTION_METER_BUDGET; } $executable.verify::().unwrap(); - let (instruction_count_interpreter, _tracer_interpreter) = { + let (instruction_count_interpreter, interpreter_final_pc, _tracer_interpreter) = { let mut mem = $mem; let mem_region = MemoryRegion::new_writable(&mut mem, ebpf::MM_INPUT_START); let mut context_object = context_object.clone(); @@ -70,6 +70,7 @@ macro_rules! test_interpreter_and_jit { ); ( instruction_count_interpreter, + vm.registers[11], vm.context_object_pointer.clone(), ) }; @@ -120,6 +121,10 @@ macro_rules! test_interpreter_and_jit { instruction_count_interpreter, instruction_count_jit, "Interpreter and JIT instruction meter diverged", ); + assert_eq!( + interpreter_final_pc, vm.registers[11], + "Interpreter and JIT instruction final PC diverged", + ); } } } @@ -133,12 +138,14 @@ macro_rules! test_interpreter_and_jit { } macro_rules! test_interpreter_and_jit_asm { - ($source:tt, $config:tt, $mem:tt, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => { + ($source:tt, $config:expr, $mem:tt, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => { #[allow(unused_mut)] { + let mut config = $config; + config.enable_instruction_tracing = true; let mut function_registry = FunctionRegistry::>::default(); $(test_interpreter_and_jit!(register, function_registry, $location => $syscall_function);)* - let loader = Arc::new(BuiltinProgram::new_loader($config, function_registry)); + let loader = Arc::new(BuiltinProgram::new_loader(config, function_registry)); let mut executable = assemble($source, loader).unwrap(); test_interpreter_and_jit!(executable, $mem, $context_object, $expected_result); } @@ -146,11 +153,7 @@ macro_rules! test_interpreter_and_jit_asm { ($source:tt, $mem:tt, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => { #[allow(unused_mut)] { - let config = Config { - enable_instruction_tracing: true, - ..Config::default() - }; - test_interpreter_and_jit_asm!($source, config, $mem, ($($location => $syscall_function),*), $context_object, $expected_result); + test_interpreter_and_jit_asm!($source, Config::default(), $mem, ($($location => $syscall_function),*), $context_object, $expected_result); } }; }