From dae28b9cfe4942875d1b099ee8513a70cf099592 Mon Sep 17 00:00:00 2001 From: Jack May Date: Thu, 26 Mar 2020 14:00:26 -0700 Subject: [PATCH] Bump rBPF to v0.1.24, update rBPF/BPF Loader error handling (#9089) --- Cargo.lock | 7 +- programs/bpf/Cargo.lock | 9 +- programs/bpf/Cargo.toml | 2 +- programs/bpf/benches/bpf_loader.rs | 10 +-- programs/bpf_loader/Cargo.toml | 2 +- programs/bpf_loader/src/bpf_verifier.rs | 114 +++++++++++------------- programs/bpf_loader/src/helpers.rs | 67 +++++++------- programs/bpf_loader/src/lib.rs | 54 +++++++---- 8 files changed, 142 insertions(+), 123 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 35c382e8a6..a89c9828de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3740,7 +3740,7 @@ dependencies = [ "num-traits 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "solana-logger 1.1.0", "solana-sdk 1.1.0", - "solana_rbpf 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)", + "solana_rbpf 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)", "thiserror 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -5078,7 +5078,7 @@ dependencies = [ [[package]] name = "solana_rbpf" -version = "0.1.23" +version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "byteorder 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -5088,6 +5088,7 @@ dependencies = [ "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", + "thiserror 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)", "time 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -6648,7 +6649,7 @@ dependencies = [ "checksum solana_libra_vm_cache_map 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)" = "37fa2e1f00a87514cd2169149a5f81a89279703b2523979688d6ef84081a4690" "checksum solana_libra_vm_runtime 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)" = "ff9f8a7b8212dc4ece5d93f2839896e633c34d7463856e4a555cbcb5c67e9c26" "checksum solana_libra_vm_runtime_types 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)" = "254c23c8f30e7c82ae4dc6694e743400d674c66d371b700eec03378ba994f00b" -"checksum solana_rbpf 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)" = "832c41d1f4184a9554c41aa29dbeb0d5f8873d72cf2be32411668bc5f047a150" +"checksum solana_rbpf 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)" = "3e73fe3cf1da0881709bf32e9a22d4894ce5fa317c9a6284c2b5d8c25be000b8" "checksum sourcefile 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "4bf77cb82ba8453b42b6ae1d692e4cdc92f9a47beaf89a847c8be83f4e328ad3" "checksum spin 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" "checksum stable_deref_trait 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dba1a27d3efae4351c8051072d619e3ade2820635c3958d826bfea39d59b54c8" diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 848f3b382d..28350a323e 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -1736,7 +1736,7 @@ dependencies = [ "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "solana-logger 1.1.0", "solana-sdk 1.1.0", - "solana_rbpf 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)", + "solana_rbpf 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)", "thiserror 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1751,7 +1751,7 @@ dependencies = [ "solana-logger 1.1.0", "solana-runtime 1.1.0", "solana-sdk 1.1.0", - "solana_rbpf 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)", + "solana_rbpf 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)", "walkdir 2.2.9 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2091,7 +2091,7 @@ dependencies = [ [[package]] name = "solana_rbpf" -version = "0.1.23" +version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "byteorder 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2101,6 +2101,7 @@ dependencies = [ "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", + "thiserror 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", "time 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2962,7 +2963,7 @@ dependencies = [ "checksum sha2 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "27044adfd2e1f077f649f59deb9490d3941d674002f7d062870a60ebe9bd47a0" "checksum slab 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" "checksum smallvec 0.6.10 (registry+https://github.com/rust-lang/crates.io-index)" = "ab606a9c5e214920bb66c458cd7be8ef094f813f20fe77a54cc7dbfff220d4b7" -"checksum solana_rbpf 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)" = "832c41d1f4184a9554c41aa29dbeb0d5f8873d72cf2be32411668bc5f047a150" +"checksum solana_rbpf 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)" = "3e73fe3cf1da0881709bf32e9a22d4894ce5fa317c9a6284c2b5d8c25be000b8" "checksum sourcefile 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "4bf77cb82ba8453b42b6ae1d692e4cdc92f9a47beaf89a847c8be83f4e328ad3" "checksum spin 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" "checksum stable_deref_trait 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dba1a27d3efae4351c8051072d619e3ade2820635c3958d826bfea39d59b54c8" diff --git a/programs/bpf/Cargo.toml b/programs/bpf/Cargo.toml index 5b2fb3c040..74e8849b60 100644 --- a/programs/bpf/Cargo.toml +++ b/programs/bpf/Cargo.toml @@ -26,7 +26,7 @@ solana-bpf-loader-program = { path = "../bpf_loader", version = "1.1.0" } solana-logger = { path = "../../logger", version = "1.1.0" } solana-runtime = { path = "../../runtime", version = "1.1.0" } solana-sdk = { path = "../../sdk", version = "1.1.0" } -solana_rbpf = "=0.1.23" +solana_rbpf = "=0.1.24" [[bench]] name = "bpf_loader" diff --git a/programs/bpf/benches/bpf_loader.rs b/programs/bpf/benches/bpf_loader.rs index 46e9703681..9d9f1136cc 100644 --- a/programs/bpf/benches/bpf_loader.rs +++ b/programs/bpf/benches/bpf_loader.rs @@ -3,8 +3,8 @@ extern crate test; use byteorder::{ByteOrder, LittleEndian, WriteBytesExt}; -use solana_rbpf::EbpfVm; -use std::{env, fs::File, io::Error, io::Read, mem, path::PathBuf}; +use solana_rbpf::{EbpfVm}; +use std::{env, fs::File, io::Read, mem, path::PathBuf}; use test::Bencher; /// BPF program file extension @@ -21,7 +21,7 @@ fn create_bpf_path(name: &str) -> PathBuf { pathbuf } -fn empty_check(_prog: &[u8]) -> Result<(), Error> { +fn empty_check(_prog: &[u8]) -> Result<(), solana_bpf_loader_program::BPFError> { Ok(()) } @@ -39,7 +39,7 @@ const ARMSTRONG_EXPECTED: u64 = 5; #[bench] fn bench_program_load_elf(bencher: &mut Bencher) { let elf = load_elf().unwrap(); - let mut vm = EbpfVm::new(None).unwrap(); + let mut vm = EbpfVm::::new(None).unwrap(); vm.set_verifier(empty_check).unwrap(); bencher.iter(|| { @@ -50,7 +50,7 @@ fn bench_program_load_elf(bencher: &mut Bencher) { #[bench] fn bench_program_verify(bencher: &mut Bencher) { let elf = load_elf().unwrap(); - let mut vm = EbpfVm::new(None).unwrap(); + let mut vm = EbpfVm::::new(None).unwrap(); vm.set_verifier(empty_check).unwrap(); vm.set_elf(&elf).unwrap(); diff --git a/programs/bpf_loader/Cargo.toml b/programs/bpf_loader/Cargo.toml index d5f0649363..2c7f05f53a 100644 --- a/programs/bpf_loader/Cargo.toml +++ b/programs/bpf_loader/Cargo.toml @@ -17,7 +17,7 @@ num-derive = { version = "0.3" } num-traits = { version = "0.2" } solana-logger = { path = "../../logger", version = "1.1.0" } solana-sdk = { path = "../../sdk", version = "1.1.0" } -solana_rbpf = "=0.1.23" +solana_rbpf = "=0.1.24" thiserror = "1.0" [lib] diff --git a/programs/bpf_loader/src/bpf_verifier.rs b/programs/bpf_loader/src/bpf_verifier.rs index c12bf3735c..718300ef2b 100644 --- a/programs/bpf_loader/src/bpf_verifier.rs +++ b/programs/bpf_loader/src/bpf_verifier.rs @@ -1,111 +1,108 @@ +use crate::BPFError; use solana_rbpf::ebpf; -use std::io::{Error, ErrorKind}; +use thiserror::Error; -fn reject>(msg: S) -> Result<(), Error> { - let full_msg = format!("[Verifier] Error: {}", msg.as_ref()); - Err(Error::new(ErrorKind::Other, full_msg)) +#[derive(Debug, Error)] +pub enum VerifierError { + #[error("program length must be a multiple of {} octets", ebpf::INSN_SIZE)] + ProgramLengthNotMultiple, + #[error("program too big, max {}, is {}", ebpf::PROG_MAX_INSNS, .0)] + ProgramTooLarge(usize), + #[error("no program set, call prog_set() to load one")] + NoProgram, + #[error("division by 0 (insn #{0})")] + DivisionByZero(usize), + #[error("unsupported argument for LE/BE (insn #{0})")] + UnsupportedLEBEArgument(usize), + #[error("LD_DW instruction cannot be last in program")] + LDDWCannotBeLast, + #[error("incomplete LD_DW instruction (insn #{0})")] + IncompleteLDDW(usize), + #[error("infinite loop (insn #{0})")] + InfiniteLoop(usize), + #[error("jump out of code to #{0} (insn #{1})")] + JumpOutOfCode(usize, usize), + #[error("jump to middle of LD_DW at #{0} (insn #{1})")] + JumpToMiddleOfLDDW(usize, usize), + #[error("invalid source register (insn #{0})")] + InvalidSourceRegister(usize), + #[error("cannot write into register r10 (insn #{0})")] + CannotWriteR10(usize), + #[error("invalid destination register (insn #{0})")] + InvalidDestinationRegister(usize), + #[error("unknown eBPF opcode {0:#2x} (insn #{1:?})")] + UnknownOpCode(u8, usize), } -fn check_prog_len(prog: &[u8]) -> Result<(), Error> { +fn check_prog_len(prog: &[u8]) -> Result<(), BPFError> { if prog.len() % ebpf::INSN_SIZE != 0 { - reject(format!( - "eBPF program length must be a multiple of {:?} octets", - ebpf::INSN_SIZE - ))?; + return Err(VerifierError::ProgramLengthNotMultiple.into()); } if prog.len() > ebpf::PROG_MAX_SIZE { - reject(format!( - "eBPF program length limited to {:?}, here {:?}", - ebpf::PROG_MAX_INSNS, - prog.len() / ebpf::INSN_SIZE - ))?; + return Err(VerifierError::ProgramTooLarge(prog.len() / ebpf::INSN_SIZE).into()); } if prog.is_empty() { - reject("No program set, call prog_set() to load one".to_string())?; + return Err(VerifierError::NoProgram.into()); } - Ok(()) } -fn check_imm_nonzero(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), Error> { +fn check_imm_nonzero(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), BPFError> { if insn.imm == 0 { - reject(format!("division by 0 (insn #{:?})", insn_ptr))?; + return Err(VerifierError::DivisionByZero(insn_ptr).into()); } - Ok(()) } -fn check_imm_endian(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), Error> { +fn check_imm_endian(insn: &ebpf::Insn, insn_ptr: usize) -> Result<(), BPFError> { match insn.imm { 16 | 32 | 64 => Ok(()), - _ => reject(format!( - "unsupported argument for LE/BE (insn #{:?})", - insn_ptr - )), + _ => Err(VerifierError::UnsupportedLEBEArgument(insn_ptr).into()), } } -fn check_load_dw(prog: &[u8], insn_ptr: usize) -> Result<(), Error> { +fn check_load_dw(prog: &[u8], insn_ptr: usize) -> Result<(), BPFError> { if insn_ptr >= (prog.len() / ebpf::INSN_SIZE) { // Last instruction cannot be LD_DW because there would be no 2nd DW - reject("LD_DW instruction cannot be last in program".to_string())?; + return Err(VerifierError::LDDWCannotBeLast.into()); } let next_insn = ebpf::get_insn(prog, insn_ptr + 1); if next_insn.opc != 0 { - reject(format!( - "incomplete LD_DW instruction (insn #{:?})", - insn_ptr - ))?; + return Err(VerifierError::IncompleteLDDW(insn_ptr).into()); } - Ok(()) } -fn check_jmp_offset(prog: &[u8], insn_ptr: usize) -> Result<(), Error> { +fn check_jmp_offset(prog: &[u8], insn_ptr: usize) -> Result<(), BPFError> { let insn = ebpf::get_insn(prog, insn_ptr); if insn.off == -1 { - reject(format!("infinite loop (insn #{:?})", insn_ptr))?; + return Err(VerifierError::InfiniteLoop(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) { - reject(format!( - "jump out of code to #{:?} (insn #{:?})", - dst_insn_ptr, insn_ptr - ))?; + return Err(VerifierError::JumpOutOfCode(dst_insn_ptr as usize, insn_ptr).into()); } - let dst_insn = ebpf::get_insn(prog, dst_insn_ptr as usize); if dst_insn.opc == 0 { - reject(format!( - "jump to middle of LD_DW at #{:?} (insn #{:?})", - dst_insn_ptr, insn_ptr - ))?; + return Err(VerifierError::JumpToMiddleOfLDDW(dst_insn_ptr as usize, insn_ptr).into()); } - Ok(()) } -fn check_registers(insn: &ebpf::Insn, store: bool, insn_ptr: usize) -> Result<(), Error> { +fn check_registers(insn: &ebpf::Insn, store: bool, insn_ptr: usize) -> Result<(), BPFError> { if insn.src > 10 { - reject(format!("invalid source register (insn #{:?})", insn_ptr))?; + return Err(VerifierError::InvalidSourceRegister(insn_ptr).into()); } - match (insn.dst, store) { (0..=9, _) | (10, true) => Ok(()), - (10, false) => reject(format!( - "cannot write into register r10 (insn #{:?})", - insn_ptr - )), - (_, _) => reject(format!( - "invalid destination register (insn #{:?})", - insn_ptr - )), + (10, false) => Err(VerifierError::CannotWriteR10(insn_ptr).into()), + (_, _) => Err(VerifierError::InvalidDestinationRegister(insn_ptr).into()), } } -pub fn check(prog: &[u8]) -> Result<(), Error> { +pub fn check(prog: &[u8]) -> Result<(), BPFError> { check_prog_len(prog)?; let mut insn_ptr: usize = 0; @@ -297,10 +294,7 @@ pub fn check(prog: &[u8]) -> Result<(), Error> { ebpf::EXIT => {} _ => { - reject(format!( - "unknown eBPF opcode {:#2x} (insn #{:?})", - insn.opc, insn_ptr - ))?; + return Err(VerifierError::UnknownOpCode(insn.opc, insn_ptr).into()); } } @@ -311,7 +305,7 @@ pub fn check(prog: &[u8]) -> Result<(), Error> { // insn_ptr should now be equal to number of instructions. if insn_ptr != prog.len() / ebpf::INSN_SIZE { - reject(format!("jumped out of code to #{:?}", insn_ptr))?; + return Err(VerifierError::JumpOutOfCode(insn_ptr, insn_ptr).into()); } Ok(()) diff --git a/programs/bpf_loader/src/helpers.rs b/programs/bpf_loader/src/helpers.rs index 825db01787..71a1a6dfc2 100644 --- a/programs/bpf_loader/src/helpers.rs +++ b/programs/bpf_loader/src/helpers.rs @@ -1,18 +1,37 @@ -use crate::alloc; +use crate::{alloc, BPFError}; use alloc::Alloc; use log::*; use solana_rbpf::{ - ebpf::{HelperObject, ELF_INSN_DUMP_OFFSET, MM_HEAP_START}, + ebpf::{EbpfError, HelperObject, ELF_INSN_DUMP_OFFSET, MM_HEAP_START}, memory_region::{translate_addr, MemoryRegion}, EbpfVm, }; +use solana_sdk::instruction::InstructionError; use std::{ alloc::Layout, - io::{Error, ErrorKind}, mem::{align_of, size_of}, slice::from_raw_parts_mut, - str::from_utf8, + str::{from_utf8, Utf8Error}, }; +use thiserror::Error as ThisError; + +/// Error definitions +#[derive(Debug, ThisError)] +pub enum HelperError { + #[error("{0}: {1:?}")] + InvalidString(Utf8Error, Vec), + #[error("BPF program called abort()!")] + Abort, + #[error("BPF program Panicked at {0}, {1}:{2}")] + Panic(String, u64, u64), + #[error("{0}")] + InstructionError(InstructionError), +} +impl From for EbpfError { + fn from(error: HelperError) -> Self { + EbpfError::UserError(error.into()) + } +} /// Program heap allocators are intended to allocate/free from a given /// chunk of memory. The specific allocator implementation is @@ -26,7 +45,9 @@ use crate::allocator_bump::BPFAllocator; /// are expected to enforce this const DEFAULT_HEAP_SIZE: usize = 32 * 1024; -pub fn register_helpers(vm: &mut EbpfVm) -> Result { +pub fn register_helpers<'a>( + vm: &mut EbpfVm<'a, BPFError>, +) -> Result> { vm.register_helper_ex("abort", helper_abort)?; vm.register_helper_ex("sol_panic", helper_sol_panic)?; vm.register_helper_ex("sol_panic_", helper_sol_panic)?; @@ -74,7 +95,6 @@ macro_rules! translate_type_mut { } }; } - #[macro_export] macro_rules! translate_type { ($t:ty, $vm_addr:expr, $regions:expr) => { @@ -95,7 +115,6 @@ macro_rules! translate_slice_mut { unsafe { from_raw_parts_mut(host_addr, $len as usize) } }}; } - #[macro_export] macro_rules! translate_slice { ($t:ty, $vm_addr:expr, $len: expr, $regions:expr) => { @@ -109,8 +128,8 @@ fn translate_string_and_do( addr: u64, len: u64, regions: &[MemoryRegion], - work: &dyn Fn(&str) -> Result, -) -> Result { + work: &dyn Fn(&str) -> Result>, +) -> Result> { let buf = translate_slice!(u8, addr, len, regions); let i = match buf.iter().position(|byte| *byte == 0) { Some(i) => i, @@ -118,10 +137,7 @@ fn translate_string_and_do( }; match from_utf8(&buf[..i]) { Ok(message) => work(message), - Err(err) => Err(Error::new( - ErrorKind::Other, - format!("Error: Invalid string {:?}", err), - )), + Err(err) => Err(HelperError::InvalidString(err, buf[..i].to_vec()).into()), } } @@ -136,11 +152,8 @@ pub fn helper_abort( _arg5: u64, _ro_regions: &[MemoryRegion], _rw_regions: &[MemoryRegion], -) -> Result { - Err(Error::new( - ErrorKind::Other, - "Error: BPF program called abort()!", - )) +) -> Result> { + Err(HelperError::Abort.into()) } /// Panic helper functions, called when the BPF program calls 'sol_panic_()` @@ -154,15 +167,9 @@ pub fn helper_sol_panic( _arg5: u64, ro_regions: &[MemoryRegion], _rw_regions: &[MemoryRegion], -) -> Result { +) -> Result> { translate_string_and_do(file, len, ro_regions, &|string: &str| { - Err(Error::new( - ErrorKind::Other, - format!( - "Error: BPF program Panicked at {}, {}:{}", - string, line, column - ), - )) + Err(HelperError::Panic(string.to_string(), line, column).into()) }) } @@ -175,7 +182,7 @@ pub fn helper_sol_log( _arg5: u64, ro_regions: &[MemoryRegion], _rw_regions: &[MemoryRegion], -) -> Result { +) -> Result> { if log_enabled!(log::Level::Info) { translate_string_and_do(addr, len, ro_regions, &|string: &str| { info!("info!: {}", string); @@ -194,7 +201,7 @@ pub fn helper_sol_log_u64( arg5: u64, _ro_regions: &[MemoryRegion], _rw_regions: &[MemoryRegion], -) -> Result { +) -> Result> { if log_enabled!(log::Level::Info) { info!( "info!: {:#x}, {:#x}, {:#x}, {:#x}, {:#x}", @@ -213,7 +220,7 @@ pub fn helper_sol_log_u64( pub struct HelperSolAllocFree { allocator: BPFAllocator, } -impl HelperObject for HelperSolAllocFree { +impl HelperObject for HelperSolAllocFree { fn call( &mut self, size: u64, @@ -223,7 +230,7 @@ impl HelperObject for HelperSolAllocFree { _arg5: u64, _ro_regions: &[MemoryRegion], _rw_regions: &[MemoryRegion], - ) -> Result { + ) -> Result> { let layout = Layout::from_size_align(size as usize, align_of::()).unwrap(); if free_addr == 0 { match self.allocator.alloc(layout) { diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 4a01ed9e7e..48bc614fe8 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -3,10 +3,15 @@ pub mod allocator_bump; pub mod bpf_verifier; pub mod helpers; +use crate::{bpf_verifier::VerifierError, helpers::HelperError}; use byteorder::{ByteOrder, LittleEndian, WriteBytesExt}; use log::*; use num_derive::{FromPrimitive, ToPrimitive}; -use solana_rbpf::{memory_region::MemoryRegion, EbpfVm}; +use solana_rbpf::{ + ebpf::{EbpfError, UserDefinedError}, + memory_region::MemoryRegion, + EbpfVm, +}; use solana_sdk::{ account::KeyedAccount, bpf_loader, @@ -18,10 +23,7 @@ use solana_sdk::{ pubkey::Pubkey, sysvar::rent, }; -use std::{ - io::{prelude::*, Error}, - mem, -}; +use std::{io::prelude::*, mem}; use thiserror::Error; solana_sdk::declare_program!( @@ -32,19 +34,28 @@ solana_sdk::declare_program!( #[derive(Error, Debug, Clone, PartialEq, FromPrimitive, ToPrimitive)] pub enum BPFLoaderError { - #[error("Failed to create virtual machine")] - VirtualMachineCreationFailed, - #[error("Virtual machine failed to run the program to completion")] - VirtualMachineFailedToRunProgram, + #[error("failed to create virtual machine")] + VirtualMachineCreationFailed = 0x0b9f_0001, + #[error("virtual machine failed to run the program to completion")] + VirtualMachineFailedToRunProgram = 0x0b9f_0002, } - impl DecodeError for BPFLoaderError { fn type_of() -> &'static str { "BPFLoaderError" } } -pub fn create_vm(prog: &[u8]) -> Result<(EbpfVm, MemoryRegion), Error> { +/// Errors returned by functions the BPF Loader registers with the vM +#[derive(Debug, Error)] +pub enum BPFError { + #[error("{0}")] + VerifierError(#[from] VerifierError), + #[error("{0}")] + HelperError(#[from] HelperError), +} +impl UserDefinedError for BPFError {} + +pub fn create_vm(prog: &[u8]) -> Result<(EbpfVm, MemoryRegion), EbpfError> { let mut vm = EbpfVm::new(None)?; vm.set_verifier(bpf_verifier::check)?; vm.set_max_instruction_count(100_000)?; @@ -55,7 +66,7 @@ pub fn create_vm(prog: &[u8]) -> Result<(EbpfVm, MemoryRegion), Error> { Ok((vm, heap_region)) } -pub fn check_elf(prog: &[u8]) -> Result<(), Error> { +pub fn check_elf(prog: &[u8]) -> Result<(), EbpfError> { let mut vm = EbpfVm::new(None)?; vm.set_verifier(bpf_verifier::check)?; vm.set_elf(&prog)?; @@ -180,13 +191,18 @@ pub fn process_instruction( Ok(status) => { if status != SUCCESS { let error: InstructionError = status.into(); - warn!("BPF program failed: {:?}", error); + warn!("BPF program failed: {}", error); return Err(error); } } - Err(e) => { - warn!("BPF VM failed to run program: {}", e); - return Err(BPFLoaderError::VirtualMachineFailedToRunProgram.into()); + Err(error) => { + warn!("BPF program failed: {}", error); + return match error { + EbpfError::UserError(BPFError::HelperError( + HelperError::InstructionError(error), + )) => Err(error), + _ => Err(BPFLoaderError::VirtualMachineFailedToRunProgram.into()), + }; } } } @@ -221,7 +237,7 @@ pub fn process_instruction( } if let Err(e) = check_elf(&program.try_account_ref()?.data) { - warn!("Invalid ELF: {}", e); + warn!("{}", e); return Err(InstructionError::InvalidAccountData); } @@ -242,7 +258,7 @@ mod tests { use std::{cell::RefCell, fs::File, io::Read}; #[test] - #[should_panic(expected = "Error: Exceeded maximum number of instructions allowed")] + #[should_panic(expected = "ExceededMaxInstructions(10)")] fn test_bpf_loader_non_terminating_program() { #[rustfmt::skip] let program = &[ @@ -252,7 +268,7 @@ mod tests { ]; let input = &mut [0x00]; - let mut vm = EbpfVm::new(None).unwrap(); + let mut vm = EbpfVm::::new(None).unwrap(); vm.set_verifier(bpf_verifier::check).unwrap(); vm.set_max_instruction_count(10).unwrap(); vm.set_program(program).unwrap();