From dc1c8deaad8c58ee957f5577060435e0776be210 Mon Sep 17 00:00:00 2001 From: Lucas Date: Wed, 9 Oct 2024 18:09:42 -0300 Subject: [PATCH] Add return to verifier, jitter and interpreter --- src/interpreter.rs | 5 ++ src/jit.rs | 4 ++ src/verifier.rs | 4 +- tests/execution.rs | 147 +++++++++++++++++++++++++++++++++------------ tests/verifier.rs | 61 +++++++++++++++++++ 5 files changed, 181 insertions(+), 40 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index 8d1dee5c..adc2ab79 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -567,6 +567,11 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { } ebpf::RETURN | ebpf::EXIT => { + if (insn.opc == ebpf::EXIT && self.executable.get_sbpf_version().static_syscalls()) + || (insn.opc == ebpf::RETURN && !self.executable.get_sbpf_version().static_syscalls()) { + throw_error!(self, EbpfError::UnsupportedInstruction); + } + if self.vm.call_depth == 0 { if config.enable_instruction_meter && self.vm.due_insn_count > self.vm.previous_instruction_meter { throw_error!(self, EbpfError::ExceededMaxInstructions); diff --git a/src/jit.rs b/src/jit.rs index 78fd1a02..cf1fd63e 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -747,6 +747,10 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { }, ebpf::RETURN | ebpf::EXIT => { + if (insn.opc == ebpf::EXIT && self.executable.get_sbpf_version().static_syscalls()) + || (insn.opc == ebpf::RETURN && !self.executable.get_sbpf_version().static_syscalls()) { + return Err(EbpfError::UnsupportedInstruction); + } self.emit_validate_instruction_count(true, Some(self.pc)); let call_depth_access = X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::CallDepth)); diff --git a/src/verifier.rs b/src/verifier.rs index 6a9d6762..e9a28024 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -390,8 +390,8 @@ impl Verifier for RequisiteVerifier { ebpf::CALL_IMM if sbpf_version.static_syscalls() && insn.src == 0 => { check_call_target(insn.imm as u32, syscall_registry)?; }, ebpf::CALL_IMM => {}, ebpf::CALL_REG => { check_callx_register(&insn, insn_ptr, sbpf_version)?; }, - ebpf::EXIT => {}, - ebpf::RETURN => {}, + ebpf::EXIT if !sbpf_version.static_syscalls() => {}, + ebpf::RETURN if sbpf_version.static_syscalls() => {}, _ => { return Err(VerifierError::UnknownOpCode(insn.opc, insn_ptr)); diff --git a/tests/execution.rs b/tests/execution.rs index c65bb78f..312e1c47 100644 --- a/tests/execution.rs +++ b/tests/execution.rs @@ -95,8 +95,8 @@ macro_rules! test_interpreter_and_jit { None ); match compilation_result { - Err(err) => assert_eq!( - format!("{:?}", err), + Err(_) => assert_eq!( + format!("{:?}", compilation_result), expected_result, "Unexpected result for JIT compilation" ), @@ -1436,54 +1436,86 @@ fn test_stxb_chain() { #[test] fn test_exit_capped() { - test_interpreter_and_jit_asm!( - " - exit", - [], - (), - TestContextObject::new(0), - ProgramResult::Err(EbpfError::ExceededMaxInstructions), - ); + for sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { + let config = Config { + enabled_sbpf_versions: sbpf_version..=sbpf_version, + ..Config::default() + }; + + test_interpreter_and_jit_asm!( + " + exit", + config, + [], + (), + TestContextObject::new(0), + ProgramResult::Err(EbpfError::ExceededMaxInstructions), + ); + } } #[test] fn test_exit_without_value() { - test_interpreter_and_jit_asm!( - " - exit", - [], - (), - TestContextObject::new(1), - ProgramResult::Ok(0x0), - ); + for sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { + let config = Config { + enabled_sbpf_versions: sbpf_version..=sbpf_version, + ..Config::default() + }; + + test_interpreter_and_jit_asm!( + " + exit", + config, + [], + (), + TestContextObject::new(1), + ProgramResult::Ok(0x0), + ); + } } #[test] fn test_exit() { - test_interpreter_and_jit_asm!( - " - mov r0, 0 - exit", - [], - (), - TestContextObject::new(2), - ProgramResult::Ok(0x0), - ); + for sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { + let config = Config { + enabled_sbpf_versions: sbpf_version..=sbpf_version, + ..Config::default() + }; + + test_interpreter_and_jit_asm!( + " + mov r0, 0 + exit", + config, + [], + (), + TestContextObject::new(2), + ProgramResult::Ok(0x0), + ); + } } #[test] fn test_early_exit() { - test_interpreter_and_jit_asm!( - " - mov r0, 3 - exit - mov r0, 4 - exit", - [], - (), - TestContextObject::new(2), - ProgramResult::Ok(0x3), - ); + for sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { + let config = Config { + enabled_sbpf_versions: sbpf_version..=sbpf_version, + ..Config::default() + }; + + test_interpreter_and_jit_asm!( + " + mov r0, 3 + exit + mov r0, 4 + exit", + config, + [], + (), + TestContextObject::new(2), + ProgramResult::Ok(0x3), + ); + } } #[test] @@ -4046,3 +4078,42 @@ fn test_mod() { ProgramResult::Err(EbpfError::DivideByZero), ); } + +#[test] +fn test_invalid_exit_or_return() { + for sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { + let inst = if sbpf_version == SBPFVersion::V1 { + 0x9d + } else { + 0x95 + }; + + let prog = &[ + 0xbf, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, // mov64 r0, 2 + inst, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit/return + ]; + + let config = Config { + enabled_sbpf_versions: sbpf_version..=sbpf_version, + enable_instruction_tracing: true, + ..Config::default() + }; + let function_registry = FunctionRegistry::>::default(); + let loader = Arc::new(BuiltinProgram::new_loader(config, function_registry)); + let mut executable = Executable::::from_text_bytes( + prog, + loader, + sbpf_version, + FunctionRegistry::default(), + ) + .unwrap(); + + test_interpreter_and_jit!( + false, + executable, + [], + TestContextObject::new(2), + ProgramResult::Err(EbpfError::UnsupportedInstruction), + ); + } +} diff --git a/tests/verifier.rs b/tests/verifier.rs index 99b4d73a..a7cf9421 100644 --- a/tests/verifier.rs +++ b/tests/verifier.rs @@ -425,3 +425,64 @@ fn test_sdiv_disabled() { } } } + +#[test] +fn return_instr() { + for sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { + let prog = &[ + 0xbf, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, // mov64 r0, 2 + 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit + 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // return + ]; + + let executable = Executable::::from_text_bytes( + prog, + Arc::new(BuiltinProgram::new_mock()), + sbpf_version, + FunctionRegistry::default(), + ) + .unwrap(); + let result = executable.verify::(); + if sbpf_version == SBPFVersion::V2 { + assert_error!(result, "VerifierError(UnknownOpCode(149, 1))"); + } else { + assert_error!(result, "VerifierError(UnknownOpCode(157, 2))"); + } + } +} + +#[test] +fn return_in_v2() { + let executable = assemble::( + "mov r0, 2 + return", + Arc::new(BuiltinProgram::new_loader( + Config { + enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2, + ..Config::default() + }, + FunctionRegistry::default(), + )), + ) + .unwrap(); + let result = executable.verify::(); + assert!(result.is_ok()); +} + +#[test] +fn function_without_return() { + let executable = assemble::( + "mov r0, 2 + add64 r0, 5", + Arc::new(BuiltinProgram::new_loader( + Config { + enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2, + ..Config::default() + }, + FunctionRegistry::default(), + )), + ) + .unwrap(); + let result = executable.verify::(); + assert_error!(result, "VerifierError(InvalidFunction(1))"); +}