From 13a268011201fed77348a62b90efb8eae1a2e80e Mon Sep 17 00:00:00 2001 From: Lucas Ste <38472950+LucasSte@users.noreply.github.com> Date: Thu, 10 Oct 2024 13:21:34 -0300 Subject: [PATCH] Introduce `return` instruction in the (dis)assembler (#609) * Introduce return instruction in the assembler --- src/assembler.rs | 11 +++++++-- src/disassembler.rs | 5 ++++- src/ebpf.rs | 4 +++- src/interpreter.rs | 4 ++-- src/jit.rs | 3 ++- src/verifier.rs | 3 ++- src/vm.rs | 2 +- test_utils/src/lib.rs | 2 +- tests/assembler.rs | 52 +++++++++++++++++++++++++++++++++++++++---- tests/disassembler.rs | 43 +++++++++++++++++++++++------------ tests/execution.rs | 8 +++---- tests/verifier.rs | 8 +++---- 12 files changed, 109 insertions(+), 36 deletions(-) diff --git a/src/assembler.rs b/src/assembler.rs index a9f50482..46a88e65 100644 --- a/src/assembler.rs +++ b/src/assembler.rs @@ -112,8 +112,15 @@ fn make_instruction_map(sbpf_version: SBPFVersion) -> HashMap( desc = format!("{name} {function_name}"); }, ebpf::CALL_REG => { name = "callx"; desc = format!("{} r{}", name, if sbpf_version.callx_uses_src_reg() { insn.src } else { insn.imm as u8 }); }, - ebpf::EXIT => { name = "exit"; desc = name.to_string(); }, + ebpf::EXIT + | ebpf::RETURN if !sbpf_version.static_syscalls() => { name = "exit"; desc = name.to_string(); }, + ebpf::EXIT + | ebpf::RETURN if sbpf_version.static_syscalls() => { name = "return"; desc = name.to_string(); }, _ => { name = "unknown"; desc = format!("{} opcode={:#x}", name, insn.opc); }, }; diff --git a/src/ebpf.rs b/src/ebpf.rs index b15a2b7f..6608452b 100644 --- a/src/ebpf.rs +++ b/src/ebpf.rs @@ -480,8 +480,10 @@ pub const JSLE_REG: u8 = BPF_JMP | BPF_X | BPF_JSLE; pub const CALL_IMM: u8 = BPF_JMP | BPF_CALL; /// BPF opcode: tail call. pub const CALL_REG: u8 = BPF_JMP | BPF_X | BPF_CALL; -/// BPF opcode: `exit` /// `return r0`. +/// BPF opcode: `exit` /// `return r0`. /// Valid only for SBPFv1 pub const EXIT: u8 = BPF_JMP | BPF_EXIT; +/// BPF opcode: `return` /// `return r0`. /// Valid only for SBPFv2 +pub const RETURN: u8 = BPF_JMP | BPF_X | BPF_EXIT; // Used in JIT /// Mask to extract the operation class from an operation code. diff --git a/src/interpreter.rs b/src/interpreter.rs index 1ed54209..8d1dee5c 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -565,8 +565,8 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> { throw_error!(self, EbpfError::UnsupportedInstruction); } } - - ebpf::EXIT => { + ebpf::RETURN + | ebpf::EXIT => { 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 f0291c94..78fd1a02 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -745,7 +745,8 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { }; self.emit_internal_call(Value::Register(target_pc)); }, - ebpf::EXIT => { + ebpf::RETURN + | ebpf::EXIT => { 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 779a7498..6a9d6762 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -236,7 +236,7 @@ impl Verifier for RequisiteVerifier { function_range.end = *function_iter.peek().unwrap_or(&program_range.end); let insn = ebpf::get_insn(prog, function_range.end.saturating_sub(1)); match insn.opc { - ebpf::JA | ebpf::EXIT => {}, + ebpf::JA | ebpf::RETURN => {}, _ => return Err(VerifierError::InvalidFunction( function_range.end.saturating_sub(1), )), @@ -391,6 +391,7 @@ impl Verifier for RequisiteVerifier { ebpf::CALL_IMM => {}, ebpf::CALL_REG => { check_callx_register(&insn, insn_ptr, sbpf_version)?; }, ebpf::EXIT => {}, + ebpf::RETURN => {}, _ => { return Err(VerifierError::UnknownOpCode(insn.opc, insn_ptr)); diff --git a/src/vm.rs b/src/vm.rs index f4a9a15d..7bff55c4 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -243,7 +243,7 @@ pub struct CallFrame { /// }; /// /// let prog = &[ -/// 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 // exit +/// 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 // exit /// ]; /// let mem = &mut [ /// 0xaa, 0xbb, 0x11, 0x22, 0xcc, 0xdd diff --git a/test_utils/src/lib.rs b/test_utils/src/lib.rs index 360b7c7e..0f8e1c2d 100644 --- a/test_utils/src/lib.rs +++ b/test_utils/src/lib.rs @@ -130,7 +130,7 @@ pub const TCP_SACK_BIN: [u8; 352] = [ 0x6d, 0x32, 0xee, 0xff, 0x00, 0x00, 0x00, 0x00, // 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, // 0xb7, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, // - 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // + 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ]; pub const TCP_SACK_MATCH: [u8; 78] = [ diff --git a/tests/assembler.rs b/tests/assembler.rs index 2898f7da..d94c2155 100644 --- a/tests/assembler.rs +++ b/tests/assembler.rs @@ -8,12 +8,19 @@ extern crate solana_rbpf; extern crate test_utils; +use solana_rbpf::program::{FunctionRegistry, SBPFVersion}; +use solana_rbpf::vm::Config; use solana_rbpf::{assembler::assemble, ebpf, program::BuiltinProgram, vm::TestContextObject}; use std::sync::Arc; use test_utils::{TCP_SACK_ASM, TCP_SACK_BIN}; fn asm(src: &str) -> Result, String> { - let executable = assemble::(src, Arc::new(BuiltinProgram::new_mock()))?; + asm_with_config(src, Config::default()) +} + +fn asm_with_config(src: &str, config: Config) -> Result, String> { + let loader = BuiltinProgram::new_loader(config, FunctionRegistry::default()); + let executable = assemble::(src, Arc::new(loader))?; let (_program_vm_addr, program) = executable.get_text_bytes(); Ok((0..program.len() / ebpf::INSN_SIZE) .map(|insn_ptr| ebpf::get_insn(program, insn_ptr)) @@ -50,7 +57,34 @@ fn test_fill() { // Example for InstructionType::NoOperand. #[test] fn test_exit() { - assert_eq!(asm("exit"), Ok(vec![insn(0, ebpf::EXIT, 0, 0, 0, 0)])); + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; + assert_eq!( + asm_with_config("exit", config.clone()), + Ok(vec![insn(0, ebpf::EXIT, 0, 0, 0, 0)]) + ); + assert_eq!( + asm_with_config("return", config), + Ok(vec![insn(0, ebpf::EXIT, 0, 0, 0, 0)]) + ); +} + +#[test] +fn test_return() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2, + ..Config::default() + }; + assert_eq!( + asm_with_config("exit", config.clone()), + Ok(vec![insn(0, ebpf::RETURN, 0, 0, 0, 0)]) + ); + assert_eq!( + asm_with_config("return", config), + Ok(vec![insn(0, ebpf::RETURN, 0, 0, 0, 0)]) + ); } // Example for InstructionType::AluBinary. @@ -471,8 +505,18 @@ fn test_large_immediate() { #[test] fn test_tcp_sack() { - let executable = - assemble::(TCP_SACK_ASM, Arc::new(BuiltinProgram::new_mock())).unwrap(); + let config = Config { + enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2, + ..Config::default() + }; + let executable = assemble::( + TCP_SACK_ASM, + Arc::new(BuiltinProgram::new_loader( + config, + FunctionRegistry::default(), + )), + ) + .unwrap(); let (_program_vm_addr, program) = executable.get_text_bytes(); assert_eq!(program, TCP_SACK_BIN.to_vec()); } diff --git a/tests/disassembler.rs b/tests/disassembler.rs index 6b3b25c3..0740787e 100644 --- a/tests/disassembler.rs +++ b/tests/disassembler.rs @@ -7,6 +7,7 @@ // copied, modified, or distributed except according to those terms. extern crate solana_rbpf; +use solana_rbpf::program::SBPFVersion; use solana_rbpf::{ assembler::assemble, program::{BuiltinProgram, FunctionRegistry}, @@ -18,14 +19,15 @@ use std::sync::Arc; // Using a macro to keep actual line numbers in failure output macro_rules! disasm { ($src:expr) => {{ + let config = Config { + enable_symbol_and_section_labels: true, + ..Config::default() + }; + disasm!($src, config); + }}; + ($src:expr, $config:expr) => {{ let src = $src; - let loader = BuiltinProgram::new_loader( - Config { - enable_symbol_and_section_labels: true, - ..Config::default() - }, - FunctionRegistry::default(), - ); + let loader = BuiltinProgram::new_loader($config, FunctionRegistry::default()); let executable = assemble::(src, Arc::new(loader)).unwrap(); let analysis = Analysis::from_executable(&executable).unwrap(); let mut reasm = Vec::new(); @@ -42,7 +44,20 @@ fn test_empty() { // Example for InstructionType::NoOperand. #[test] fn test_exit() { - disasm!("entrypoint:\n exit\n"); + let config = Config { + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, + ..Config::default() + }; + disasm!("entrypoint:\n exit\n", config); +} + +#[test] +fn test_return() { + let config = Config { + enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2, + ..Config::default() + }; + disasm!("entrypoint:\n return\n", config); } // Example for InstructionType::AluBinary. @@ -80,7 +95,7 @@ fn test_ja() { "entrypoint: ja lbb_1 lbb_1: - exit + return " ); } @@ -92,14 +107,14 @@ fn test_jeq() { "entrypoint: jeq r1, 4, lbb_1 lbb_1: - exit + return " ); disasm!( "entrypoint: jeq r1, r3, lbb_1 lbb_1: - exit + return " ); } @@ -112,7 +127,7 @@ fn test_call() { call function_1 function_1: - exit + return " ); } @@ -301,7 +316,7 @@ fn test_jump_conditional() { jslt r1, r2, lbb_11 jsle r1, r2, lbb_11 lbb_11: - exit + return " ); @@ -319,7 +334,7 @@ lbb_11: jslt r1, 2, lbb_11 jsle r1, 2, lbb_11 lbb_11: - exit + return " ); } diff --git a/tests/execution.rs b/tests/execution.rs index b3f52fdc..c65bb78f 100644 --- a/tests/execution.rs +++ b/tests/execution.rs @@ -687,7 +687,7 @@ fn test_pqr() { prog[24] = ebpf::HOR64_IMM; prog[25] = 1; // dst = R1 prog[33] = 16; // src = R1 - prog[40] = ebpf::EXIT; + prog[40] = ebpf::RETURN; let loader = Arc::new(BuiltinProgram::new_mock()); for (opc, dst, src, expected_result) in [ (ebpf::UHMUL64_IMM, 13u64, 4u64, 0u64), @@ -841,7 +841,7 @@ fn test_pqr() { fn test_err_divide_by_zero() { let mut prog = [0; 24]; prog[0] = ebpf::MOV32_IMM; - prog[16] = ebpf::EXIT; + prog[16] = ebpf::RETURN; let loader = Arc::new(BuiltinProgram::new_mock()); for opc in [ ebpf::UDIV32_REG, @@ -882,7 +882,7 @@ fn test_err_divide_overflow() { LittleEndian::write_i32(&mut prog[20..], -1); prog[25] = 16; // src = R1 LittleEndian::write_i32(&mut prog[28..], -1); - prog[32] = ebpf::EXIT; + prog[32] = ebpf::RETURN; let loader = Arc::new(BuiltinProgram::new_mock()); for opc in [ ebpf::SDIV32_IMM, @@ -2261,7 +2261,7 @@ fn test_err_mem_access_out_of_bound() { prog[0] = ebpf::MOV32_IMM; prog[8] = ebpf::HOR64_IMM; prog[16] = ebpf::ST_1B_IMM; - prog[24] = ebpf::EXIT; + prog[24] = ebpf::RETURN; let loader = Arc::new(BuiltinProgram::new_mock()); for address in [0x2u64, 0x8002u64, 0x80000002u64, 0x8000000000000002u64] { LittleEndian::write_u32(&mut prog[4..], address as u32); diff --git a/tests/verifier.rs b/tests/verifier.rs index 854ab8d4..99b4d73a 100644 --- a/tests/verifier.rs +++ b/tests/verifier.rs @@ -122,7 +122,7 @@ fn test_verifier_err_endian_size() { let prog = &[ 0xdc, 0x01, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, // 0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // - 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // + 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ]; let executable = Executable::::from_text_bytes( prog, @@ -290,7 +290,7 @@ fn test_verifier_err_jmp_out_start() { fn test_verifier_err_unknown_opcode() { let prog = &[ 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // - 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // + 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ]; let executable = Executable::::from_text_bytes( prog, @@ -307,7 +307,7 @@ fn test_verifier_err_unknown_opcode() { fn test_verifier_unknown_sycall() { let prog = &[ 0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe - 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, @@ -323,7 +323,7 @@ fn test_verifier_unknown_sycall() { fn test_verifier_known_syscall() { let prog = &[ 0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe - 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit + 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // return ]; let mut function_registry = FunctionRegistry::>::default(); function_registry