Skip to content

Commit

Permalink
Fix - ExceededMaxInstructions final PC (#593)
Browse files Browse the repository at this point in the history 
* Removes unused immediate.

* Moves instruction meter validation first in internal function calls.

* Splits emit_validate_and_profile_instruction_count in exit instruction.

* Fixes chaotic final PC of ExceededMaxInstructions.

* Removes exclusive parameter emit_validate_and_profile_instruction_count.
  • Loading branch information
@Lichtso
Lichtso authored Sep 12, 2024
1 parent 1223789 commit 57139e9
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 21 deletions.
5 changes: 0 additions & 5 deletions src/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,11 +527,6 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> {
_ => throw_error!(self, EbpfError::UnsupportedInstruction),
}

if config.enable_instruction_meter && self.vm.due_insn_count >= self.vm.previous_instruction_meter {
self.reg[11] += 1;
throw_error!(self, EbpfError::ExceededMaxInstructions);
}

self.reg[11] = next_pc;
true
}
Expand Down
35 changes: 19 additions & 16 deletions src/jit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
}

ebpf::LD_DW_IMM if self.executable.get_sbpf_version().enable_lddw() => {
self.emit_validate_and_profile_instruction_count(true, false, Some(self.pc + 2));
self.emit_validate_and_profile_instruction_count(false, Some(self.pc + 2));
self.pc += 1;
self.result.pc_section[self.pc] = self.anchors[ANCHOR_CALL_UNSUPPORTED_INSTRUCTION] as usize;
ebpf::augment_lddw_unchecked(self.program, &mut insn);
Expand Down Expand Up @@ -628,7 +628,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {

// BPF_JMP class
ebpf::JA => {
self.emit_validate_and_profile_instruction_count(false, true, Some(target_pc));
self.emit_validate_and_profile_instruction_count(true, Some(target_pc));
self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, REGISTER_SCRATCH, target_pc as i64));
let jump_offset = self.relative_to_target_pc(target_pc, 5);
self.emit_ins(X86Instruction::jump_immediate(jump_offset));
Expand Down Expand Up @@ -667,7 +667,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) {
self.emit_validate_and_profile_instruction_count(true, false, Some(0));
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)));
self.emit_undo_profile_instruction_count(0);
Expand Down Expand Up @@ -696,6 +696,8 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
self.emit_internal_call(Value::Register(target_pc));
},
ebpf::EXIT => {
self.emit_validate_instruction_count(true, Some(self.pc));

let call_depth_access = X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::CallDepth));
self.emit_ins(X86Instruction::load(OperandSize::S64, REGISTER_PTR_TO_VM, REGISTER_MAP[FRAME_PTR_REG], call_depth_access));

Expand All @@ -718,7 +720,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
}

// and return
self.emit_validate_and_profile_instruction_count(false, false, Some(0));
self.emit_profile_instruction_count(false, Some(0));
self.emit_ins(X86Instruction::return_near());
},

Expand All @@ -732,7 +734,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
if self.offset_in_text_section + MAX_MACHINE_CODE_LENGTH_PER_INSTRUCTION * 2 >= self.result.text_section.len() {
return Err(EbpfError::ExhaustedTextSegment(self.pc));
}
self.emit_validate_and_profile_instruction_count(true, false, Some(self.pc + 1));
self.emit_validate_and_profile_instruction_count(false, Some(self.pc + 1));
self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, REGISTER_SCRATCH, self.pc as i64)); // Save pc
self.emit_set_exception_kind(EbpfError::ExecutionOverrun);
self.emit_ins(X86Instruction::jump_immediate(self.relative_to_anchor(ANCHOR_THROW_EXCEPTION, 5)));
Expand Down Expand Up @@ -895,6 +897,9 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {

#[inline]
fn emit_profile_instruction_count(&mut self, user_provided: bool, target_pc: Option<usize>) {
if !self.config.enable_instruction_meter {
return;
}
match target_pc {
Some(target_pc) => {
// instruction_meter += target_pc - (self.pc + 1);
Expand All @@ -907,17 +912,15 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
},
None => {
self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x81, 5, REGISTER_INSTRUCTION_METER, self.pc as i64 + 1, None)); // instruction_meter -= self.pc + 1;
self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x01, REGISTER_SCRATCH, REGISTER_INSTRUCTION_METER, self.pc as i64, None)); // instruction_meter += target_pc;
self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x01, REGISTER_SCRATCH, REGISTER_INSTRUCTION_METER, 0, None)); // instruction_meter += target_pc;
},
}
}

#[inline]
fn emit_validate_and_profile_instruction_count(&mut self, exclusive: bool, user_provided: bool, target_pc: Option<usize>) {
if self.config.enable_instruction_meter {
self.emit_validate_instruction_count(exclusive, Some(self.pc));
self.emit_profile_instruction_count(user_provided, target_pc);
}
fn emit_validate_and_profile_instruction_count(&mut self, user_provided: bool, target_pc: Option<usize>) {
self.emit_validate_instruction_count(true, Some(self.pc));
self.emit_profile_instruction_count(user_provided, target_pc);
}

#[inline]
Expand Down Expand Up @@ -1037,6 +1040,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
// Store PC in case the bounds check fails
self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, REGISTER_SCRATCH, self.pc as i64));

self.emit_validate_instruction_count(true, Some(self.pc));
self.emit_ins(X86Instruction::call_immediate(self.relative_to_anchor(ANCHOR_ANCHOR_INTERNAL_FUNCTION_CALL_PROLOGUE, 5)));

match dst {
Expand All @@ -1049,14 +1053,14 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {

self.emit_ins(X86Instruction::call_immediate(self.relative_to_anchor(ANCHOR_ANCHOR_INTERNAL_FUNCTION_CALL_REG, 5)));

self.emit_validate_and_profile_instruction_count(false, false, None);
self.emit_profile_instruction_count(false, None);
self.emit_ins(X86Instruction::mov(OperandSize::S64, REGISTER_MAP[0], REGISTER_OTHER_SCRATCH));
self.emit_ins(X86Instruction::pop(REGISTER_MAP[0])); // Restore RAX
self.emit_ins(X86Instruction::call_reg(REGISTER_OTHER_SCRATCH, None)); // callq *REGISTER_OTHER_SCRATCH
},
Value::Constant64(target_pc, user_provided) => {
debug_assert!(user_provided);
self.emit_validate_and_profile_instruction_count(false, user_provided, Some(target_pc as usize));
self.emit_profile_instruction_count(user_provided, Some(target_pc as usize));
if user_provided && self.should_sanitize_constant(target_pc) {
self.emit_sanitized_load_immediate(OperandSize::S64, REGISTER_SCRATCH, target_pc);
} else {
Expand Down Expand Up @@ -1149,7 +1153,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {

#[inline]
fn emit_conditional_branch_reg(&mut self, op: u8, bitwise: bool, first_operand: u8, second_operand: u8, target_pc: usize) {
self.emit_validate_and_profile_instruction_count(false, true, Some(target_pc));
self.emit_validate_and_profile_instruction_count(true, Some(target_pc));
if bitwise { // Logical
self.emit_ins(X86Instruction::test(OperandSize::S64, first_operand, second_operand, None));
} else { // Arithmetic
Expand All @@ -1163,7 +1167,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {

#[inline]
fn emit_conditional_branch_imm(&mut self, op: u8, bitwise: bool, immediate: i64, second_operand: u8, target_pc: usize) {
self.emit_validate_and_profile_instruction_count(false, true, Some(target_pc));
self.emit_validate_and_profile_instruction_count(true, Some(target_pc));
if self.should_sanitize_constant(immediate) {
self.emit_sanitized_load_immediate(OperandSize::S64, REGISTER_SCRATCH, immediate);
if bitwise { // Logical
Expand Down Expand Up @@ -1379,7 +1383,6 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {

// Quit gracefully
self.set_anchor(ANCHOR_EXIT);
self.emit_validate_instruction_count(false, None);
self.emit_ins(X86Instruction::lea(OperandSize::S64, REGISTER_PTR_TO_VM, REGISTER_OTHER_SCRATCH, Some(X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::ProgramResult)))));
self.emit_ins(X86Instruction::store(OperandSize::S64, REGISTER_MAP[0], REGISTER_OTHER_SCRATCH, X86IndirectAccess::Offset(std::mem::size_of::<u64>() as i32))); // result.return_value = R0;
self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, REGISTER_MAP[0], 0));
Expand Down

0 comments on commit 57139e9

Please sign in to comment.