diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 548fd48887..756b808e13 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -206,6 +206,13 @@ impl<'a> StackFrame<'a> { } } +struct SyscallContext { + check_aligned: bool, + check_size: bool, + orig_account_lengths: Vec, + allocator: Rc>, +} + pub struct InvokeContext<'a> { pub transaction_context: &'a mut TransactionContext, #[allow(deprecated)] @@ -224,10 +231,7 @@ pub struct InvokeContext<'a> { pub timings: ExecuteDetailsTimings, pub blockhash: Hash, pub lamports_per_signature: u64, - check_aligned: bool, - check_size: bool, - orig_account_lengths: Vec>>, - allocators: Vec>>>, + syscall_context: Vec>, } impl<'a> InvokeContext<'a> { @@ -262,10 +266,7 @@ impl<'a> InvokeContext<'a> { timings: ExecuteDetailsTimings::default(), blockhash, lamports_per_signature, - check_aligned: true, - check_size: true, - orig_account_lengths: Vec::new(), - allocators: Vec::new(), + syscall_context: Vec::new(), } } @@ -454,8 +455,7 @@ impl<'a> InvokeContext<'a> { std::mem::transmute(keyed_accounts.as_slice()) }), )); - self.orig_account_lengths.push(None); - self.allocators.push(None); + self.syscall_context.push(None); self.transaction_context.push( program_indices, instruction_accounts, @@ -467,8 +467,7 @@ impl<'a> InvokeContext<'a> { /// Pop a stack frame from the invocation stack pub fn pop(&mut self) -> Result<(), InstructionError> { - self.orig_account_lengths.pop(); - self.allocators.pop(); + self.syscall_context.pop(); self.invoke_stack.pop(); self.transaction_context.pop() } @@ -1077,66 +1076,61 @@ impl<'a> InvokeContext<'a> { .get_key_of_account_at_index(index_in_transaction) } - /// Set the original account lengths - pub fn set_orig_account_lengths( + // Set this instruction syscall context + pub fn set_syscall_context( &mut self, + check_aligned: bool, + check_size: bool, orig_account_lengths: Vec, + allocator: Rc>, ) -> Result<(), InstructionError> { *self - .orig_account_lengths + .syscall_context .last_mut() - .ok_or(InstructionError::CallDepth)? = Some(orig_account_lengths); + .ok_or(InstructionError::CallDepth)? = Some(SyscallContext { + check_aligned, + check_size, + orig_account_lengths, + allocator, + }); Ok(()) } - /// Get the original account lengths - pub fn get_orig_account_lengths(&self) -> Result<&[usize], InstructionError> { - self.orig_account_lengths - .last() - .and_then(|orig_account_lengths| orig_account_lengths.as_ref()) - .map(|orig_account_lengths| orig_account_lengths.as_slice()) - .ok_or(InstructionError::CallDepth) - } - - // Set should alignment be enforced during user pointer translation - pub fn set_check_aligned(&mut self, check_aligned: bool) { - self.check_aligned = check_aligned; - } - // Should alignment be enforced during user pointer translation pub fn get_check_aligned(&self) -> bool { - self.check_aligned - } - - // Set should type size be checked during user pointer translation - pub fn set_check_size(&mut self, check_size: bool) { - self.check_size = check_size; + self.syscall_context + .last() + .and_then(|context| context.as_ref()) + .map(|context| context.check_aligned) + .unwrap_or(true) } // Set should type size be checked during user pointer translation pub fn get_check_size(&self) -> bool { - self.check_size + self.syscall_context + .last() + .and_then(|context| context.as_ref()) + .map(|context| context.check_size) + .unwrap_or(true) + } + + /// Get the original account lengths + pub fn get_orig_account_lengths(&self) -> Result<&[usize], InstructionError> { + self.syscall_context + .last() + .and_then(|context| context.as_ref()) + .map(|context| context.orig_account_lengths.as_slice()) + .ok_or(InstructionError::CallDepth) } // Get this instruction's memory allocator pub fn get_allocator(&self) -> Result>, InstructionError> { - self.allocators + self.syscall_context .last() - .and_then(|allocator| allocator.clone()) + .and_then(|context| context.as_ref()) + .map(|context| context.allocator.clone()) .ok_or(InstructionError::CallDepth) } - - // Set this instruction's memory allocator - pub fn set_allocator( - &mut self, - allocator: Rc>, - ) -> Result<(), InstructionError> { - *self - .allocators - .last_mut() - .ok_or(InstructionError::CallDepth)? = Some(allocator); - Ok(()) - } } pub struct MockInvokeContextPreparation { diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index b799c42fd9..41efbef47d 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -3983,6 +3983,13 @@ dependencies = [ "solana-program 1.11.0", ] +[[package]] +name = "solana-bpf-rust-inner_instruction_alignment_check" +version = "1.11.0" +dependencies = [ + "solana-program 1.11.0", +] + [[package]] name = "solana-bpf-rust-instruction-introspection" version = "1.11.0" diff --git a/programs/bpf/Cargo.toml b/programs/bpf/Cargo.toml index c462fc41e1..bb9119c934 100644 --- a/programs/bpf/Cargo.toml +++ b/programs/bpf/Cargo.toml @@ -59,6 +59,7 @@ members = [ "rust/external_spend", "rust/get_minimum_delegation", "rust/finalize", + "rust/inner_instruction_alignment_check", "rust/instruction_introspection", "rust/invoke", "rust/invoke_and_error", diff --git a/programs/bpf/benches/bpf_loader.rs b/programs/bpf/benches/bpf_loader.rs index cc07075f63..cb20544a81 100644 --- a/programs/bpf/benches/bpf_loader.rs +++ b/programs/bpf/benches/bpf_loader.rs @@ -115,8 +115,7 @@ fn bench_program_alu(bencher: &mut Bencher) { Executable::::jit_compile(&mut executable).unwrap(); let compute_meter = invoke_context.get_compute_meter(); let mut instruction_meter = ThisInstructionMeter { compute_meter }; - invoke_context.set_orig_account_lengths(vec![]).unwrap(); - let mut vm = create_vm(&executable, &mut inner_iter, invoke_context).unwrap(); + let mut vm = create_vm(&executable, &mut inner_iter, vec![], invoke_context).unwrap(); println!("Interpreted:"); assert_eq!( @@ -220,9 +219,6 @@ fn bench_create_vm(bencher: &mut Bencher) { .unwrap(), ) .unwrap(); - invoke_context - .set_orig_account_lengths(account_lengths) - .unwrap(); let executable = Executable::::from_elf( &elf, @@ -233,7 +229,13 @@ fn bench_create_vm(bencher: &mut Bencher) { .unwrap(); bencher.iter(|| { - let _ = create_vm(&executable, serialized.as_slice_mut(), invoke_context).unwrap(); + let _ = create_vm( + &executable, + serialized.as_slice_mut(), + account_lengths.clone(), + invoke_context, + ) + .unwrap(); }); }); } @@ -268,10 +270,13 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) { .unwrap(); let compute_meter = invoke_context.get_compute_meter(); let mut instruction_meter = ThisInstructionMeter { compute_meter }; - invoke_context - .set_orig_account_lengths(account_lengths) - .unwrap(); - let mut vm = create_vm(&executable, serialized.as_slice_mut(), invoke_context).unwrap(); + let mut vm = create_vm( + &executable, + serialized.as_slice_mut(), + account_lengths, + invoke_context, + ) + .unwrap(); let mut measure = Measure::start("tune"); let _ = vm.execute_program_interpreted(&mut instruction_meter); diff --git a/programs/bpf/build.rs b/programs/bpf/build.rs index c9aebb6d56..f237b656c2 100644 --- a/programs/bpf/build.rs +++ b/programs/bpf/build.rs @@ -71,6 +71,7 @@ fn main() { "external_spend", "finalize", "get_minimum_delegation", + "inner_instruction_alignment_check", "instruction_introspection", "invoke", "invoke_and_error", diff --git a/programs/bpf/rust/inner_instruction_alignment_check/Cargo.toml b/programs/bpf/rust/inner_instruction_alignment_check/Cargo.toml new file mode 100644 index 0000000000..1faf9bc2dc --- /dev/null +++ b/programs/bpf/rust/inner_instruction_alignment_check/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "solana-bpf-rust-inner_instruction_alignment_check" +version = "1.11.0" +description = "Solana BPF test program written in Rust" +authors = ["Solana Maintainers "] +repository = "https://github.com/solana-labs/solana" +license = "Apache-2.0" +homepage = "https://solana.com/" +documentation = "https://docs.rs/solana-bpf-rust-inner_instruction_alignment_check" +edition = "2021" + +[dependencies] +solana-program = { path = "../../../../sdk/program", version = "=1.11.0" } + +[lib] +crate-type = ["cdylib"] + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] diff --git a/programs/bpf/rust/inner_instruction_alignment_check/src/lib.rs b/programs/bpf/rust/inner_instruction_alignment_check/src/lib.rs new file mode 100644 index 0000000000..109e52db9a --- /dev/null +++ b/programs/bpf/rust/inner_instruction_alignment_check/src/lib.rs @@ -0,0 +1,41 @@ +//! Example Rust-based BPF noop program + +use solana_program::{ + account_info::AccountInfo, + entrypoint_deprecated::ProgramResult, + instruction::{AccountMeta, Instruction}, + msg, + program::invoke, + pubkey::Pubkey, +}; + +#[no_mangle] +fn custom_panic(info: &core::panic::PanicInfo<'_>) { + // Full panic reporting + msg!(&format!("{}", info)); +} + +solana_program::entrypoint_deprecated!(process_instruction); +#[allow(clippy::unnecessary_wraps)] +fn process_instruction( + _program_id: &Pubkey, + accounts: &[AccountInfo], + instruction_data: &[u8], +) -> ProgramResult { + let to_call = accounts[0].key; + let infos = accounts; + let instruction = Instruction { + accounts: vec![AccountMeta { + pubkey: *accounts[1].key, + is_signer: accounts[1].is_signer, + is_writable: accounts[1].is_writable, + }], + data: instruction_data.to_owned(), + program_id: *to_call, + }; + + let _ = invoke(&instruction, infos); + let _ = invoke(&instruction, infos); + + Ok(()) +} diff --git a/programs/bpf/rust/invoke_and_ok/src/lib.rs b/programs/bpf/rust/invoke_and_ok/src/lib.rs index 790b89c734..c23b796ccf 100644 --- a/programs/bpf/rust/invoke_and_ok/src/lib.rs +++ b/programs/bpf/rust/invoke_and_ok/src/lib.rs @@ -1,4 +1,4 @@ -//! Invokes an instruction and returns an error, the instruction invoked +//! Invokes an instruction and return ok no matter what, the instruction invoked //! uses the instruction data provided and all the accounts use solana_program::{ diff --git a/programs/bpf/rust/invoke_and_return/src/lib.rs b/programs/bpf/rust/invoke_and_return/src/lib.rs index c5033ab1db..257dd4cc2f 100644 --- a/programs/bpf/rust/invoke_and_return/src/lib.rs +++ b/programs/bpf/rust/invoke_and_return/src/lib.rs @@ -1,4 +1,4 @@ -//! Invokes an instruction and returns an error, the instruction invoked +//! Invokes an instruction and returns the invoke result, the instruction invoked //! uses the instruction data provided and all the accounts use solana_program::{ diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 841062e7fd..4793025f10 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -246,11 +246,13 @@ fn run_program(name: &str) -> u64 { .unwrap(); let mut parameter_bytes = parameter_bytes.clone(); { - invoke_context - .set_orig_account_lengths(account_lengths.clone()) - .unwrap(); - let mut vm = - create_vm(&executable, parameter_bytes.as_slice_mut(), invoke_context).unwrap(); + let mut vm = create_vm( + &executable, + parameter_bytes.as_slice_mut(), + account_lengths.clone(), + invoke_context, + ) + .unwrap(); let result = if i == 0 { vm.execute_program_interpreted(&mut instruction_meter) } else { @@ -3555,3 +3557,56 @@ fn test_get_minimum_delegation() { let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); assert!(result.is_ok()); } + +#[cfg(feature = "bpf_rust")] +#[test] +fn test_program_bpf_inner_instruction_alignment_checks() { + solana_logger::setup(); + + let GenesisConfigInfo { + mut genesis_config, + mint_keypair, + .. + } = create_genesis_config(50); + genesis_config + .accounts + .remove(&solana_sdk::feature_set::disable_deprecated_loader::id()) + .unwrap(); + let mut bank = Bank::new_for_tests(&genesis_config); + let (name, id, entrypoint) = solana_bpf_loader_program!(); + bank.add_builtin(&name, &id, entrypoint); + let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!(); + bank.add_builtin(&name, &id, entrypoint); + let bank_client = BankClient::new(bank); + + // load aligned program + let noop = load_bpf_program( + &bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_bpf_rust_noop", + ); + + // Load unaligned program + let inner_instruction_alignment_check = load_bpf_program( + &bank_client, + &bpf_loader_deprecated::id(), + &mint_keypair, + "solana_bpf_rust_inner_instruction_alignment_check", + ); + + // invoke unaligned program, which will call aligned program twice, + // unaligned should be allowed once invoke completes + let mut instruction = Instruction::new_with_bytes( + inner_instruction_alignment_check, + &[0], + vec![ + AccountMeta::new_readonly(noop, false), + AccountMeta::new_readonly(mint_keypair.pubkey(), false), + ], + ); + + instruction.data[0] += 1; + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction.clone()); + assert!(result.is_ok()); +} diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 8368cb6bb3..7555fc6236 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -249,6 +249,7 @@ fn check_loader_id(id: &Pubkey) -> bool { pub fn create_vm<'a, 'b>( program: &'a Pin>>, parameter_bytes: &mut [u8], + orig_account_lengths: Vec, invoke_context: &'a mut InvokeContext<'b>, ) -> Result, EbpfError> { let compute_budget = invoke_context.get_compute_budget(); @@ -267,7 +268,7 @@ pub fn create_vm<'a, 'b>( AlignedMemory::new_with_size(compute_budget.heap_size.unwrap_or(HEAP_LENGTH), HOST_ALIGN); let parameter_region = MemoryRegion::new_writable(parameter_bytes, MM_INPUT_START); let mut vm = EbpfVm::new(program, heap.as_slice_mut(), vec![parameter_region])?; - syscalls::bind_syscall_context_objects(&mut vm, invoke_context, heap)?; + syscalls::bind_syscall_context_objects(&mut vm, invoke_context, heap, orig_account_lengths)?; Ok(vm) } @@ -1165,13 +1166,14 @@ impl Executor for BpfExecutor { let (mut parameter_bytes, account_lengths) = serialize_parameters(invoke_context.transaction_context, instruction_context)?; serialize_time.stop(); - invoke_context.set_orig_account_lengths(account_lengths)?; + let mut create_vm_time = Measure::start("create_vm"); let mut execute_time; let execution_result = { let mut vm = match create_vm( &self.executable, parameter_bytes.as_slice_mut(), + account_lengths, invoke_context, ) { Ok(info) => info, diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 1432c32a2e..c2bf69c686 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -384,30 +384,28 @@ pub fn bind_syscall_context_objects<'a, 'b>( vm: &mut EbpfVm<'a, BpfError, crate::ThisInstructionMeter>, invoke_context: &'a mut InvokeContext<'b>, heap: AlignedMemory, + orig_account_lengths: Vec, ) -> Result<(), EbpfError> { - invoke_context.set_check_aligned( - bpf_loader_deprecated::id() - != invoke_context - .transaction_context - .get_current_instruction_context() - .and_then(|instruction_context| { - instruction_context - .try_borrow_program_account(invoke_context.transaction_context) - }) - .map(|program_account| *program_account.get_owner()) - .map_err(SyscallError::InstructionError)?, - ); - invoke_context.set_check_size( - invoke_context - .feature_set - .is_active(&check_slice_translation_size::id()), - ); + let check_aligned = bpf_loader_deprecated::id() + != invoke_context + .transaction_context + .get_current_instruction_context() + .and_then(|instruction_context| { + instruction_context.try_borrow_program_account(invoke_context.transaction_context) + }) + .map(|program_account| *program_account.get_owner()) + .map_err(SyscallError::InstructionError)?; + let check_size = invoke_context + .feature_set + .is_active(&check_slice_translation_size::id()); invoke_context - .set_allocator(Rc::new(RefCell::new(BpfAllocator::new( - heap, - ebpf::MM_HEAP_START, - )))) + .set_syscall_context( + check_aligned, + check_size, + orig_account_lengths, + Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))), + ) .map_err(SyscallError::InstructionError)?; let invoke_context = Rc::new(RefCell::new(invoke_context)); @@ -3253,7 +3251,7 @@ fn call<'a, 'b: 'a>( memory_mapping, caller_account.vm_data_addr, new_len as u64, - invoke_context.get_check_aligned(), + false, // Don't care since it is byte aligned invoke_context.get_check_size(), )?; *caller_account.ref_to_len_in_vm = new_len as u64; @@ -4330,10 +4328,12 @@ mod tests { ) .unwrap(); invoke_context - .set_allocator(Rc::new(RefCell::new(BpfAllocator::new( - heap, - ebpf::MM_HEAP_START, - )))) + .set_syscall_context( + true, + true, + vec![], + Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))), + ) .unwrap(); let mut syscall = SyscallAllocFree { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), @@ -4370,12 +4370,13 @@ mod tests { ) .unwrap(); invoke_context - .set_allocator(Rc::new(RefCell::new(BpfAllocator::new( - heap, - ebpf::MM_HEAP_START, - )))) + .set_syscall_context( + false, + true, + vec![], + Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))), + ) .unwrap(); - invoke_context.set_check_aligned(false); let mut syscall = SyscallAllocFree { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), }; @@ -4410,10 +4411,12 @@ mod tests { ) .unwrap(); invoke_context - .set_allocator(Rc::new(RefCell::new(BpfAllocator::new( - heap, - ebpf::MM_HEAP_START, - )))) + .set_syscall_context( + true, + true, + vec![], + Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))), + ) .unwrap(); let mut syscall = SyscallAllocFree { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), @@ -4451,10 +4454,12 @@ mod tests { ) .unwrap(); invoke_context - .set_allocator(Rc::new(RefCell::new(BpfAllocator::new( - heap, - ebpf::MM_HEAP_START, - )))) + .set_syscall_context( + true, + true, + vec![], + Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))), + ) .unwrap(); let mut syscall = SyscallAllocFree { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), diff --git a/rbpf-cli/src/main.rs b/rbpf-cli/src/main.rs index ff8f965372..ebbbd28899 100644 --- a/rbpf-cli/src/main.rs +++ b/rbpf-cli/src/main.rs @@ -313,12 +313,10 @@ native machine code before execting it in the virtual machine.", _ => {} } - invoke_context - .set_orig_account_lengths(account_lengths) - .unwrap(); let mut vm = create_vm( &executable, parameter_bytes.as_slice_mut(), + account_lengths, &mut invoke_context, ) .unwrap();