diff --git a/src/assembler.rs b/src/assembler.rs index 3e6ab63b..9727a141 100644 --- a/src/assembler.rs +++ b/src/assembler.rs @@ -212,7 +212,7 @@ fn insn(opc: u8, dst: i64, src: i64, off: i64, imm: i64) -> Result /// ``` pub fn assemble( src: &str, - verifier: Option>, + verifier: Option, config: Config, ) -> Result>, String> { fn resolve_label( diff --git a/src/ebpf.rs b/src/ebpf.rs index d88a006d..c92f6a3d 100644 --- a/src/ebpf.rs +++ b/src/ebpf.rs @@ -25,8 +25,6 @@ use std::fmt; pub const PROG_MAX_INSNS: usize = 65_536; /// Size of an eBPF instructions, in bytes. pub const INSN_SIZE: usize = 8; -/// Maximum size of an eBPF program, in bytes. -pub const PROG_MAX_SIZE: usize = PROG_MAX_INSNS * INSN_SIZE; /// Stack register pub const STACK_REG: usize = 10; /// First scratch register diff --git a/src/error.rs b/src/error.rs index b221a13a..761e253f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -17,7 +17,7 @@ //! , or for a shorter version of //! the list of the operation codes: -use crate::{elf::ElfError, memory_region::AccessType}; +use crate::{elf::ElfError, memory_region::AccessType, verifier::VerifierError}; /// User defined errors must implement this trait pub trait UserDefinedError: 'static + std::error::Error {} @@ -86,4 +86,7 @@ pub enum EbpfError { /// Compilation is too big to fit #[error("Compilation exhaused text segment at instruction {0}")] ExhausedTextSegment(usize), + /// ELF error + #[error("Verifier error: {0}")] + VerifierError(#[from] VerifierError), } diff --git a/src/verifier.rs b/src/verifier.rs index 9879daa5..bfffbeec 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -21,44 +21,42 @@ //! //! Contrary to the verifier of the Linux kernel, this one does not modify the bytecode at all. -use crate::{ebpf, error::UserDefinedError, user_error::UserError}; +use crate::ebpf; +use thiserror::Error; /// Error definitions -#[derive(Debug, thiserror::Error, PartialEq, Eq)] +#[derive(Debug, Error, Eq, PartialEq)] pub enum VerifierError { /// ProgramLengthNotMultiple #[error("program length must be a multiple of {} octets", ebpf::INSN_SIZE)] ProgramLengthNotMultiple, - /// ProgramTooLarge - #[error("program too big, max {}, is {}", ebpf::PROG_MAX_INSNS, .0)] + /// Deprecated + #[error("Deprecated")] ProgramTooLarge(usize), /// NoProgram #[error("no program set, call prog_set() to load one")] NoProgram, - ///InvalidLastInstruction - #[error("program does not end with “EXIT” instruction")] - InvalidLastInstruction, - /// DivisionByZero + /// Division by zero #[error("division by 0 (insn #{0})")] DivisionByZero(usize), - /// UnsupportedLeBeArgument + /// UnsupportedLEBEArgument #[error("unsupported argument for LE/BE (insn #{0})")] - UnsupportedLeBeArgument(usize), - /// LddwCannotBeLast + UnsupportedLEBEArgument(usize), + /// LDDWCannotBeLast #[error("LD_DW instruction cannot be last in program")] - LddwCannotBeLast, - /// IncompleteLddw + LDDWCannotBeLast, + /// IncompleteLDDW #[error("incomplete LD_DW instruction (insn #{0})")] - IncompleteLddw(usize), + IncompleteLDDW(usize), /// InfiniteLoop #[error("infinite loop (insn #{0})")] InfiniteLoop(usize), /// JumpOutOfCode #[error("jump out of code to #{0} (insn #{1})")] JumpOutOfCode(usize, usize), - /// JumpToMiddleOfLddw + /// JumpToMiddleOfLDDW #[error("jump to middle of LD_DW at #{0} (insn #{1})")] - JumpToMiddleOfLddw(usize, usize), + JumpToMiddleOfLDDW(usize, usize), /// InvalidSourceRegister #[error("invalid source register (insn #{0})")] InvalidSourceRegister(usize), @@ -78,35 +76,24 @@ pub enum VerifierError { #[error("Invalid register specified at instruction {0}")] InvalidRegister(usize), } -impl UserDefinedError for VerifierError {} -impl From for UserError { - fn from(error: VerifierError) -> Self { - UserError::VerifierError(error) - } + +fn adj_insn_ptr(insn_ptr: usize) -> usize { + insn_ptr + ebpf::ELF_INSN_DUMP_OFFSET } fn check_prog_len(prog: &[u8]) -> Result<(), VerifierError> { if prog.len() % ebpf::INSN_SIZE != 0 { return Err(VerifierError::ProgramLengthNotMultiple); } - if prog.len() > ebpf::PROG_MAX_SIZE { - return Err(VerifierError::ProgramTooLarge(prog.len() / ebpf::INSN_SIZE)); - } - if prog.is_empty() { return Err(VerifierError::NoProgram); } - let last_insn = ebpf::get_insn(prog, (prog.len() / ebpf::INSN_SIZE) - 1); - if last_insn.opc != ebpf::EXIT { - return Err(VerifierError::InvalidLastInstruction); - } - Ok(()) } fn check_imm_nonzero(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierError> { if insn.imm == 0 { - return Err(VerifierError::DivisionByZero(insn_ptr)); + return Err(VerifierError::DivisionByZero(adj_insn_ptr(insn_ptr))); } Ok(()) } @@ -114,38 +101,42 @@ fn check_imm_nonzero(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierE fn check_imm_endian(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierError> { match insn.imm { 16 | 32 | 64 => Ok(()), - _ => Err(VerifierError::UnsupportedLeBeArgument(insn_ptr)), + _ => Err(VerifierError::UnsupportedLEBEArgument(adj_insn_ptr( + insn_ptr, + ))), } } fn check_load_dw(prog: &[u8], insn_ptr: usize) -> Result<(), VerifierError> { - // We know we can reach next insn since we enforce an EXIT insn at the end of program, while - // this function should be called only for LD_DW insn, that cannot be last in program. + if insn_ptr + 1 >= (prog.len() / ebpf::INSN_SIZE) { + // Last instruction cannot be LD_DW because there would be no 2nd DW + return Err(VerifierError::LDDWCannotBeLast); + } let next_insn = ebpf::get_insn(prog, insn_ptr + 1); if next_insn.opc != 0 { - return Err(VerifierError::IncompleteLddw(insn_ptr)); + return Err(VerifierError::IncompleteLDDW(adj_insn_ptr(insn_ptr))); } Ok(()) } fn check_jmp_offset(prog: &[u8], insn_ptr: usize) -> Result<(), VerifierError> { let insn = ebpf::get_insn(prog, insn_ptr); - if insn.off == -1 { - return Err(VerifierError::InfiniteLoop(insn_ptr)); - } + // if insn.off == -1 { + // return Err(VerifierError::InfiniteLoop(adj_insn_ptr(insn_ptr)).into()); + // } let dst_insn_ptr = insn_ptr as isize + 1 + insn.off as isize; if dst_insn_ptr < 0 || dst_insn_ptr as usize >= (prog.len() / ebpf::INSN_SIZE) { return Err(VerifierError::JumpOutOfCode( dst_insn_ptr as usize, - insn_ptr, + adj_insn_ptr(insn_ptr), )); } let dst_insn = ebpf::get_insn(prog, dst_insn_ptr as usize); if dst_insn.opc == 0 { - return Err(VerifierError::JumpToMiddleOfLddw( + return Err(VerifierError::JumpToMiddleOfLDDW( dst_insn_ptr as usize, - insn_ptr, + adj_insn_ptr(insn_ptr), )); } Ok(()) @@ -153,19 +144,21 @@ fn check_jmp_offset(prog: &[u8], insn_ptr: usize) -> Result<(), VerifierError> { fn check_registers(insn: &ebpf::Insn, store: bool, insn_ptr: usize) -> Result<(), VerifierError> { if insn.src > 10 { - return Err(VerifierError::InvalidSourceRegister(insn_ptr)); + return Err(VerifierError::InvalidSourceRegister(adj_insn_ptr(insn_ptr))); } match (insn.dst, store) { (0..=9, _) | (10, true) => Ok(()), - (10, false) => Err(VerifierError::CannotWriteR10(insn_ptr)), - (_, _) => Err(VerifierError::InvalidDestinationRegister(insn_ptr)), + (10, false) => Err(VerifierError::CannotWriteR10(adj_insn_ptr(insn_ptr))), + (_, _) => Err(VerifierError::InvalidDestinationRegister(adj_insn_ptr( + insn_ptr, + ))), } } /// Check that the imm is a valid shift operand fn check_imm_shift(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierError> { if insn.imm < 0 || insn.imm as u64 >= 64 { - return Err(VerifierError::ShiftWithOverflow(insn_ptr)); + return Err(VerifierError::ShiftWithOverflow(adj_insn_ptr(insn_ptr))); } Ok(()) } @@ -173,14 +166,14 @@ fn check_imm_shift(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierErr /// Check that the imm is a valid register number fn check_imm_register(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), VerifierError> { if insn.imm < 0 || insn.imm > 10 { - return Err(VerifierError::InvalidRegister(insn_ptr)); + return Err(VerifierError::InvalidRegister(adj_insn_ptr(insn_ptr))); } Ok(()) } -/// Default eBPF verifier +/// Check the program against the verifier's rules #[rustfmt::skip] -pub fn check(prog: &[u8]) -> Result<(), UserError> { +pub fn check(prog: &[u8]) -> Result<(), VerifierError> { check_prog_len(prog)?; let mut insn_ptr: usize = 0; @@ -201,7 +194,6 @@ pub fn check(prog: &[u8]) -> Result<(), UserError> { ebpf::LD_IND_DW => {}, ebpf::LD_DW_IMM => { - store = true; check_load_dw(prog, insn_ptr)?; insn_ptr += 1; }, @@ -258,7 +250,7 @@ pub fn check(prog: &[u8]) -> Result<(), UserError> { ebpf::ADD64_REG => {}, ebpf::SUB64_IMM => {}, ebpf::SUB64_REG => {}, - ebpf::MUL64_IMM => {}, + ebpf::MUL64_IMM => { check_imm_nonzero(&insn, insn_ptr)?; }, ebpf::MUL64_REG => {}, ebpf::DIV64_IMM => { check_imm_nonzero(&insn, insn_ptr)?; }, ebpf::DIV64_REG => {}, @@ -309,7 +301,7 @@ pub fn check(prog: &[u8]) -> Result<(), UserError> { ebpf::EXIT => {}, _ => { - return Err(VerifierError::UnknownOpCode(insn.opc, insn_ptr).into()); + return Err(VerifierError::UnknownOpCode(insn.opc, adj_insn_ptr(insn_ptr))); } } @@ -320,7 +312,7 @@ pub fn check(prog: &[u8]) -> Result<(), UserError> { // insn_ptr should now be equal to number of instructions. if insn_ptr != prog.len() / ebpf::INSN_SIZE { - return Err(VerifierError::JumpOutOfCode(insn_ptr, insn_ptr).into()); + return Err(VerifierError::JumpOutOfCode(adj_insn_ptr(insn_ptr), adj_insn_ptr(insn_ptr))); } Ok(()) diff --git a/src/vm.rs b/src/vm.rs index ed278f07..c61b0f88 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -20,6 +20,7 @@ use crate::{ jit::{JitProgram, JitProgramArgument}, memory_region::{AccessType, MemoryMapping, MemoryRegion}, user_error::UserError, + verifier::VerifierError, }; use log::debug; use std::{ @@ -36,7 +37,7 @@ use std::{ /// - Unknown instructions. /// - Bad formed instruction. /// - Unknown eBPF syscall index. -pub type Verifier = fn(prog: &[u8]) -> Result<(), E>; +pub type Verifier = fn(prog: &[u8]) -> Result<(), VerifierError>; /// Return value of programs and syscalls pub type ProgramResult = Result>; @@ -225,7 +226,7 @@ impl dyn Executable { /// Creates a post relocaiton/fixup executable from an ELF file pub fn from_elf( elf_bytes: &[u8], - verifier: Option>, + verifier: Option, config: Config, ) -> Result, EbpfError> { let ebpf_elf = EBpfElf::load(config, elf_bytes)?; @@ -239,11 +240,11 @@ impl dyn Executable { pub fn from_text_bytes( text_bytes: &[u8], bpf_functions: BTreeMap, - verifier: Option>, + verifier: Option, config: Config, ) -> Result, EbpfError> { if let Some(verifier) = verifier { - verifier(text_bytes)?; + verifier(text_bytes).map_err(EbpfError::VerifierError)?; } Ok(Box::new(EBpfElf::new_from_text_bytes( config, diff --git a/tests/ubpf_execution.rs b/tests/ubpf_execution.rs index 2841807e..518341fa 100644 --- a/tests/ubpf_execution.rs +++ b/tests/ubpf_execution.rs @@ -14,16 +14,14 @@ extern crate thiserror; use rand::{rngs::SmallRng, RngCore, SeedableRng}; use solana_rbpf::{ assembler::assemble, - ebpf, - elf::{register_bpf_function, ElfError}, + elf::ElfError, error::EbpfError, memory_region::AccessType, syscalls, user_error::UserError, - verifier::check, - vm::{Config, DefaultInstructionMeter, EbpfVm, Executable, SyscallObject, SyscallRegistry}, + vm::{Config, EbpfVm, Executable, SyscallObject, SyscallRegistry}, }; -use std::{collections::BTreeMap, fs::File, io::Read}; +use std::{fs::File, io::Read}; use test_utils::{ BpfSyscallString, BpfSyscallU64, BpfTracePrintf, Result, SyscallWithContext, TestInstructionMeter, PROG_TCP_PORT_80, TCP_SACK_ASM, TCP_SACK_MATCH, TCP_SACK_NOMATCH, @@ -2412,6 +2410,24 @@ fn test_err_static_jmp_lddw() { }, 2 ); + test_interpreter_and_jit_asm!( + " + mov r0, 0 + mov r1, 0 + mov r2, 0 + lddw r0, 0x1 + ja +2 + lddw r1, 0x1 + lddw r2, 0x1 + add r1, r2 + add r0, r1 + exit + ", + [], + (), + { |_vm, res: Result| { res.unwrap() == 0x2 } }, + 9 + ); test_interpreter_and_jit_asm!( " jeq r0, 0, 1 @@ -3247,70 +3263,13 @@ fn test_tcp_sack_nomatch() { ); } -#[test] -fn test_large_program() { - let mut insns = vec![ebpf::Insn::default(); ebpf::PROG_MAX_INSNS * 2 - 1]; - for index in (0..(ebpf::PROG_MAX_INSNS * 2 - 1)).step_by(2) { - insns[index].opc = ebpf::LD_DW_IMM; - } - insns[ebpf::PROG_MAX_INSNS - 4].opc = ebpf::JA; - insns[ebpf::PROG_MAX_INSNS - 4].off = 9; - insns[ebpf::PROG_MAX_INSNS - 3].opc = ebpf::MOV64_IMM; - insns[ebpf::PROG_MAX_INSNS * 2 - 2].opc = ebpf::EXIT; - - let config = Config { - enable_instruction_tracing: true, - ..Config::default() - }; - - let mut program = insns - .iter() - .map(|insn| insn.to_vec()) - .flatten() - .collect::>(); - #[allow(unused_mut)] - { - let mut bpf_functions = BTreeMap::new(); - register_bpf_function(&mut bpf_functions, 0, "entrypoint").unwrap(); - let mut executable = >::from_text_bytes( - program.as_slice(), - bpf_functions, - None, - config, - ) - .unwrap(); - test_interpreter_and_jit!( - executable, - [], - (), - { |_vm, res: Result| res.unwrap() == 0x0 }, - ebpf::PROG_MAX_INSNS as u64 - 4 - ); - } - - { - // Test program that is too large - let exit_insn = program[(ebpf::PROG_MAX_INSNS * 2 - 2) * ebpf::INSN_SIZE - ..(ebpf::PROG_MAX_INSNS * 2 - 1) * ebpf::INSN_SIZE] - .to_vec(); - program.extend_from_slice(exit_insn.as_slice()); - - assert!( - >::from_text_bytes( - program.as_slice(), - BTreeMap::new(), - Some(check), - config, - ) - .is_err() - ); - } -} - // Fuzzy #[cfg(not(windows))] fn execute_generated_program(prog: &[u8]) -> bool { + use solana_rbpf::{elf::register_bpf_function, verifier::check}; + use std::collections::BTreeMap; + let max_instruction_count = 1024; let mem_size = 1024 * 1024; let mut bpf_functions = BTreeMap::new(); @@ -3375,6 +3334,7 @@ fn execute_generated_program(prog: &[u8]) -> bool { #[cfg(not(windows))] #[test] fn test_total_chaos() { + use solana_rbpf::ebpf; let instruction_count = 6; let iteration_count = 1000000; let mut program = vec![0; instruction_count * ebpf::INSN_SIZE]; diff --git a/tests/ubpf_verifier.rs b/tests/ubpf_verifier.rs index 503309a7..107348e1 100644 --- a/tests/ubpf_verifier.rs +++ b/tests/ubpf_verifier.rs @@ -24,10 +24,9 @@ extern crate thiserror; use solana_rbpf::{ assembler::assemble, - ebpf, error::UserDefinedError, user_error::UserError, - verifier::check, + verifier::{check, VerifierError}, vm::{Config, DefaultInstructionMeter, EbpfVm, Executable}, }; use std::collections::BTreeMap; @@ -43,7 +42,7 @@ impl UserDefinedError for VerifierTestError {} #[test] fn test_verifier_success() { - let executable = assemble::( + let executable = assemble::( " mov32 r0, 0xBEE exit", @@ -51,21 +50,17 @@ fn test_verifier_success() { Config::default(), ) .unwrap(); - let _vm = EbpfVm::::new( - executable.as_ref(), - &mut [], - &[], - ) - .unwrap(); + let _vm = EbpfVm::::new(executable.as_ref(), &mut [], &[]) + .unwrap(); } #[test] -#[should_panic(expected = "Gaggablaghblagh!")] +#[should_panic(expected = "NoProgram")] fn test_verifier_fail() { - fn verifier_fail(_prog: &[u8]) -> Result<(), VerifierTestError> { - Err(VerifierTestError::Rejected("Gaggablaghblagh!".to_string())) + fn verifier_fail(_prog: &[u8]) -> Result<(), VerifierError> { + Err(VerifierError::NoProgram) } - let _executable = assemble::( + let _executable = assemble::( " mov32 r0, 0xBEE exit", @@ -76,7 +71,7 @@ fn test_verifier_fail() { } #[test] -#[should_panic(expected = "DivisionByZero(1)")] +#[should_panic(expected = "DivisionByZero(30)")] fn test_verifier_err_div_by_zero_imm() { let _executable = assemble::( " @@ -90,7 +85,7 @@ fn test_verifier_err_div_by_zero_imm() { } #[test] -#[should_panic(expected = "UnsupportedLeBeArgument(0)")] +#[should_panic(expected = "UnsupportedLEBEArgument(29)")] fn test_verifier_err_endian_size() { let prog = &[ 0xdc, 0x01, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, // @@ -107,7 +102,7 @@ fn test_verifier_err_endian_size() { } #[test] -#[should_panic(expected = "IncompleteLddw(0)")] +#[should_panic(expected = "IncompleteLDDW(29)")] fn test_verifier_err_incomplete_lddw() { // Note: ubpf has test-err-incomplete-lddw2, which is the same let prog = &[ @@ -124,20 +119,7 @@ fn test_verifier_err_incomplete_lddw() { } #[test] -#[should_panic(expected = "InfiniteLoop(0)")] -fn test_verifier_err_infinite_loop() { - let _executable = assemble::( - " - ja -1 - exit", - Some(check), - Config::default(), - ) - .unwrap(); -} - -#[test] -#[should_panic(expected = "InvalidDestinationRegister(0)")] +#[should_panic(expected = "InvalidDestinationRegister(29)")] fn test_verifier_err_invalid_reg_dst() { let _executable = assemble::( " @@ -150,7 +132,7 @@ fn test_verifier_err_invalid_reg_dst() { } #[test] -#[should_panic(expected = "InvalidSourceRegister(0)")] +#[should_panic(expected = "InvalidSourceRegister(29)")] fn test_verifier_err_invalid_reg_src() { let _executable = assemble::( " @@ -163,7 +145,7 @@ fn test_verifier_err_invalid_reg_src() { } #[test] -#[should_panic(expected = "JumpToMiddleOfLddw(2, 0)")] +#[should_panic(expected = "JumpToMiddleOfLDDW(2, 29)")] fn test_verifier_err_jmp_lddw() { let _executable = assemble::( " @@ -177,7 +159,7 @@ fn test_verifier_err_jmp_lddw() { } #[test] -#[should_panic(expected = "JumpOutOfCode(3, 0)")] +#[should_panic(expected = "JumpOutOfCode(3, 29)")] fn test_verifier_err_jmp_out() { let _executable = assemble::( " @@ -190,40 +172,7 @@ fn test_verifier_err_jmp_out() { } #[test] -#[should_panic(expected = "InvalidLastInstruction")] -fn test_verifier_err_no_exit() { - let _executable = assemble::( - " - mov32 r0, 0", - Some(check), - Config::default(), - ) - .unwrap(); -} - -#[test] -#[should_panic(expected = "ProgramTooLarge(65537)")] -fn test_verifier_err_too_many_instructions() { - let mut prog = (0..(65536 * ebpf::INSN_SIZE)) - .map(|x| match x % 8 { - 0 => 0xb7, - 1 => 0x01, - _ => 0, - }) - .collect::>(); - prog.append(&mut vec![0x95, 0, 0, 0, 0, 0, 0, 0]); - - let _ = >::from_text_bytes( - &prog, - BTreeMap::new(), - Some(check), - Config::default(), - ) - .unwrap(); -} - -#[test] -#[should_panic(expected = "UnknownOpCode(6, 0)")] +#[should_panic(expected = "UnknownOpCode(6, 29)")] fn test_verifier_err_unknown_opcode() { let prog = &[ 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // @@ -239,7 +188,7 @@ fn test_verifier_err_unknown_opcode() { } #[test] -#[should_panic(expected = "CannotWriteR10(0)")] +#[should_panic(expected = "CannotWriteR10(29)")] fn test_verifier_err_write_r10() { let _executable = assemble::( "