diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 87369ab9a663c7..f93088f0d80f65 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -123,7 +123,8 @@ pub fn builtin_process_instruction( invoke_context .transaction_context .get_current_instruction_context()?, - true, + true, // should_cap_ix_accounts + true, // copy_account_data // There is no VM so direct mapping can not be implemented here )?; // Deserialize data back into instruction params diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index e14f55476ca542..87d1fc4f8a8c21 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -126,7 +126,20 @@ fn bench_serialize_unaligned(bencher: &mut Bencher) { .get_current_instruction_context() .unwrap(); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap(); + let _ = + serialize_parameters(&transaction_context, instruction_context, true, false).unwrap(); + }); +} + +#[bench] +fn bench_serialize_unaligned_copy_account_data(bencher: &mut Bencher) { + let transaction_context = create_inputs(bpf_loader_deprecated::id(), 7); + let instruction_context = transaction_context + .get_current_instruction_context() + .unwrap(); + bencher.iter(|| { + let _ = + serialize_parameters(&transaction_context, instruction_context, true, true).unwrap(); }); } @@ -138,7 +151,21 @@ fn bench_serialize_aligned(bencher: &mut Bencher) { .unwrap(); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap(); + let _ = + serialize_parameters(&transaction_context, instruction_context, true, false).unwrap(); + }); +} + +#[bench] +fn bench_serialize_aligned_copy_account_data(bencher: &mut Bencher) { + let transaction_context = create_inputs(bpf_loader::id(), 7); + let instruction_context = transaction_context + .get_current_instruction_context() + .unwrap(); + + bencher.iter(|| { + let _ = + serialize_parameters(&transaction_context, instruction_context, true, true).unwrap(); }); } @@ -149,7 +176,8 @@ fn bench_serialize_unaligned_max_accounts(bencher: &mut Bencher) { .get_current_instruction_context() .unwrap(); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap(); + let _ = + serialize_parameters(&transaction_context, instruction_context, true, false).unwrap(); }); } @@ -161,6 +189,7 @@ fn bench_serialize_aligned_max_accounts(bencher: &mut Bencher) { .unwrap(); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap(); + let _ = + serialize_parameters(&transaction_context, instruction_context, true, false).unwrap(); }); } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index cf4f841cadf89c..347ef8328e8ea8 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -20,21 +20,24 @@ use { aligned_memory::AlignedMemory, ebpf::{self, HOST_ALIGN, MM_HEAP_START}, elf::Executable, - memory_region::{MemoryCowCallback, MemoryMapping, MemoryRegion}, + error::EbpfError, + memory_region::{AccessType, MemoryCowCallback, MemoryMapping, MemoryRegion}, verifier::RequisiteVerifier, vm::{ContextObject, EbpfVm, ProgramResult, VerifiedExecutable}, }, solana_sdk::{ + account::WritableAccount, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::Slot, - entrypoint::SUCCESS, + entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, feature_set::{ - cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts, - delay_visibility_of_program_deployment, disable_deploy_of_alloc_free_syscall, - enable_bpf_loader_extend_program_ix, enable_bpf_loader_set_authority_checked_ix, - enable_program_redeployment_cooldown, limit_max_instruction_trace_length, - native_programs_consume_cu, remove_bpf_loader_incorrect_program_id, FeatureSet, + bpf_account_data_direct_mapping, cap_accounts_data_allocations_per_transaction, + cap_bpf_program_instruction_accounts, delay_visibility_of_program_deployment, + disable_deploy_of_alloc_free_syscall, enable_bpf_loader_extend_program_ix, + enable_bpf_loader_set_authority_checked_ix, enable_program_redeployment_cooldown, + limit_max_instruction_trace_length, native_programs_consume_cu, + remove_bpf_loader_incorrect_program_id, FeatureSet, }, instruction::{AccountMeta, InstructionError}, loader_instruction::LoaderInstruction, @@ -302,8 +305,31 @@ pub fn create_vm<'a, 'b>( ) -> Result>, Box> { let stack_size = stack.len(); let heap_size = heap.len(); - let memory_mapping = - create_memory_mapping(program.get_executable(), stack, heap, regions, None)?; + let accounts = Arc::clone(invoke_context.transaction_context.accounts()); + let memory_mapping = create_memory_mapping( + program.get_executable(), + stack, + heap, + regions, + Some(Box::new(move |index_in_transaction| { + // The two calls below can't really fail. If they fail because of a bug, + // whatever is writing will trigger an EbpfError::AccessViolation like + // if the region was readonly, and the transaction will fail gracefully. + let mut account = accounts + .try_borrow_mut(index_in_transaction as IndexOfAccount) + .map_err(|_| ())?; + accounts + .touch(index_in_transaction as IndexOfAccount) + .map_err(|_| ())?; + + if account.is_shared() { + // See BorrowedAccount::make_data_mut() as to why we reserve extra + // MAX_PERMITTED_DATA_INCREASE bytes here. + account.reserve(MAX_PERMITTED_DATA_INCREASE); + } + Ok(account.data_as_mut_slice().as_mut_ptr() as u64) + })), + )?; invoke_context.set_syscall_context(SyscallContext { allocator: BpfAllocator::new(heap_size as u64), orig_account_lengths, @@ -1524,6 +1550,9 @@ fn execute<'a, 'b: 'a>( let use_jit = false; #[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))] let use_jit = executable.get_executable().get_compiled_program().is_some(); + let bpf_account_data_direct_mapping = invoke_context + .feature_set + .is_active(&bpf_account_data_direct_mapping::id()); let mut serialize_time = Measure::start("serialize"); let (parameter_bytes, regions, account_lengths) = serialization::serialize_parameters( @@ -1532,9 +1561,19 @@ fn execute<'a, 'b: 'a>( invoke_context .feature_set .is_active(&cap_bpf_program_instruction_accounts::ID), + !bpf_account_data_direct_mapping, )?; serialize_time.stop(); + // save the account addresses so in case of AccessViolation below we can + // map to InstructionError::ReadonlyDataModified, which is easier to + // diagnose from developers + let account_region_addrs = regions + .iter() + .map(|r| r.vm_addr..r.vm_addr.saturating_add(r.len)) + .collect::>(); + let addr_is_account_data = |addr: u64| account_region_addrs.iter().any(|r| r.contains(&addr)); + let mut create_vm_time = Measure::start("create_vm"); let mut execute_time; let execution_result = { @@ -1597,7 +1636,24 @@ fn execute<'a, 'b: 'a>( }; Err(Box::new(error) as Box) } - ProgramResult::Err(error) => Err(error), + ProgramResult::Err(error) => { + let error = match error.downcast_ref() { + Some(EbpfError::AccessViolation( + _pc, + AccessType::Store, + address, + _size, + _section_name, + )) if addr_is_account_data(*address) => { + // We can get here if direct_mapping is enabled and a program tries to + // write to a readonly account. Map the error to ReadonlyDataModified so + // it's easier for devs to diagnose what happened. + Box::new(InstructionError::ReadonlyDataModified) + } + _ => error, + }; + Err(error) + } _ => Ok(()), } }; @@ -1606,12 +1662,14 @@ fn execute<'a, 'b: 'a>( fn deserialize_parameters( invoke_context: &mut InvokeContext, parameter_bytes: &[u8], + copy_account_data: bool, ) -> Result<(), InstructionError> { serialization::deserialize_parameters( invoke_context.transaction_context, invoke_context .transaction_context .get_current_instruction_context()?, + copy_account_data, parameter_bytes, &invoke_context.get_syscall_context()?.orig_account_lengths, ) @@ -1619,8 +1677,12 @@ fn execute<'a, 'b: 'a>( let mut deserialize_time = Measure::start("deserialize"); let execute_or_deserialize_result = execution_result.and_then(|_| { - deserialize_parameters(invoke_context, parameter_bytes.as_slice()) - .map_err(|error| Box::new(error) as Box) + deserialize_parameters( + invoke_context, + parameter_bytes.as_slice(), + !bpf_account_data_direct_mapping, + ) + .map_err(|error| Box::new(error) as Box) }); deserialize_time.stop(); diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs index 6b595e3c0708c2..7a36a5229d95ea 100644 --- a/programs/bpf_loader/src/serialization.rs +++ b/programs/bpf_loader/src/serialization.rs @@ -17,7 +17,7 @@ use { BorrowedAccount, IndexOfAccount, InstructionContext, TransactionContext, }, }, - std::{mem, mem::size_of}, + std::mem::{self, size_of}, }; /// Maximum number of instruction accounts that can be serialized into the @@ -35,16 +35,18 @@ struct Serializer { vaddr: u64, region_start: usize, aligned: bool, + copy_account_data: bool, } impl Serializer { - fn new(size: usize, start_addr: u64, aligned: bool) -> Serializer { + fn new(size: usize, start_addr: u64, aligned: bool, copy_account_data: bool) -> Serializer { Serializer { buffer: AlignedMemory::with_capacity(size), regions: Vec::new(), region_start: 0, vaddr: start_addr, aligned, + copy_account_data, } } @@ -75,19 +77,68 @@ impl Serializer { } } - fn write_account(&mut self, account: &BorrowedAccount<'_>) -> Result<(), InstructionError> { - self.write_all(account.get_data()); + fn write_account(&mut self, account: &mut BorrowedAccount<'_>) -> Result<(), InstructionError> { + if self.copy_account_data { + self.write_all(account.get_data()); + } else { + self.push_account_region(account)?; + } if self.aligned { let align_offset = (account.get_data().len() as *const u8).align_offset(BPF_ALIGN_OF_U128); - self.fill_write(MAX_PERMITTED_DATA_INCREASE + align_offset, 0) - .map_err(|_| InstructionError::InvalidArgument)?; + if self.copy_account_data { + self.fill_write(MAX_PERMITTED_DATA_INCREASE + align_offset, 0) + .map_err(|_| InstructionError::InvalidArgument)?; + } else { + // The deserialization code is going to align the vm_addr to + // BPF_ALIGN_OF_U128. Always add one BPF_ALIGN_OF_U128 worth of + // padding and shift the start of the next region, so that once + // vm_addr is aligned, the corresponding host_addr is aligned + // too. + self.fill_write(MAX_PERMITTED_DATA_INCREASE + BPF_ALIGN_OF_U128, 0) + .map_err(|_| InstructionError::InvalidArgument)?; + self.region_start += BPF_ALIGN_OF_U128.saturating_sub(align_offset); + } } Ok(()) } + fn push_account_region( + &mut self, + account: &mut BorrowedAccount<'_>, + ) -> Result<(), InstructionError> { + self.push_region(); + let account_len = account.get_data().len(); + if account_len > 0 { + let region = if account.can_data_be_changed().is_ok() { + if account.is_shared() { + // If the account is still shared it means it wasn't written to yet during this + // transaction. We map it as CoW and it'll be copied the first time something + // tries to write into it. + let index_in_transaction = account.get_index_in_transaction(); + + MemoryRegion::new_cow( + account.get_data(), + self.vaddr, + index_in_transaction as u64, + ) + } else { + // The account isn't shared anymore, meaning it was written to earlier during + // this transaction. We can just map as writable no need for any fancy CoW + // business. + MemoryRegion::new_writable(account.get_data_mut()?, self.vaddr) + } + } else { + MemoryRegion::new_readonly(account.get_data(), self.vaddr) + }; + self.vaddr += region.len; + self.regions.push(region); + } + Ok(()) + } + fn push_region(&mut self) { let range = self.region_start..self.buffer.len(); let region = MemoryRegion::new_writable( @@ -123,6 +174,7 @@ pub fn serialize_parameters( transaction_context: &TransactionContext, instruction_context: &InstructionContext, should_cap_ix_accounts: bool, + copy_account_data: bool, ) -> Result<(AlignedMemory, Vec, Vec), InstructionError> { let num_ix_accounts = instruction_context.get_number_of_instruction_accounts(); if should_cap_ix_accounts && num_ix_accounts > MAX_INSTRUCTION_ACCOUNTS as IndexOfAccount { @@ -156,9 +208,19 @@ pub fn serialize_parameters( } if is_loader_deprecated { - serialize_parameters_unaligned(transaction_context, instruction_context, accounts) + serialize_parameters_unaligned( + transaction_context, + instruction_context, + accounts, + copy_account_data, + ) } else { - serialize_parameters_aligned(transaction_context, instruction_context, accounts) + serialize_parameters_aligned( + transaction_context, + instruction_context, + accounts, + copy_account_data, + ) } .map(|(buffer, regions)| (buffer, regions, account_lengths)) } @@ -166,6 +228,7 @@ pub fn serialize_parameters( pub fn deserialize_parameters( transaction_context: &TransactionContext, instruction_context: &InstructionContext, + copy_account_data: bool, buffer: &[u8], account_lengths: &[usize], ) -> Result<(), InstructionError> { @@ -177,6 +240,7 @@ pub fn deserialize_parameters( deserialize_parameters_unaligned( transaction_context, instruction_context, + copy_account_data, buffer, account_lengths, ) @@ -184,6 +248,7 @@ pub fn deserialize_parameters( deserialize_parameters_aligned( transaction_context, instruction_context, + copy_account_data, buffer, account_lengths, ) @@ -194,6 +259,7 @@ fn serialize_parameters_unaligned( transaction_context: &TransactionContext, instruction_context: &InstructionContext, accounts: Vec, + copy_account_data: bool, ) -> Result<(AlignedMemory, Vec), InstructionError> { // Calculate size in order to alloc once let mut size = size_of::(); @@ -207,10 +273,12 @@ fn serialize_parameters_unaligned( + size_of::() // key + size_of::() // lamports + size_of::() // data len - + account.get_data().len() // data + size_of::() // owner + size_of::() // executable + size_of::(); // rent_epoch + if copy_account_data { + size += account.get_data().len(); + } } } } @@ -218,21 +286,20 @@ fn serialize_parameters_unaligned( + instruction_context.get_instruction_data().len() // instruction data + size_of::(); // program id - let mut s = Serializer::new(size, MM_INPUT_START, false); + let mut s = Serializer::new(size, MM_INPUT_START, false, copy_account_data); s.write::((accounts.len() as u64).to_le()); for account in accounts { match account { SerializeAccount::Duplicate(position) => s.write(position as u8), - SerializeAccount::Account(_, account) => { + SerializeAccount::Account(_, mut account) => { s.write::(NON_DUP_MARKER); s.write::(account.is_signer() as u8); s.write::(account.is_writable() as u8); s.write_all(account.get_key().as_ref()); s.write::(account.get_lamports().to_le()); s.write::((account.get_data().len() as u64).to_le()); - s.write_account(&account) - .map_err(|_| InstructionError::InvalidArgument)?; + s.write_account(&mut account)?; s.write_all(account.get_owner().as_ref()); s.write::(account.is_executable() as u8); s.write::((account.get_rent_epoch()).to_le()); @@ -254,6 +321,7 @@ fn serialize_parameters_unaligned( pub fn deserialize_parameters_unaligned( transaction_context: &TransactionContext, instruction_context: &InstructionContext, + copy_account_data: bool, buffer: &[u8], account_lengths: &[usize], ) -> Result<(), InstructionError> { @@ -280,20 +348,22 @@ pub fn deserialize_parameters_unaligned( } start += size_of::() // lamports + size_of::(); // data length - let data = buffer - .get(start..start + pre_len) - .ok_or(InstructionError::InvalidArgument)?; - // The redundant check helps to avoid the expensive data comparison if we can - match borrowed_account - .can_data_be_resized(data.len()) - .and_then(|_| borrowed_account.can_data_be_changed()) - { - Ok(()) => borrowed_account.set_data_from_slice(data)?, - Err(err) if borrowed_account.get_data() != data => return Err(err), - _ => {} + if copy_account_data { + let data = buffer + .get(start..start + pre_len) + .ok_or(InstructionError::InvalidArgument)?; + // The redundant check helps to avoid the expensive data comparison if we can + match borrowed_account + .can_data_be_resized(data.len()) + .and_then(|_| borrowed_account.can_data_be_changed()) + { + Ok(()) => borrowed_account.set_data_from_slice(data)?, + Err(err) if borrowed_account.get_data() != data => return Err(err), + _ => {} + } + start += pre_len; // data } - start += pre_len // data - + size_of::() // owner + start += size_of::() // owner + size_of::() // executable + size_of::(); // rent_epoch } @@ -305,6 +375,7 @@ fn serialize_parameters_aligned( transaction_context: &TransactionContext, instruction_context: &InstructionContext, accounts: Vec, + copy_account_data: bool, ) -> Result<(AlignedMemory, Vec), InstructionError> { // Calculate size in order to alloc once let mut size = size_of::(); @@ -322,10 +393,13 @@ fn serialize_parameters_aligned( + size_of::() // owner + size_of::() // lamports + size_of::() // data len - + data_len + MAX_PERMITTED_DATA_INCREASE - + (data_len as *const u8).align_offset(BPF_ALIGN_OF_U128) + size_of::(); // rent epoch + if copy_account_data { + size += data_len + (data_len as *const u8).align_offset(BPF_ALIGN_OF_U128); + } else { + size += BPF_ALIGN_OF_U128; + } } } } @@ -333,13 +407,13 @@ fn serialize_parameters_aligned( + instruction_context.get_instruction_data().len() + size_of::(); // program id; - let mut s = Serializer::new(size, MM_INPUT_START, true); + let mut s = Serializer::new(size, MM_INPUT_START, true, copy_account_data); // Serialize into the buffer s.write::((accounts.len() as u64).to_le()); for account in accounts { match account { - SerializeAccount::Account(_, borrowed_account) => { + SerializeAccount::Account(_, mut borrowed_account) => { s.write::(NON_DUP_MARKER); s.write::(borrowed_account.is_signer() as u8); s.write::(borrowed_account.is_writable() as u8); @@ -349,8 +423,7 @@ fn serialize_parameters_aligned( s.write_all(borrowed_account.get_owner().as_ref()); s.write::(borrowed_account.get_lamports().to_le()); s.write::((borrowed_account.get_data().len() as u64).to_le()); - s.write_account(&borrowed_account) - .map_err(|_| InstructionError::InvalidArgument)?; + s.write_account(&mut borrowed_account)?; s.write::((borrowed_account.get_rent_epoch()).to_le()); } SerializeAccount::Duplicate(position) => { @@ -374,6 +447,7 @@ fn serialize_parameters_aligned( pub fn deserialize_parameters_aligned( transaction_context: &TransactionContext, instruction_context: &InstructionContext, + copy_account_data: bool, buffer: &[u8], account_lengths: &[usize], ) -> Result<(), InstructionError> { @@ -418,21 +492,52 @@ pub fn deserialize_parameters_aligned( { return Err(InstructionError::InvalidRealloc); } - let data_end = start + post_len; - let data = buffer - .get(start..data_end) - .ok_or(InstructionError::InvalidArgument)?; // The redundant check helps to avoid the expensive data comparison if we can - match borrowed_account - .can_data_be_resized(data.len()) - .and_then(|_| borrowed_account.can_data_be_changed()) - { - Ok(()) => borrowed_account.set_data_from_slice(data)?, - Err(err) if borrowed_account.get_data() != data => return Err(err), - _ => {} + let alignment_offset = (*pre_len as *const u8).align_offset(BPF_ALIGN_OF_U128); + if copy_account_data { + let data = buffer + .get(start..start + post_len) + .ok_or(InstructionError::InvalidArgument)?; + match borrowed_account + .can_data_be_resized(post_len) + .and_then(|_| borrowed_account.can_data_be_changed()) + { + Ok(()) => borrowed_account.set_data_from_slice(data)?, + Err(err) if borrowed_account.get_data() != data => return Err(err), + _ => {} + } + start += *pre_len; // data + } else { + // See Serializer::write_account() as to why we have this + // padding before the realloc region here. + start += BPF_ALIGN_OF_U128.saturating_sub(alignment_offset); + let data = buffer + .get(start..start + MAX_PERMITTED_DATA_INCREASE) + .ok_or(InstructionError::InvalidArgument)?; + match borrowed_account + .can_data_be_resized(post_len) + .and_then(|_| borrowed_account.can_data_be_changed()) + { + Ok(()) => { + borrowed_account.set_data_length(post_len)?; + let allocated_bytes = post_len.saturating_sub(*pre_len); + if allocated_bytes > 0 { + borrowed_account + .get_data_mut()? + .get_mut(*pre_len..pre_len.saturating_add(allocated_bytes)) + .ok_or(InstructionError::InvalidArgument)? + .copy_from_slice( + data.get(0..allocated_bytes) + .ok_or(InstructionError::InvalidArgument)?, + ); + } + } + Err(err) if borrowed_account.get_data().len() != post_len => return Err(err), + _ => {} + } } - start += *pre_len + MAX_PERMITTED_DATA_INCREASE; // data - start += (start as *const u8).align_offset(BPF_ALIGN_OF_U128); + start += MAX_PERMITTED_DATA_INCREASE; + start += alignment_offset; start += size_of::(); // rent_epoch if borrowed_account.get_owner().to_bytes() != owner { // Change the owner at the end so that we are allowed to change the lamports and data before @@ -444,6 +549,7 @@ pub fn deserialize_parameters_aligned( } #[cfg(test)] +#[allow(clippy::indexing_slicing)] mod tests { use { super::*, @@ -453,13 +559,12 @@ mod tests { account_info::AccountInfo, bpf_loader, entrypoint::deserialize, - sysvar::rent::Rent, transaction_context::InstructionAccount, }, std::{ cell::RefCell, rc::Rc, - slice::{from_raw_parts, from_raw_parts_mut}, + slice::{self, from_raw_parts, from_raw_parts_mut}, }, }; @@ -473,369 +578,418 @@ mod tests { name: &'static str, } - for TestCase { - num_ix_accounts, - append_dup_account, - should_cap_ix_accounts, - expected_err, - name, - } in [ - TestCase { - name: "serialize max accounts without cap", - num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), - should_cap_ix_accounts: false, - append_dup_account: false, - expected_err: None, - }, - TestCase { - name: "serialize max accounts and append dup without cap", - num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), - should_cap_ix_accounts: false, - append_dup_account: true, - expected_err: None, - }, - TestCase { - name: "serialize max accounts with cap", - num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), - should_cap_ix_accounts: true, - append_dup_account: false, - expected_err: None, - }, - TestCase { - name: "serialize too many accounts with cap", - num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS) + 1, - should_cap_ix_accounts: true, - append_dup_account: false, - expected_err: Some(InstructionError::MaxAccountsExceeded), - }, - TestCase { - name: "serialize too many accounts and append dup with cap", - num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), - should_cap_ix_accounts: true, - append_dup_account: true, - expected_err: Some(InstructionError::MaxAccountsExceeded), - }, - // This test case breaks parameter deserialization and can be cleaned up - // when should_cap_ix_accounts is enabled. - // - // TestCase { - // name: "serialize too many accounts and append dup without cap", - // num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS) + 1, - // should_cap_ix_accounts: false, - // append_dup_account: true, - // expected_err: None, - // }, - ] { - let program_id = solana_sdk::pubkey::new_rand(); - let mut transaction_accounts = vec![( - program_id, - AccountSharedData::from(Account { - lamports: 0, - data: vec![], - owner: bpf_loader::id(), - executable: true, - rent_epoch: 0, - }), - )]; - for _ in 0..num_ix_accounts { - transaction_accounts.push(( - Pubkey::new_unique(), + for copy_account_data in [true] { + for TestCase { + num_ix_accounts, + append_dup_account, + should_cap_ix_accounts, + expected_err, + name, + } in [ + TestCase { + name: "serialize max accounts without cap", + num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), + should_cap_ix_accounts: false, + append_dup_account: false, + expected_err: None, + }, + TestCase { + name: "serialize max accounts and append dup without cap", + num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), + should_cap_ix_accounts: false, + append_dup_account: true, + expected_err: None, + }, + TestCase { + name: "serialize max accounts with cap", + num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), + should_cap_ix_accounts: true, + append_dup_account: false, + expected_err: None, + }, + TestCase { + name: "serialize too many accounts with cap", + num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS) + 1, + should_cap_ix_accounts: true, + append_dup_account: false, + expected_err: Some(InstructionError::MaxAccountsExceeded), + }, + TestCase { + name: "serialize too many accounts and append dup with cap", + num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS), + should_cap_ix_accounts: true, + append_dup_account: true, + expected_err: Some(InstructionError::MaxAccountsExceeded), + }, + // This test case breaks parameter deserialization and can be cleaned up + // when should_cap_ix_accounts is enabled. + // + // TestCase { + // name: "serialize too many accounts and append dup without cap", + // num_ix_accounts: usize::from(MAX_INSTRUCTION_ACCOUNTS) + 1, + // should_cap_ix_accounts: false, + // append_dup_account: true, + // expected_err: None, + // }, + ] { + let program_id = solana_sdk::pubkey::new_rand(); + let mut transaction_accounts = vec![( + program_id, AccountSharedData::from(Account { lamports: 0, data: vec![], - owner: program_id, - executable: false, + owner: bpf_loader::id(), + executable: true, rent_epoch: 0, }), - )); + )]; + for _ in 0..num_ix_accounts { + transaction_accounts.push(( + Pubkey::new_unique(), + AccountSharedData::from(Account { + lamports: 0, + data: vec![], + owner: program_id, + executable: false, + rent_epoch: 0, + }), + )); + } + let mut instruction_accounts: Vec<_> = (0..num_ix_accounts as IndexOfAccount) + .map(|index_in_callee| InstructionAccount { + index_in_transaction: index_in_callee + 1, + index_in_caller: index_in_callee + 1, + index_in_callee, + is_signer: false, + is_writable: false, + }) + .collect(); + if append_dup_account { + instruction_accounts.push(instruction_accounts.last().cloned().unwrap()); + } + let program_indices = [0]; + let instruction_data = vec![]; + + with_mock_invoke_context!( + invoke_context, + transaction_context, + transaction_accounts + ); + invoke_context + .transaction_context + .get_next_instruction_context() + .unwrap() + .configure(&program_indices, &instruction_accounts, &instruction_data); + invoke_context.push().unwrap(); + let instruction_context = invoke_context + .transaction_context + .get_current_instruction_context() + .unwrap(); + + let serialization_result = serialize_parameters( + invoke_context.transaction_context, + instruction_context, + should_cap_ix_accounts, + copy_account_data, + ); + assert_eq!( + serialization_result.as_ref().err(), + expected_err.as_ref(), + "{name} test case failed", + ); + if expected_err.is_some() { + continue; + } + + let (mut serialized, regions, _account_lengths) = serialization_result.unwrap(); + let mut serialized_regions = concat_regions(®ions); + let (de_program_id, de_accounts, de_instruction_data) = unsafe { + deserialize( + if copy_account_data { + serialized.as_slice_mut() + } else { + serialized_regions.as_slice_mut() + } + .first_mut() + .unwrap() as *mut u8, + ) + }; + assert_eq!(de_program_id, &program_id); + assert_eq!(de_instruction_data, &instruction_data); + for account_info in de_accounts { + let index_in_transaction = invoke_context + .transaction_context + .find_index_of_account(account_info.key) + .unwrap(); + let account = invoke_context + .transaction_context + .get_account_at_index(index_in_transaction) + .unwrap() + .borrow(); + assert_eq!(account.lamports(), account_info.lamports()); + assert_eq!(account.data(), &account_info.data.borrow()[..]); + assert_eq!(account.owner(), account_info.owner); + assert_eq!(account.executable(), account_info.executable); + assert_eq!(account.rent_epoch(), account_info.rent_epoch); + } } - let mut instruction_accounts: Vec<_> = (0..num_ix_accounts as IndexOfAccount) - .map(|index_in_callee| InstructionAccount { - index_in_transaction: index_in_callee + 1, - index_in_caller: index_in_callee + 1, - index_in_callee, - is_signer: false, - is_writable: false, - }) + } + } + + #[test] + fn test_serialize_parameters() { + for copy_account_data in [false, true] { + let program_id = solana_sdk::pubkey::new_rand(); + let transaction_accounts = vec![ + ( + program_id, + AccountSharedData::from(Account { + lamports: 0, + data: vec![], + owner: bpf_loader::id(), + executable: true, + rent_epoch: 0, + }), + ), + ( + solana_sdk::pubkey::new_rand(), + AccountSharedData::from(Account { + lamports: 1, + data: vec![1u8, 2, 3, 4, 5], + owner: bpf_loader::id(), + executable: false, + rent_epoch: 100, + }), + ), + ( + solana_sdk::pubkey::new_rand(), + AccountSharedData::from(Account { + lamports: 2, + data: vec![11u8, 12, 13, 14, 15, 16, 17, 18, 19], + owner: bpf_loader::id(), + executable: true, + rent_epoch: 200, + }), + ), + ( + solana_sdk::pubkey::new_rand(), + AccountSharedData::from(Account { + lamports: 3, + data: vec![], + owner: bpf_loader::id(), + executable: false, + rent_epoch: 3100, + }), + ), + ( + solana_sdk::pubkey::new_rand(), + AccountSharedData::from(Account { + lamports: 4, + data: vec![1u8, 2, 3, 4, 5], + owner: bpf_loader::id(), + executable: false, + rent_epoch: 100, + }), + ), + ( + solana_sdk::pubkey::new_rand(), + AccountSharedData::from(Account { + lamports: 5, + data: vec![11u8, 12, 13, 14, 15, 16, 17, 18, 19], + owner: bpf_loader::id(), + executable: true, + rent_epoch: 200, + }), + ), + ( + solana_sdk::pubkey::new_rand(), + AccountSharedData::from(Account { + lamports: 6, + data: vec![], + owner: bpf_loader::id(), + executable: false, + rent_epoch: 3100, + }), + ), + ]; + let instruction_accounts: Vec = [1, 1, 2, 3, 4, 4, 5, 6] + .into_iter() + .enumerate() + .map( + |(index_in_instruction, index_in_transaction)| InstructionAccount { + index_in_transaction, + index_in_caller: index_in_transaction, + index_in_callee: index_in_transaction - 1, + is_signer: false, + is_writable: index_in_instruction >= 4, + }, + ) .collect(); - if append_dup_account { - instruction_accounts.push(instruction_accounts.last().cloned().unwrap()); - } + let instruction_data = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]; let program_indices = [0]; - let instruction_data = vec![]; - - let mut transaction_context = - TransactionContext::new(transaction_accounts, Some(Rent::default()), 1, 1); - transaction_context + let mut original_accounts = transaction_accounts.clone(); + with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts); + invoke_context + .transaction_context .get_next_instruction_context() .unwrap() .configure(&program_indices, &instruction_accounts, &instruction_data); - transaction_context.push().unwrap(); - let instruction_context = transaction_context - .get_instruction_context_at_index_in_trace(0) + invoke_context.push().unwrap(); + let instruction_context = invoke_context + .transaction_context + .get_current_instruction_context() .unwrap(); - let serialization_result = serialize_parameters( - &transaction_context, + // check serialize_parameters_aligned + let (mut serialized, regions, account_lengths) = serialize_parameters( + invoke_context.transaction_context, instruction_context, - should_cap_ix_accounts, - ); + true, + copy_account_data, + ) + .unwrap(); + + let mut serialized_regions = concat_regions(®ions); + if copy_account_data { + assert_eq!(serialized.as_slice(), serialized_regions.as_slice()); + } + let (de_program_id, de_accounts, de_instruction_data) = unsafe { + deserialize( + if copy_account_data { + serialized.as_slice_mut() + } else { + serialized_regions.as_slice_mut() + } + .first_mut() + .unwrap() as *mut u8, + ) + }; + + assert_eq!(&program_id, de_program_id); + assert_eq!(instruction_data, de_instruction_data); assert_eq!( - serialization_result.as_ref().err(), - expected_err.as_ref(), - "{name} test case failed" + (de_instruction_data.first().unwrap() as *const u8).align_offset(BPF_ALIGN_OF_U128), + 0 ); - if expected_err.is_some() { - continue; - } + for account_info in de_accounts { + let index_in_transaction = invoke_context + .transaction_context + .find_index_of_account(account_info.key) + .unwrap(); + let account = invoke_context + .transaction_context + .get_account_at_index(index_in_transaction) + .unwrap() + .borrow(); + assert_eq!(account.lamports(), account_info.lamports()); + assert_eq!(account.data(), &account_info.data.borrow()[..]); + assert_eq!(account.owner(), account_info.owner); + assert_eq!(account.executable(), account_info.executable); + assert_eq!(account.rent_epoch(), account_info.rent_epoch); - let (mut serialized, _regions, _account_lengths) = serialization_result.unwrap(); - let (de_program_id, de_accounts, de_instruction_data) = - unsafe { deserialize(serialized.as_slice_mut().first_mut().unwrap() as *mut u8) }; - assert_eq!(de_program_id, &program_id); - assert_eq!(de_instruction_data, &instruction_data); - for (index, account_info) in de_accounts.into_iter().enumerate() { - let ix_account = &instruction_accounts.get(index).unwrap(); assert_eq!( - account_info.key, - transaction_context - .get_key_of_account_at_index(ix_account.index_in_transaction) - .unwrap() + (*account_info.lamports.borrow() as *const u64).align_offset(BPF_ALIGN_OF_U128), + 0 + ); + assert_eq!( + account_info + .data + .borrow() + .as_ptr() + .align_offset(BPF_ALIGN_OF_U128), + 0 ); - assert_eq!(account_info.owner, &program_id); - assert!(!account_info.executable); - assert!(account_info.data_is_empty()); - assert!(!account_info.is_writable); - assert!(!account_info.is_signer); } - } - } - #[test] - fn test_serialize_parameters() { - let program_id = solana_sdk::pubkey::new_rand(); - let transaction_accounts = vec![ - ( - program_id, - AccountSharedData::from(Account { - lamports: 0, - data: vec![], - owner: bpf_loader::id(), - executable: true, - rent_epoch: 0, - }), - ), - ( - solana_sdk::pubkey::new_rand(), - AccountSharedData::from(Account { - lamports: 1, - data: vec![1u8, 2, 3, 4, 5], - owner: bpf_loader::id(), - executable: false, - rent_epoch: 100, - }), - ), - ( - solana_sdk::pubkey::new_rand(), - AccountSharedData::from(Account { - lamports: 2, - data: vec![11u8, 12, 13, 14, 15, 16, 17, 18, 19], - owner: bpf_loader::id(), - executable: true, - rent_epoch: 200, - }), - ), - ( - solana_sdk::pubkey::new_rand(), - AccountSharedData::from(Account { - lamports: 3, - data: vec![], - owner: bpf_loader::id(), - executable: false, - rent_epoch: 3100, - }), - ), - ( - solana_sdk::pubkey::new_rand(), - AccountSharedData::from(Account { - lamports: 4, - data: vec![1u8, 2, 3, 4, 5], - owner: bpf_loader::id(), - executable: false, - rent_epoch: 100, - }), - ), - ( - solana_sdk::pubkey::new_rand(), - AccountSharedData::from(Account { - lamports: 5, - data: vec![11u8, 12, 13, 14, 15, 16, 17, 18, 19], - owner: bpf_loader::id(), - executable: true, - rent_epoch: 200, - }), - ), - ( - solana_sdk::pubkey::new_rand(), - AccountSharedData::from(Account { - lamports: 6, - data: vec![], - owner: bpf_loader::id(), - executable: false, - rent_epoch: 3100, - }), - ), - ]; - let instruction_accounts: Vec = [1, 1, 2, 3, 4, 4, 5, 6] - .into_iter() - .enumerate() - .map( - |(index_in_instruction, index_in_transaction)| InstructionAccount { - index_in_transaction, - index_in_caller: index_in_transaction, - index_in_callee: index_in_transaction - 1, - is_signer: false, - is_writable: index_in_instruction >= 4, - }, + deserialize_parameters( + invoke_context.transaction_context, + instruction_context, + copy_account_data, + serialized.as_slice(), + &account_lengths, ) - .collect(); - let instruction_data = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]; - let program_indices = [0]; - let mut original_accounts = transaction_accounts.clone(); - with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts); - invoke_context - .transaction_context - .get_next_instruction_context() - .unwrap() - .configure(&program_indices, &instruction_accounts, &instruction_data); - invoke_context.push().unwrap(); - let instruction_context = invoke_context - .transaction_context - .get_current_instruction_context() .unwrap(); + for (index_in_transaction, (_key, original_account)) in + original_accounts.iter().enumerate() + { + let account = invoke_context + .transaction_context + .get_account_at_index(index_in_transaction as IndexOfAccount) + .unwrap() + .borrow(); + assert_eq!(&*account, original_account); + } - // check serialize_parameters_aligned - - let (mut serialized, _regions, account_lengths) = serialize_parameters( - invoke_context.transaction_context, - instruction_context, - true, - ) - .unwrap(); - - let (de_program_id, de_accounts, de_instruction_data) = - unsafe { deserialize(serialized.as_slice_mut().first_mut().unwrap() as *mut u8) }; - - assert_eq!(&program_id, de_program_id); - assert_eq!(instruction_data, de_instruction_data); - assert_eq!( - (de_instruction_data.first().unwrap() as *const u8).align_offset(BPF_ALIGN_OF_U128), - 0 - ); - for account_info in de_accounts { - let index_in_transaction = invoke_context - .transaction_context - .find_index_of_account(account_info.key) - .unwrap(); - let account = invoke_context - .transaction_context - .get_account_at_index(index_in_transaction) + // check serialize_parameters_unaligned + original_accounts + .first_mut() .unwrap() - .borrow(); - assert_eq!(account.lamports(), account_info.lamports()); - assert_eq!(account.data(), &account_info.data.borrow()[..]); - assert_eq!(account.owner(), account_info.owner); - assert_eq!(account.executable(), account_info.executable); - assert_eq!(account.rent_epoch(), account_info.rent_epoch); - - assert_eq!( - (*account_info.lamports.borrow() as *const u64).align_offset(BPF_ALIGN_OF_U128), - 0 - ); - assert_eq!( - account_info - .data - .borrow() - .as_ptr() - .align_offset(BPF_ALIGN_OF_U128), - 0 - ); - } - - deserialize_parameters( - invoke_context.transaction_context, - instruction_context, - serialized.as_slice(), - &account_lengths, - ) - .unwrap(); - for (index_in_transaction, (_key, original_account)) in original_accounts.iter().enumerate() - { - let account = invoke_context + .1 + .set_owner(bpf_loader_deprecated::id()); + invoke_context .transaction_context - .get_account_at_index(index_in_transaction as IndexOfAccount) + .get_account_at_index(0) .unwrap() - .borrow(); - assert_eq!(&*account, original_account); - } + .borrow_mut() + .set_owner(bpf_loader_deprecated::id()); - // check serialize_parameters_unaligned - original_accounts - .first_mut() - .unwrap() - .1 - .set_owner(bpf_loader_deprecated::id()); - invoke_context - .transaction_context - .get_account_at_index(0) - .unwrap() - .borrow_mut() - .set_owner(bpf_loader_deprecated::id()); - - let (mut serialized, _regions, account_lengths) = serialize_parameters( - invoke_context.transaction_context, - instruction_context, - true, - ) - .unwrap(); - - let (de_program_id, de_accounts, de_instruction_data) = unsafe { - deserialize_unaligned(serialized.as_slice_mut().first_mut().unwrap() as *mut u8) - }; - assert_eq!(&program_id, de_program_id); - assert_eq!(instruction_data, de_instruction_data); - for account_info in de_accounts { - let index_in_transaction = invoke_context - .transaction_context - .find_index_of_account(account_info.key) - .unwrap(); - let account = invoke_context - .transaction_context - .get_account_at_index(index_in_transaction) - .unwrap() - .borrow(); - assert_eq!(account.lamports(), account_info.lamports()); - assert_eq!(account.data(), &account_info.data.borrow()[..]); - assert_eq!(account.owner(), account_info.owner); - assert_eq!(account.executable(), account_info.executable); - assert_eq!(account.rent_epoch(), account_info.rent_epoch); - } + let (mut serialized, regions, account_lengths) = serialize_parameters( + invoke_context.transaction_context, + instruction_context, + true, + copy_account_data, + ) + .unwrap(); + let mut serialized_regions = concat_regions(®ions); + + let (de_program_id, de_accounts, de_instruction_data) = unsafe { + deserialize_unaligned( + if copy_account_data { + serialized.as_slice_mut() + } else { + serialized_regions.as_slice_mut() + } + .first_mut() + .unwrap() as *mut u8, + ) + }; + assert_eq!(&program_id, de_program_id); + assert_eq!(instruction_data, de_instruction_data); + for account_info in de_accounts { + let index_in_transaction = invoke_context + .transaction_context + .find_index_of_account(account_info.key) + .unwrap(); + let account = invoke_context + .transaction_context + .get_account_at_index(index_in_transaction) + .unwrap() + .borrow(); + assert_eq!(account.lamports(), account_info.lamports()); + assert_eq!(account.data(), &account_info.data.borrow()[..]); + assert_eq!(account.owner(), account_info.owner); + assert_eq!(account.executable(), account_info.executable); + assert_eq!(account.rent_epoch(), account_info.rent_epoch); + } - deserialize_parameters( - invoke_context.transaction_context, - instruction_context, - serialized.as_slice(), - &account_lengths, - ) - .unwrap(); - for (index_in_transaction, (_key, original_account)) in original_accounts.iter().enumerate() - { - let account = invoke_context - .transaction_context - .get_account_at_index(index_in_transaction as IndexOfAccount) - .unwrap() - .borrow(); - assert_eq!(&*account, original_account); + deserialize_parameters( + invoke_context.transaction_context, + instruction_context, + copy_account_data, + serialized.as_slice(), + &account_lengths, + ) + .unwrap(); + for (index_in_transaction, (_key, original_account)) in + original_accounts.iter().enumerate() + { + let account = invoke_context + .transaction_context + .get_account_at_index(index_in_transaction as IndexOfAccount) + .unwrap() + .borrow(); + assert_eq!(&*account, original_account); + } } } @@ -925,4 +1079,17 @@ mod tests { (program_id, accounts, instruction_data) } + + fn concat_regions(regions: &[MemoryRegion]) -> AlignedMemory { + let len = regions.iter().fold(0, |len, region| len + region.len) as usize; + let mut mem = AlignedMemory::zero_filled(len); + for region in regions { + let host_slice = unsafe { + slice::from_raw_parts(region.host_addr.get() as *const u8, region.len as usize) + }; + mem.as_slice_mut()[(region.vm_addr - MM_INPUT_START) as usize..][..region.len as usize] + .copy_from_slice(host_slice) + } + mem + } } diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index d9b5d29bfc4793..a53629e0a7fba2 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -9,7 +9,7 @@ use { }, transaction_context::BorrowedAccount, }, - std::mem, + std::{mem, ptr, slice}, }; /// Host side representation of AccountInfo or SolAccountInfo passed to the CPI syscall. @@ -19,8 +19,20 @@ use { struct CallerAccount<'a> { lamports: &'a mut u64, owner: &'a mut Pubkey, + // The original data length of the account at the start of the current + // instruction. We use this to determine wether an account was shrunk or + // grown before or after CPI, and to derive the vm address of the realloc + // region. original_data_len: usize, - data: &'a mut [u8], + // This points to the data section for this account, as serialized and + // mapped inside the vm (see serialize_parameters() in + // BpfExecutor::execute). + // + // When direct mapping is off, this includes both the account data _and_ the + // realloc padding. When direct mapping is on, account data is mapped in its + // own separate memory region which is directly mutated from inside the vm, + // and the serialized buffer includes only the realloc padding. + serialized_data: &'a mut [u8], // Given the corresponding input AccountInfo::data, vm_data_addr points to // the pointer field and ref_to_len_in_vm points to the length field. vm_data_addr: u64, @@ -36,6 +48,7 @@ impl<'a> CallerAccount<'a> { fn from_account_info( invoke_context: &InvokeContext, memory_mapping: &MemoryMapping, + is_loader_deprecated: bool, _vm_addr: u64, account_info: &AccountInfo, original_data_len: usize, @@ -58,7 +71,7 @@ impl<'a> CallerAccount<'a> { invoke_context.get_check_aligned(), )?; - let (data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr) = { + let (serialized_data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr) = { // Double translate data out of RefCell let data = *translate_type::<&[u8]>( memory_mapping, @@ -95,14 +108,32 @@ impl<'a> CallerAccount<'a> { )? }; let vm_data_addr = data.as_ptr() as u64; + + let bpf_account_data_direct_mapping = invoke_context + .feature_set + .is_active(&feature_set::bpf_account_data_direct_mapping::id()); + let serialized_data = translate_slice_mut::( + memory_mapping, + if bpf_account_data_direct_mapping { + vm_data_addr.saturating_add(original_data_len as u64) + } else { + vm_data_addr + }, + if bpf_account_data_direct_mapping { + if is_loader_deprecated { + 0 + } else { + MAX_PERMITTED_DATA_INCREASE + } + } else { + data.len() + } as u64, + invoke_context.get_check_aligned(), + invoke_context.get_check_size(), + )?; + ( - translate_slice_mut::( - memory_mapping, - vm_data_addr, - data.len() as u64, - invoke_context.get_check_aligned(), - invoke_context.get_check_size(), - )?, + serialized_data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, @@ -113,7 +144,7 @@ impl<'a> CallerAccount<'a> { lamports, owner, original_data_len, - data, + serialized_data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, @@ -126,6 +157,7 @@ impl<'a> CallerAccount<'a> { fn from_sol_account_info( invoke_context: &InvokeContext, memory_mapping: &MemoryMapping, + is_loader_deprecated: bool, vm_addr: u64, account_info: &SolAccountInfo, original_data_len: usize, @@ -152,10 +184,25 @@ impl<'a> CallerAccount<'a> { .saturating_div(invoke_context.get_compute_budget().cpi_bytes_per_unit), )?; - let data = translate_slice_mut::( + let bpf_account_data_direct_mapping = invoke_context + .feature_set + .is_active(&feature_set::bpf_account_data_direct_mapping::id()); + let serialized_data = translate_slice_mut::( memory_mapping, - vm_data_addr, - account_info.data_len, + if bpf_account_data_direct_mapping { + vm_data_addr.saturating_add(original_data_len as u64) + } else { + vm_data_addr + }, + if bpf_account_data_direct_mapping { + if is_loader_deprecated { + 0 + } else { + MAX_PERMITTED_DATA_INCREASE as u64 + } + } else { + account_info.data_len + }, invoke_context.get_check_aligned(), invoke_context.get_check_size(), )?; @@ -194,7 +241,7 @@ impl<'a> CallerAccount<'a> { lamports, owner, original_data_len, - data, + serialized_data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, @@ -218,6 +265,7 @@ trait SyscallInvokeSigned { program_indices: &[IndexOfAccount], account_infos_addr: u64, account_infos_len: u64, + is_loader_deprecated: bool, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, ) -> Result, Error>; @@ -310,6 +358,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { program_indices: &[IndexOfAccount], account_infos_addr: u64, account_infos_len: u64, + is_loader_deprecated: bool, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, ) -> Result, Error> { @@ -327,6 +376,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { &account_info_keys, account_infos, account_infos_addr, + is_loader_deprecated, invoke_context, memory_mapping, CallerAccount::from_account_info, @@ -541,6 +591,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { program_indices: &[IndexOfAccount], account_infos_addr: u64, account_infos_len: u64, + is_loader_deprecated: bool, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, ) -> Result, Error> { @@ -558,6 +609,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { &account_info_keys, account_infos, account_infos_addr, + is_loader_deprecated, invoke_context, memory_mapping, CallerAccount::from_sol_account_info, @@ -657,12 +709,13 @@ fn translate_and_update_accounts<'a, T, F>( account_info_keys: &[&Pubkey], account_infos: &[T], account_infos_addr: u64, + is_loader_deprecated: bool, invoke_context: &mut InvokeContext, memory_mapping: &MemoryMapping, do_translate: F, ) -> Result, Error> where - F: Fn(&InvokeContext, &MemoryMapping, u64, &T, usize) -> Result, Error>, + F: Fn(&InvokeContext, &MemoryMapping, bool, u64, &T, usize) -> Result, Error>, { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; @@ -680,6 +733,10 @@ where .unwrap() .orig_account_lengths; + let direct_mapping = invoke_context + .feature_set + .is_active(&feature_set::bpf_account_data_direct_mapping::id()); + for (instruction_account_index, instruction_account) in instruction_accounts.iter().enumerate() { if instruction_account_index as IndexOfAccount != instruction_account.index_in_callee { @@ -722,6 +779,7 @@ where do_translate( invoke_context, memory_mapping, + is_loader_deprecated, account_infos_addr.saturating_add( caller_account_index.saturating_mul(mem::size_of::()) as u64, ), @@ -735,7 +793,13 @@ where // account (caller_account). We need to update the corresponding // BorrowedAccount (callee_account) so the callee can see the // changes. - update_callee_account(invoke_context, &caller_account, callee_account)?; + update_callee_account( + invoke_context, + is_loader_deprecated, + &caller_account, + callee_account, + direct_mapping, + )?; let caller_account = if instruction_account.is_writable { Some(caller_account) @@ -887,14 +951,20 @@ fn cpi_common( memory_mapping, invoke_context, )?; + let is_loader_deprecated = *instruction_context + .try_borrow_last_program_account(transaction_context)? + .get_owner() + == bpf_loader_deprecated::id(); let (instruction_accounts, program_indices) = invoke_context.prepare_instruction(&instruction, &signers)?; check_authorized_program(&instruction.program_id, &instruction.data, invoke_context)?; + let mut accounts = S::translate_accounts( &instruction_accounts, &program_indices, account_infos_addr, account_infos_len, + is_loader_deprecated, memory_mapping, invoke_context, )?; @@ -916,15 +986,21 @@ fn cpi_common( // CPI exit. // // Synchronize the callee's account changes so the caller can see them. + let direct_mapping = invoke_context + .feature_set + .is_active(&feature_set::bpf_account_data_direct_mapping::id()); + for (index_in_caller, caller_account) in accounts.iter_mut() { if let Some(caller_account) = caller_account { - let callee_account = instruction_context + let mut callee_account = instruction_context .try_borrow_instruction_account(transaction_context, *index_in_caller)?; update_caller_account( invoke_context, memory_mapping, + is_loader_deprecated, caller_account, - &callee_account, + &mut callee_account, + direct_mapping, )?; } } @@ -942,27 +1018,58 @@ fn cpi_common( // changes. fn update_callee_account( invoke_context: &InvokeContext, + is_loader_deprecated: bool, caller_account: &CallerAccount, mut callee_account: BorrowedAccount<'_>, + direct_mapping: bool, ) -> Result<(), Error> { let is_disable_cpi_setting_executable_and_rent_epoch_active = invoke_context .feature_set .is_active(&disable_cpi_setting_executable_and_rent_epoch::id()); - if callee_account.get_lamports() != *caller_account.lamports { callee_account.set_lamports(*caller_account.lamports)?; } - // The redundant check helps to avoid the expensive data comparison if we can - match callee_account - .can_data_be_resized(caller_account.data.len()) - .and_then(|_| callee_account.can_data_be_changed()) - { - Ok(()) => callee_account.set_data_from_slice(caller_account.data)?, - Err(err) if callee_account.get_data() != caller_account.data => { - return Err(Box::new(err)); + if direct_mapping { + let prev_len = callee_account.get_data().len(); + let post_len = *caller_account.ref_to_len_in_vm as usize; + match callee_account + .can_data_be_resized(post_len) + .and_then(|_| callee_account.can_data_be_changed()) + { + Ok(()) => { + callee_account.set_data_length(post_len)?; + let realloc_bytes_used = post_len.saturating_sub(caller_account.original_data_len); + if !is_loader_deprecated && realloc_bytes_used > 0 { + callee_account + .get_data_mut()? + .get_mut(caller_account.original_data_len..post_len) + .ok_or(SyscallError::InvalidLength)? + .copy_from_slice( + caller_account + .serialized_data + .get(0..realloc_bytes_used) + .ok_or(SyscallError::InvalidLength)?, + ); + } + } + Err(err) if prev_len != post_len => { + return Err(Box::new(err)); + } + _ => {} + } + } else { + // The redundant check helps to avoid the expensive data comparison if we can + match callee_account + .can_data_be_resized(caller_account.serialized_data.len()) + .and_then(|_| callee_account.can_data_be_changed()) + { + Ok(()) => callee_account.set_data_from_slice(caller_account.serialized_data)?, + Err(err) if callee_account.get_data() != caller_account.serialized_data => { + return Err(Box::new(err)); + } + _ => {} } - _ => {} } if !is_disable_cpi_setting_executable_and_rent_epoch_active @@ -1011,42 +1118,101 @@ fn update_callee_account( // changes. fn update_caller_account( invoke_context: &InvokeContext, - memory_mapping: &MemoryMapping, + memory_mapping: &mut MemoryMapping, + is_loader_deprecated: bool, caller_account: &mut CallerAccount, - callee_account: &BorrowedAccount<'_>, + callee_account: &mut BorrowedAccount<'_>, + direct_mapping: bool, ) -> Result<(), Error> { *caller_account.lamports = callee_account.get_lamports(); *caller_account.owner = *callee_account.get_owner(); - let new_len = callee_account.get_data().len(); - if caller_account.data.len() != new_len { - let data_overflow = new_len + + if direct_mapping && caller_account.original_data_len > 0 { + // Since each instruction account is directly mapped in a memory region + // with a *fixed* length, upon returning from CPI we must ensure that the + // current capacity is at least the original length (what is mapped in + // memory), so that the account's memory region never points to an + // invalid address. + let min_capacity = caller_account.original_data_len; + if callee_account.capacity() < min_capacity { + callee_account.reserve(min_capacity.saturating_sub(callee_account.capacity()))? + } + + // If an account's data pointer has changed - because of CoW or because + // of using AccountSharedData directly (deprecated) - we must update the + // corresponding MemoryRegion in the caller's address space. Address + // spaces are fixed so we don't need to update the MemoryRegion's length. + let region = memory_mapping.region(AccessType::Load, caller_account.vm_data_addr)?; + let callee_ptr = callee_account.get_data().as_ptr() as u64; + if region.host_addr.get() != callee_ptr { + region.host_addr.set(callee_ptr); + } + } + let prev_len = *caller_account.ref_to_len_in_vm as usize; + let post_len = callee_account.get_data().len(); + if prev_len != post_len { + let max_increase = if direct_mapping && !invoke_context.get_check_aligned() { + 0 + } else { + MAX_PERMITTED_DATA_INCREASE + }; + let data_overflow = post_len > caller_account .original_data_len - .saturating_add(MAX_PERMITTED_DATA_INCREASE); + .saturating_add(max_increase); if data_overflow { ic_msg!( invoke_context, - "Account data size realloc limited to {} in inner instructions", - MAX_PERMITTED_DATA_INCREASE + "Account data size realloc limited to {max_increase} in inner instructions", ); return Err(Box::new(InstructionError::InvalidRealloc)); } - if new_len < caller_account.data.len() { - caller_account - .data - .get_mut(new_len..) - .ok_or_else(|| Box::new(InstructionError::AccountDataTooSmall))? - .fill(0); + if post_len < prev_len { + if direct_mapping { + if post_len < caller_account.original_data_len { + // zero the spare capacity in the account data. We only need + // to zero up to the original data length, everything else + // is not accessible from the vm anyway. + let spare_len = caller_account.original_data_len.saturating_sub(post_len); + let dst = callee_account + .spare_data_capacity_mut()? + .get_mut(..spare_len) + .ok_or_else(|| Box::new(InstructionError::AccountDataTooSmall))? + .as_mut_ptr(); + // Safety: we check bounds above + unsafe { ptr::write_bytes(dst, 0, spare_len) }; + } + + // zero the spare capacity in the realloc padding + let spare_realloc = unsafe { + slice::from_raw_parts_mut( + caller_account.serialized_data.as_mut_ptr(), + prev_len.saturating_sub(caller_account.original_data_len), + ) + }; + spare_realloc.fill(0); + } else { + caller_account + .serialized_data + .get_mut(post_len..) + .ok_or_else(|| Box::new(InstructionError::AccountDataTooSmall))? + .fill(0); + } + } + + // with direct_mapping on, serialized_data is fixed and holds the + // realloc padding + if !direct_mapping { + caller_account.serialized_data = translate_slice_mut::( + memory_mapping, + caller_account.vm_data_addr, + post_len as u64, + false, // Don't care since it is byte aligned + invoke_context.get_check_size(), + )?; } - caller_account.data = translate_slice_mut::( - memory_mapping, - caller_account.vm_data_addr, - new_len as u64, - false, // Don't care since it is byte aligned - invoke_context.get_check_size(), - )?; // this is the len field in the AccountInfo::data slice - *caller_account.ref_to_len_in_vm = new_len as u64; + *caller_account.ref_to_len_in_vm = post_len as u64; // this is the len field in the serialized parameters if invoke_context @@ -1060,22 +1226,35 @@ fn update_caller_account( .saturating_sub(std::mem::size_of::() as u64), invoke_context.get_check_aligned(), )?; - *serialized_len_ptr = new_len as u64; + *serialized_len_ptr = post_len as u64; } else { unsafe { - *caller_account.serialized_len_ptr = new_len as u64; + *caller_account.serialized_len_ptr = post_len as u64; } } } - let to_slice = &mut caller_account.data; - let from_slice = callee_account - .get_data() - .get(0..new_len) - .ok_or(SyscallError::InvalidLength)?; - if to_slice.len() != from_slice.len() { - return Err(Box::new(InstructionError::AccountDataTooSmall)); + let realloc_bytes_used = post_len.saturating_sub(caller_account.original_data_len); + if !direct_mapping { + let to_slice = &mut caller_account.serialized_data; + let from_slice = callee_account + .get_data() + .get(0..post_len) + .ok_or(SyscallError::InvalidLength)?; + if to_slice.len() != from_slice.len() { + return Err(Box::new(InstructionError::AccountDataTooSmall)); + } + to_slice.copy_from_slice(from_slice); + } else if !is_loader_deprecated && realloc_bytes_used > 0 { + let to_slice = caller_account + .serialized_data + .get_mut(0..realloc_bytes_used) + .ok_or(SyscallError::InvalidLength)?; + let from_slice = callee_account + .get_data() + .get(caller_account.original_data_len..post_len) + .ok_or(SyscallError::InvalidLength)?; + to_slice.copy_from_slice(from_slice); } - to_slice.copy_from_slice(from_slice); Ok(()) } @@ -1092,7 +1271,9 @@ mod tests { solana_sdk::{ account::{Account, AccountSharedData}, clock::Epoch, + feature_set::bpf_account_data_direct_mapping, instruction::Instruction, + system_program, transaction_context::TransactionAccount, }, std::{ @@ -1129,7 +1310,9 @@ mod tests { .into_iter() .map(|a| (a.0, a.1)) .collect::>(); - with_mock_invoke_context!($invoke_context, transaction_context, transaction_accounts); + with_mock_invoke_context!($invoke_context, $transaction_context, transaction_accounts); + let feature_set = Arc::make_mut(&mut $invoke_context.feature_set); + feature_set.deactivate(&bpf_account_data_direct_mapping::id()); $invoke_context .transaction_context .get_next_instruction_context() @@ -1139,9 +1322,22 @@ mod tests { }; } + macro_rules! borrow_instruction_account { + ($invoke_context:expr, $index:expr) => {{ + let instruction_context = $invoke_context + .transaction_context + .get_current_instruction_context() + .unwrap(); + instruction_context + .try_borrow_instruction_account($invoke_context.transaction_context, $index) + .unwrap() + }}; + } + #[test] fn test_translate_instruction() { - let transaction_accounts = transaction_with_one_instruction_account(b"foo".to_vec()); + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foo".to_vec()); mock_invoke_context!( invoke_context, transaction_context, @@ -1185,7 +1381,8 @@ mod tests { #[test] fn test_translate_signers() { - let transaction_accounts = transaction_with_one_instruction_account(b"foo".to_vec()); + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foo".to_vec()); mock_invoke_context!( invoke_context, transaction_context, @@ -1220,7 +1417,8 @@ mod tests { #[test] fn test_caller_account_from_account_info() { - let transaction_accounts = transaction_with_one_instruction_account(b"foo".to_vec()); + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foo".to_vec()); let account = transaction_accounts[1].1.clone(); mock_invoke_context!( invoke_context, @@ -1246,6 +1444,7 @@ mod tests { let caller_account = CallerAccount::from_account_info( &invoke_context, &memory_mapping, + false, vm_addr, account_info, account.data().len(), @@ -1258,14 +1457,14 @@ mod tests { *caller_account.ref_to_len_in_vm as usize, account.data().len() ); - assert_eq!(caller_account.data, account.data()); + assert_eq!(caller_account.serialized_data, account.data()); assert_eq!(caller_account.executable, account.executable()); assert_eq!(caller_account.rent_epoch, account.rent_epoch()); } #[test] fn test_update_caller_account_lamports_owner() { - let transaction_accounts = transaction_with_one_instruction_account(vec![]); + let transaction_accounts = transaction_with_one_writable_instruction_account(vec![]); let account = transaction_accounts[1].1.clone(); mock_invoke_context!( invoke_context, @@ -1276,26 +1475,24 @@ mod tests { &[1] ); - let instruction_context = invoke_context - .transaction_context - .get_current_instruction_context() - .unwrap(); - - let mut mock_caller_account = - MockCallerAccount::new(1234, *account.owner(), 0xFFFFFFFF00000000, account.data()); + let mut mock_caller_account = MockCallerAccount::new( + 1234, + *account.owner(), + 0xFFFFFFFF00000000, + account.data(), + false, + ); let config = Config { aligned_memory_mapping: false, ..Config::default() }; - let memory_mapping = + let mut memory_mapping = MemoryMapping::new(mock_caller_account.regions.split_off(0), &config).unwrap(); let mut caller_account = mock_caller_account.caller_account(); - let mut callee_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, 0) - .unwrap(); + let mut callee_account = borrow_instruction_account!(invoke_context, 0); callee_account.set_lamports(42).unwrap(); callee_account @@ -1304,9 +1501,11 @@ mod tests { update_caller_account( &invoke_context, - &memory_mapping, + &mut memory_mapping, + false, &mut caller_account, - &callee_account, + &mut callee_account, + false, ) .unwrap(); @@ -1316,7 +1515,8 @@ mod tests { #[test] fn test_update_caller_account_data() { - let transaction_accounts = transaction_with_one_instruction_account(b"foobar".to_vec()); + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foobar".to_vec()); let account = transaction_accounts[1].1.clone(); let original_data_len = account.data().len(); @@ -1329,23 +1529,19 @@ mod tests { &[1] ); - let instruction_context = invoke_context - .transaction_context - .get_current_instruction_context() - .unwrap(); - let mut mock_caller_account = MockCallerAccount::new( account.lamports(), *account.owner(), 0xFFFFFFFF00000000, account.data(), + false, ); let config = Config { aligned_memory_mapping: false, ..Config::default() }; - let memory_mapping = + let mut memory_mapping = MemoryMapping::new(mock_caller_account.regions.split_off(0), &config).unwrap(); let data_slice = mock_caller_account.data_slice(); @@ -1357,31 +1553,34 @@ mod tests { let serialized_len = || unsafe { *len_ptr.cast::() as usize }; let mut caller_account = mock_caller_account.caller_account(); - let mut callee_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, 0) - .unwrap(); + let mut callee_account = borrow_instruction_account!(invoke_context, 0); for (new_value, expected_realloc_size) in [ (b"foo".to_vec(), MAX_PERMITTED_DATA_INCREASE + 3), (b"foobaz".to_vec(), MAX_PERMITTED_DATA_INCREASE), (b"foobazbad".to_vec(), MAX_PERMITTED_DATA_INCREASE - 3), ] { - assert_eq!(caller_account.data, callee_account.get_data()); + assert_eq!(caller_account.serialized_data, callee_account.get_data()); callee_account.set_data_from_slice(&new_value).unwrap(); update_caller_account( &invoke_context, - &memory_mapping, + &mut memory_mapping, + false, &mut caller_account, - &callee_account, + &mut callee_account, + false, ) .unwrap(); let data_len = callee_account.get_data().len(); assert_eq!(data_len, *caller_account.ref_to_len_in_vm as usize); assert_eq!(data_len, serialized_len()); - assert_eq!(data_len, caller_account.data.len()); - assert_eq!(callee_account.get_data(), &caller_account.data[..data_len]); + assert_eq!(data_len, caller_account.serialized_data.len()); + assert_eq!( + callee_account.get_data(), + &caller_account.serialized_data[..data_len] + ); assert_eq!(data_slice[data_len..].len(), expected_realloc_size); assert!(is_zeroed(&data_slice[data_len..])); } @@ -1391,9 +1590,11 @@ mod tests { .unwrap(); update_caller_account( &invoke_context, - &memory_mapping, + &mut memory_mapping, + false, &mut caller_account, - &callee_account, + &mut callee_account, + false, ) .unwrap(); let data_len = callee_account.get_data().len(); @@ -1406,29 +1607,272 @@ mod tests { assert!(matches!( update_caller_account( &invoke_context, - &memory_mapping, + &mut memory_mapping, + false, &mut caller_account, - &callee_account, + &mut callee_account, + false, ), Err(error) if error.downcast_ref::().unwrap() == &InstructionError::InvalidRealloc, )); + + // close the account + callee_account.set_data_length(0).unwrap(); + callee_account + .set_owner(system_program::id().as_ref()) + .unwrap(); + update_caller_account( + &invoke_context, + &mut memory_mapping, + false, + &mut caller_account, + &mut callee_account, + false, + ) + .unwrap(); + let data_len = callee_account.get_data().len(); + assert_eq!(data_len, 0); } - macro_rules! borrow_instruction_account { - ($invoke_context:expr, $index:expr) => {{ - let instruction_context = $invoke_context - .transaction_context - .get_current_instruction_context() + #[test] + fn test_update_caller_account_data_direct_mapping() { + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foobar".to_vec()); + let account = transaction_accounts[1].1.clone(); + let original_data_len = account.data().len(); + + mock_invoke_context!( + invoke_context, + transaction_context, + b"instruction data", + transaction_accounts, + &[0], + &[1] + ); + + let mut mock_caller_account = MockCallerAccount::new( + account.lamports(), + *account.owner(), + 0xFFFFFFFF00000000, + account.data(), + true, + ); + + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mut memory_mapping = + MemoryMapping::new(mock_caller_account.regions.split_off(0), &config).unwrap(); + + let data_slice = mock_caller_account.data_slice(); + let len_ptr = unsafe { + data_slice + .as_ptr() + .offset(-(mem::size_of::() as isize)) + }; + let serialized_len = || unsafe { *len_ptr.cast::() as usize }; + let mut caller_account = mock_caller_account.caller_account(); + + let mut callee_account = borrow_instruction_account!(invoke_context, 0); + + for change_ptr in [false, true] { + for (new_value, expected_realloc_used) in [ + (b"foobazbad".to_vec(), 3), // > original_data_len, writes into realloc + (b"foo".to_vec(), 0), // < original_data_len, zeroes account capacity + realloc capacity + (b"foobaz".to_vec(), 0), // = original_data_len + (vec![], 0), // check lower bound + ] { + if change_ptr { + callee_account.set_data(new_value).unwrap(); + } else { + callee_account.set_data_from_slice(&new_value).unwrap(); + } + + update_caller_account( + &invoke_context, + &mut memory_mapping, + false, + &mut caller_account, + &mut callee_account, + true, + ) .unwrap(); - instruction_context - .try_borrow_instruction_account($invoke_context.transaction_context, $index) + + // check that the caller account data pointer always matches the callee account data pointer + assert_eq!( + translate_slice::( + &memory_mapping, + caller_account.vm_data_addr, + 1, + true, + true + ) + .unwrap() + .as_ptr(), + callee_account.get_data().as_ptr() + ); + + let data_len = callee_account.get_data().len(); + // the account info length must get updated + assert_eq!(data_len, *caller_account.ref_to_len_in_vm as usize); + // the length slot in the serialization parameters must be updated + assert_eq!(data_len, serialized_len()); + + // with direct mapping on, serialized_data contains the realloc padding + let realloc_area = &caller_account.serialized_data; + + if data_len < original_data_len { + // if an account gets resized below its original data length, + // the spare capacity is zeroed + let original_data_slice = unsafe { + slice::from_raw_parts(callee_account.get_data().as_ptr(), original_data_len) + }; + + let spare_capacity = &original_data_slice[original_data_len - data_len..]; + assert!( + is_zeroed(spare_capacity), + "dirty account spare capacity {spare_capacity:?}", + ); + } + + // if an account gets extended past its original length, the end + // gets written in the realloc padding + assert_eq!( + &realloc_area[..expected_realloc_used], + &callee_account.get_data()[data_len - expected_realloc_used..] + ); + + // the unused realloc padding is always zeroed + assert!( + is_zeroed(&realloc_area[expected_realloc_used..]), + "dirty realloc padding {realloc_area:?}", + ); + } + } + + callee_account + .set_data_length(original_data_len + MAX_PERMITTED_DATA_INCREASE) + .unwrap(); + update_caller_account( + &invoke_context, + &mut memory_mapping, + false, + &mut caller_account, + &mut callee_account, + true, + ) + .unwrap(); + assert!( + is_zeroed(caller_account.serialized_data), + "dirty realloc padding {:?}", + caller_account.serialized_data + ); + + callee_account + .set_data_length(original_data_len + MAX_PERMITTED_DATA_INCREASE + 1) + .unwrap(); + assert!(matches!( + update_caller_account( + &invoke_context, + &mut memory_mapping, + false, + &mut caller_account, + &mut callee_account, + false, + ), + Err(error) if error.downcast_ref::().unwrap() == &InstructionError::InvalidRealloc + )); + + // close the account + callee_account.set_data_length(0).unwrap(); + callee_account + .set_owner(system_program::id().as_ref()) + .unwrap(); + update_caller_account( + &invoke_context, + &mut memory_mapping, + false, + &mut caller_account, + &mut callee_account, + true, + ) + .unwrap(); + let data_len = callee_account.get_data().len(); + assert_eq!(data_len, 0); + } + + #[test] + fn test_update_caller_account_data_capacity_direct_mapping() { + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foobar".to_vec()); + let account = transaction_accounts[1].1.clone(); + + mock_invoke_context!( + invoke_context, + transaction_context, + b"instruction data", + transaction_accounts, + &[0], + &[1] + ); + + let mut mock_caller_account = MockCallerAccount::new( + account.lamports(), + *account.owner(), + 0xFFFFFFFF00000000, + account.data(), + true, + ); + + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mut memory_mapping = + MemoryMapping::new(mock_caller_account.regions.split_off(0), &config).unwrap(); + + let mut caller_account = mock_caller_account.caller_account(); + + { + let mut account = invoke_context + .transaction_context + .get_account_at_index(1) .unwrap() - }}; + .borrow_mut(); + account.set_data(b"baz".to_vec()); + } + + let mut callee_account = borrow_instruction_account!(invoke_context, 0); + assert_eq!(callee_account.get_data().len(), 3); + assert_eq!(callee_account.capacity(), 3); + + update_caller_account( + &invoke_context, + &mut memory_mapping, + false, + &mut caller_account, + &mut callee_account, + true, + ) + .unwrap(); + + assert_eq!(callee_account.get_data().len(), 3); + assert!(callee_account.capacity() >= caller_account.original_data_len); + let data = translate_slice::( + &memory_mapping, + caller_account.vm_data_addr, + callee_account.get_data().len() as u64, + true, + true, + ) + .unwrap(); + assert_eq!(data, callee_account.get_data()); } #[test] fn test_update_callee_account_lamports_owner() { - let transaction_accounts = transaction_with_one_instruction_account(vec![]); + let transaction_accounts = transaction_with_one_writable_instruction_account(vec![]); let account = transaction_accounts[1].1.clone(); mock_invoke_context!( @@ -1440,8 +1884,13 @@ mod tests { &[1] ); - let mut mock_caller_account = - MockCallerAccount::new(1234, *account.owner(), 0xFFFFFFFF00000000, account.data()); + let mut mock_caller_account = MockCallerAccount::new( + 1234, + *account.owner(), + 0xFFFFFFFF00000000, + account.data(), + false, + ); let caller_account = mock_caller_account.caller_account(); @@ -1450,7 +1899,14 @@ mod tests { *caller_account.lamports = 42; *caller_account.owner = Pubkey::new_unique(); - update_callee_account(&invoke_context, &caller_account, callee_account).unwrap(); + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + false, + ) + .unwrap(); let callee_account = borrow_instruction_account!(invoke_context, 0); assert_eq!(callee_account.get_lamports(), 42); @@ -1459,7 +1915,8 @@ mod tests { #[test] fn test_update_callee_account_data() { - let transaction_accounts = transaction_with_one_instruction_account(b"foobar".to_vec()); + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foobar".to_vec()); let account = transaction_accounts[1].1.clone(); mock_invoke_context!( @@ -1471,25 +1928,200 @@ mod tests { &[1] ); - let mut mock_caller_account = - MockCallerAccount::new(1234, *account.owner(), 0xFFFFFFFF00000000, account.data()); + let mut mock_caller_account = MockCallerAccount::new( + 1234, + *account.owner(), + 0xFFFFFFFF00000000, + account.data(), + false, + ); let mut caller_account = mock_caller_account.caller_account(); let callee_account = borrow_instruction_account!(invoke_context, 0); let mut data = b"foo".to_vec(); - caller_account.data = &mut data; + caller_account.serialized_data = &mut data; - update_callee_account(&invoke_context, &caller_account, callee_account).unwrap(); + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + false, + ) + .unwrap(); let callee_account = borrow_instruction_account!(invoke_context, 0); - assert_eq!(callee_account.get_data(), caller_account.data); + assert_eq!(callee_account.get_data(), caller_account.serialized_data); + + // close the account + let mut data = Vec::new(); + caller_account.serialized_data = &mut data; + *caller_account.ref_to_len_in_vm = 0; + let mut owner = system_program::id(); + caller_account.owner = &mut owner; + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + false, + ) + .unwrap(); + let callee_account = borrow_instruction_account!(invoke_context, 0); + assert_eq!(callee_account.get_data(), b""); + } + + #[test] + fn test_update_callee_account_data_readonly() { + let transaction_accounts = + transaction_with_one_readonly_instruction_account(b"foobar".to_vec()); + let account = transaction_accounts[1].1.clone(); + + mock_invoke_context!( + invoke_context, + transaction_context, + b"instruction data", + transaction_accounts, + &[0], + &[1] + ); + + let mut mock_caller_account = MockCallerAccount::new( + 1234, + *account.owner(), + 0xFFFFFFFF00000000, + account.data(), + false, + ); + + let mut caller_account = mock_caller_account.caller_account(); + + let callee_account = borrow_instruction_account!(invoke_context, 0); + + caller_account.serialized_data[0] = b'b'; + assert!(matches!( + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + false, + ), + Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ExternalAccountDataModified + )); + + // without direct mapping + let mut data = b"foobarbaz".to_vec(); + *caller_account.ref_to_len_in_vm = data.len() as u64; + caller_account.serialized_data = &mut data; + + let callee_account = borrow_instruction_account!(invoke_context, 0); + assert!(matches!( + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + false, + ), + Err(error) if error.downcast_ref::().unwrap() == &InstructionError::AccountDataSizeChanged + )); + + // with direct mapping + let mut data = b"baz".to_vec(); + *caller_account.ref_to_len_in_vm = 9; + caller_account.serialized_data = &mut data; + + let callee_account = borrow_instruction_account!(invoke_context, 0); + assert!(matches!( + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + true, + ), + Err(error) if error.downcast_ref::().unwrap() == &InstructionError::AccountDataSizeChanged + )); + } + + #[test] + fn test_update_callee_account_data_direct_mapping() { + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foobar".to_vec()); + let account = transaction_accounts[1].1.clone(); + + mock_invoke_context!( + invoke_context, + transaction_context, + b"instruction data", + transaction_accounts, + &[0], + &[1] + ); + + let mut mock_caller_account = MockCallerAccount::new( + 1234, + *account.owner(), + 0xFFFFFFFF00000000, + account.data(), + true, + ); + + let mut caller_account = mock_caller_account.caller_account(); + + let mut callee_account = borrow_instruction_account!(invoke_context, 0); + + // this is done when a writable account is mapped, and it ensures + // through make_data_mut() that the account is made writable and resized + // with enough padding to hold the realloc padding + callee_account.get_data_mut().unwrap(); + + let mut data = b"baz".to_vec(); + caller_account.serialized_data = &mut data; + + for (len, expected) in [ + (9, b"foobarbaz".to_vec()), // > original_data_len, copies from realloc region + (6, b"foobar".to_vec()), // == original_data_len, truncates + (3, b"foo".to_vec()), // < original_data_len, truncates + ] { + *caller_account.ref_to_len_in_vm = len as u64; + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + true, + ) + .unwrap(); + callee_account = borrow_instruction_account!(invoke_context, 0); + assert_eq!(callee_account.get_data(), expected); + } + + // close the account + let mut data = Vec::new(); + caller_account.serialized_data = &mut data; + *caller_account.ref_to_len_in_vm = 0; + let mut owner = system_program::id(); + caller_account.owner = &mut owner; + update_callee_account( + &invoke_context, + false, + &caller_account, + callee_account, + true, + ) + .unwrap(); + callee_account = borrow_instruction_account!(invoke_context, 0); + assert_eq!(callee_account.get_data(), b""); } #[test] fn test_translate_accounts_rust() { - let transaction_accounts = transaction_with_one_instruction_account(b"foobar".to_vec()); + let transaction_accounts = + transaction_with_one_writable_instruction_account(b"foobar".to_vec()); let account = transaction_accounts[1].1.clone(); let key = transaction_accounts[1].0; let original_data_len = account.data().len(); @@ -1538,6 +2170,7 @@ mod tests { &[0], vm_addr, 1, + false, &mut memory_mapping, &mut invoke_context, ) @@ -1545,12 +2178,11 @@ mod tests { assert_eq!(accounts.len(), 2); assert!(accounts[0].1.is_none()); let caller_account = accounts[1].1.as_ref().unwrap(); - assert_eq!(caller_account.data, account.data()); + assert_eq!(caller_account.serialized_data, account.data()); assert_eq!(caller_account.original_data_len, original_data_len); } pub type TestTransactionAccount = (Pubkey, AccountSharedData, bool); - struct MockCallerAccount { lamports: u64, owner: Pubkey, @@ -1561,18 +2193,54 @@ mod tests { } impl MockCallerAccount { - fn new(lamports: u64, owner: Pubkey, vm_addr: u64, data: &[u8]) -> MockCallerAccount { - // write [len][data] into a vec so we can check that they get - // properly updated by update_caller_account() - let mut d = vec![0; mem::size_of::() + data.len() + MAX_PERMITTED_DATA_INCREASE]; + fn new( + lamports: u64, + owner: Pubkey, + vm_addr: u64, + data: &[u8], + direct_mapping: bool, + ) -> MockCallerAccount { + let mut regions = vec![]; + + let mut d = vec![ + 0; + mem::size_of::() + + if direct_mapping { 0 } else { data.len() } + + MAX_PERMITTED_DATA_INCREASE + ]; + // always write the [len] part even when direct mapping unsafe { ptr::write_unaligned::(d.as_mut_ptr().cast(), data.len() as u64) }; - d[mem::size_of::()..][..data.len()].copy_from_slice(data); - // the memory region must include the realloc data - let regions = vec![MemoryRegion::new_writable(d.as_mut_slice(), vm_addr)]; + // write the account data when not direct mapping + if !direct_mapping { + d[mem::size_of::()..][..data.len()].copy_from_slice(data); + } - // caller_account.data must have the actual data length - d.truncate(mem::size_of::() + data.len()); + // create a region for [len][data+realloc if !direct_mapping] + let mut region_addr = vm_addr; + let region_len = mem::size_of::() + + if direct_mapping { + 0 + } else { + data.len() + MAX_PERMITTED_DATA_INCREASE + }; + regions.push(MemoryRegion::new_writable(&mut d[..region_len], vm_addr)); + region_addr += region_len as u64; + + if direct_mapping { + // create a region for the directly mapped data + regions.push(MemoryRegion::new_readonly(data, region_addr)); + region_addr += data.len() as u64; + + // create a region for the realloc padding + regions.push(MemoryRegion::new_writable( + &mut d[mem::size_of::()..], + region_addr, + )); + } else { + // caller_account.serialized_data must have the actual data length + d.truncate(mem::size_of::() + data.len()); + } MockCallerAccount { lamports, @@ -1599,8 +2267,8 @@ mod tests { CallerAccount { lamports: &mut self.lamports, owner: &mut self.owner, - original_data_len: data.len(), - data, + original_data_len: self.len as usize, + serialized_data: data, vm_data_addr: self.vm_addr + mem::size_of::() as u64, ref_to_len_in_vm: &mut self.len, serialized_len_ptr: std::ptr::null_mut(), @@ -1610,7 +2278,9 @@ mod tests { } } - fn transaction_with_one_instruction_account(data: Vec) -> Vec { + fn transaction_with_one_writable_instruction_account( + data: Vec, + ) -> Vec { let program_id = Pubkey::new_unique(); let account = AccountSharedData::from(Account { lamports: 1, @@ -1635,6 +2305,34 @@ mod tests { ] } + fn transaction_with_one_readonly_instruction_account( + data: Vec, + ) -> Vec { + let program_id = Pubkey::new_unique(); + let account_owner = Pubkey::new_unique(); + let account = AccountSharedData::from(Account { + lamports: 1, + data, + owner: account_owner, + executable: false, + rent_epoch: 100, + }); + vec![ + ( + program_id, + AccountSharedData::from(Account { + lamports: 0, + data: vec![], + owner: bpf_loader::id(), + executable: true, + rent_epoch: 0, + }), + false, + ), + (Pubkey::new_unique(), account, true), + ] + } + struct MockInstruction { program_id: Pubkey, accounts: Vec, diff --git a/programs/bpf_loader/src/syscalls/mem_ops.rs b/programs/bpf_loader/src/syscalls/mem_ops.rs index d501311461339f..2ecd9a9ad4f77e 100644 --- a/programs/bpf_loader/src/syscalls/mem_ops.rs +++ b/programs/bpf_loader/src/syscalls/mem_ops.rs @@ -1,4 +1,9 @@ -use {super::*, crate::declare_syscall}; +use { + super::*, + crate::declare_syscall, + solana_rbpf::{error::EbpfError, memory_region::MemoryRegion}, + std::slice, +}; fn mem_op_consume(invoke_context: &mut InvokeContext, n: u64) -> Result<(), Error> { let compute_budget = invoke_context.get_compute_budget(); @@ -26,32 +31,8 @@ declare_syscall!( return Err(SyscallError::CopyOverlapping.into()); } - let dst_ptr = translate_slice_mut::( - memory_mapping, - dst_addr, - n, - invoke_context.get_check_aligned(), - invoke_context.get_check_size(), - )? - .as_mut_ptr(); - let src_ptr = translate_slice::( - memory_mapping, - src_addr, - n, - invoke_context.get_check_aligned(), - invoke_context.get_check_size(), - )? - .as_ptr(); - if !is_nonoverlapping(src_ptr as usize, n as usize, dst_ptr as usize, n as usize) { - unsafe { - std::ptr::copy(src_ptr, dst_ptr, n as usize); - } - } else { - unsafe { - std::ptr::copy_nonoverlapping(src_ptr, dst_ptr, n as usize); - } - } - Ok(0) + // host addresses can overlap so we always invoke memmove + memmove(invoke_context, dst_addr, src_addr, n, memory_mapping) } ); @@ -69,24 +50,7 @@ declare_syscall!( ) -> Result { mem_op_consume(invoke_context, n)?; - let dst = translate_slice_mut::( - memory_mapping, - dst_addr, - n, - invoke_context.get_check_aligned(), - invoke_context.get_check_size(), - )?; - let src = translate_slice::( - memory_mapping, - src_addr, - n, - invoke_context.get_check_aligned(), - invoke_context.get_check_size(), - )?; - unsafe { - std::ptr::copy(src.as_ptr(), dst.as_mut_ptr(), n as usize); - } - Ok(0) + memmove(invoke_context, dst_addr, src_addr, n, memory_mapping) } ); @@ -104,36 +68,46 @@ declare_syscall!( ) -> Result { mem_op_consume(invoke_context, n)?; - let s1 = translate_slice::( - memory_mapping, - s1_addr, - n, - invoke_context.get_check_aligned(), - invoke_context.get_check_size(), - )?; - let s2 = translate_slice::( - memory_mapping, - s2_addr, - n, - invoke_context.get_check_aligned(), - invoke_context.get_check_size(), - )?; - let cmp_result = translate_type_mut::( - memory_mapping, - cmp_result_addr, - invoke_context.get_check_aligned(), - )?; - let mut i = 0; - while i < n as usize { - let a = *s1.get(i).ok_or(SyscallError::InvalidLength)?; - let b = *s2.get(i).ok_or(SyscallError::InvalidLength)?; - if a != b { - *cmp_result = (a as i32).saturating_sub(b as i32); - return Ok(0); - }; - i = i.saturating_add(1); + if invoke_context + .feature_set + .is_active(&feature_set::bpf_account_data_direct_mapping::id()) + { + let cmp_result = translate_type_mut::( + memory_mapping, + cmp_result_addr, + invoke_context.get_check_aligned(), + )?; + *cmp_result = memcmp_non_contiguous(s1_addr, s2_addr, n, memory_mapping)?; + } else { + let s1 = translate_slice::( + memory_mapping, + s1_addr, + n, + invoke_context.get_check_aligned(), + invoke_context.get_check_size(), + )?; + let s2 = translate_slice::( + memory_mapping, + s2_addr, + n, + invoke_context.get_check_aligned(), + invoke_context.get_check_size(), + )?; + let cmp_result = translate_type_mut::( + memory_mapping, + cmp_result_addr, + invoke_context.get_check_aligned(), + )?; + + debug_assert_eq!(s1.len(), n as usize); + debug_assert_eq!(s2.len(), n as usize); + // Safety: + // memcmp is marked unsafe since it assumes that the inputs are at least + // `n` bytes long. `s1` and `s2` are guaranteed to be exactly `n` bytes + // long because `translate_slice` would have failed otherwise. + *cmp_result = unsafe { memcmp(s1, s2, n as usize) }; } - *cmp_result = 0; + Ok(0) } ); @@ -143,7 +117,7 @@ declare_syscall!( SyscallMemset, fn inner_call( invoke_context: &mut InvokeContext, - s_addr: u64, + dst_addr: u64, c: u64, n: u64, _arg4: u64, @@ -152,16 +126,793 @@ declare_syscall!( ) -> Result { mem_op_consume(invoke_context, n)?; - let s = translate_slice_mut::( + if invoke_context + .feature_set + .is_active(&feature_set::bpf_account_data_direct_mapping::id()) + { + memset_non_contiguous(dst_addr, c as u8, n, memory_mapping) + } else { + let s = translate_slice_mut::( + memory_mapping, + dst_addr, + n, + invoke_context.get_check_aligned(), + invoke_context.get_check_size(), + )?; + s.fill(c as u8); + Ok(0) + } + } +); + +fn memmove( + invoke_context: &mut InvokeContext, + dst_addr: u64, + src_addr: u64, + n: u64, + memory_mapping: &MemoryMapping, +) -> Result { + if invoke_context + .feature_set + .is_active(&feature_set::bpf_account_data_direct_mapping::id()) + { + memmove_non_contiguous(dst_addr, src_addr, n, memory_mapping) + } else { + let dst_ptr = translate_slice_mut::( memory_mapping, - s_addr, + dst_addr, n, invoke_context.get_check_aligned(), invoke_context.get_check_size(), - )?; - for val in s.iter_mut().take(n as usize) { - *val = c as u8; - } + )? + .as_mut_ptr(); + let src_ptr = translate_slice::( + memory_mapping, + src_addr, + n, + invoke_context.get_check_aligned(), + invoke_context.get_check_size(), + )? + .as_ptr(); + + unsafe { std::ptr::copy(src_ptr, dst_ptr, n as usize) }; Ok(0) } -); +} + +fn memmove_non_contiguous( + dst_addr: u64, + src_addr: u64, + n: u64, + memory_mapping: &MemoryMapping, +) -> Result { + let reverse = dst_addr.wrapping_sub(src_addr) < n; + iter_memory_pair_chunks( + AccessType::Load, + src_addr, + AccessType::Store, + dst_addr, + n, + memory_mapping, + reverse, + |src_host_addr, dst_host_addr, chunk_len| { + unsafe { + std::ptr::copy( + src_host_addr as *const u8, + dst_host_addr as *mut u8, + chunk_len, + ) + }; + Ok(0) + }, + ) +} + +// Marked unsafe since it assumes that the slices are at least `n` bytes long. +unsafe fn memcmp(s1: &[u8], s2: &[u8], n: usize) -> i32 { + for i in 0..n { + let a = *s1.get_unchecked(i); + let b = *s2.get_unchecked(i); + if a != b { + return (a as i32).saturating_sub(b as i32); + }; + } + + 0 +} + +fn memcmp_non_contiguous( + src_addr: u64, + dst_addr: u64, + n: u64, + memory_mapping: &MemoryMapping, +) -> Result { + match iter_memory_pair_chunks( + AccessType::Load, + src_addr, + AccessType::Load, + dst_addr, + n, + memory_mapping, + false, + |s1_addr, s2_addr, chunk_len| { + let res = unsafe { + let s1 = slice::from_raw_parts(s1_addr as *const u8, chunk_len); + let s2 = slice::from_raw_parts(s2_addr as *const u8, chunk_len); + // Safety: + // memcmp is marked unsafe since it assumes that s1 and s2 are exactly chunk_len + // long. The whole point of iter_memory_pair_chunks is to find same length chunks + // across two memory regions. + memcmp(s1, s2, chunk_len) + }; + if res != 0 { + return Err(MemcmpError::Diff(res).into()); + } + Ok(0) + }, + ) { + Ok(res) => Ok(res), + Err(error) => match error.downcast_ref() { + Some(MemcmpError::Diff(diff)) => Ok(*diff), + _ => Err(error), + }, + } +} + +#[derive(Debug)] +enum MemcmpError { + Diff(i32), +} + +impl std::fmt::Display for MemcmpError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + MemcmpError::Diff(diff) => write!(f, "memcmp diff: {diff}"), + } + } +} + +impl std::error::Error for MemcmpError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + MemcmpError::Diff(_) => None, + } + } +} + +fn memset_non_contiguous( + dst_addr: u64, + c: u8, + n: u64, + memory_mapping: &MemoryMapping, +) -> Result { + let dst_chunk_iter = MemoryChunkIterator::new(memory_mapping, AccessType::Store, dst_addr, n)?; + for item in dst_chunk_iter { + let (dst_region, dst_vm_addr, dst_len) = item?; + let dst_host_addr = Result::from(dst_region.vm_to_host(dst_vm_addr, dst_len as u64))?; + unsafe { slice::from_raw_parts_mut(dst_host_addr as *mut u8, dst_len).fill(c) } + } + + Ok(0) +} + +fn iter_memory_pair_chunks( + src_access: AccessType, + src_addr: u64, + dst_access: AccessType, + mut dst_addr: u64, + n: u64, + memory_mapping: &MemoryMapping, + reverse: bool, + mut fun: F, +) -> Result +where + T: Default, + F: FnMut(*const u8, *const u8, usize) -> Result, +{ + let mut src_chunk_iter = MemoryChunkIterator::new(memory_mapping, src_access, src_addr, n) + .map_err(EbpfError::from)?; + loop { + // iterate source chunks + let (src_region, src_vm_addr, mut src_len) = match if reverse { + src_chunk_iter.next_back() + } else { + src_chunk_iter.next() + } { + Some(item) => item?, + None => break, + }; + + let mut src_host_addr = Result::from(src_region.vm_to_host(src_vm_addr, src_len as u64))?; + let mut dst_chunk_iter = MemoryChunkIterator::new(memory_mapping, dst_access, dst_addr, n) + .map_err(EbpfError::from)?; + // iterate over destination chunks until this source chunk has been completely copied + while src_len > 0 { + loop { + let (dst_region, dst_vm_addr, dst_len) = match if reverse { + dst_chunk_iter.next_back() + } else { + dst_chunk_iter.next() + } { + Some(item) => item?, + None => break, + }; + let dst_host_addr = + Result::from(dst_region.vm_to_host(dst_vm_addr, dst_len as u64))?; + let chunk_len = src_len.min(dst_len); + fun( + src_host_addr as *const u8, + dst_host_addr as *const u8, + chunk_len, + )?; + src_len = src_len.saturating_sub(chunk_len); + if reverse { + dst_addr = dst_addr.saturating_sub(chunk_len as u64); + } else { + dst_addr = dst_addr.saturating_add(chunk_len as u64); + } + if src_len == 0 { + break; + } + src_host_addr = src_host_addr.saturating_add(chunk_len as u64); + } + } + } + + Ok(T::default()) +} + +struct MemoryChunkIterator<'a> { + memory_mapping: &'a MemoryMapping<'a>, + access_type: AccessType, + initial_vm_addr: u64, + vm_addr_start: u64, + // exclusive end index (start + len, so one past the last valid address) + vm_addr_end: u64, + len: u64, +} + +impl<'a> MemoryChunkIterator<'a> { + fn new( + memory_mapping: &'a MemoryMapping, + access_type: AccessType, + vm_addr: u64, + len: u64, + ) -> Result, EbpfError> { + let vm_addr_end = vm_addr.checked_add(len).ok_or(EbpfError::AccessViolation( + 0, + access_type, + vm_addr, + len, + "unknown", + ))?; + Ok(MemoryChunkIterator { + memory_mapping, + access_type, + initial_vm_addr: vm_addr, + len, + vm_addr_start: vm_addr, + vm_addr_end, + }) + } + + fn region(&mut self, vm_addr: u64) -> Result<&'a MemoryRegion, Error> { + match self.memory_mapping.region(self.access_type, vm_addr) { + Ok(region) => Ok(region), + Err(error) => match error.downcast_ref() { + Some(EbpfError::AccessViolation(pc, access_type, _vm_addr, _len, name)) => { + Err(Box::new(EbpfError::AccessViolation( + *pc, + *access_type, + self.initial_vm_addr, + self.len, + name, + ))) + } + Some(EbpfError::StackAccessViolation(pc, access_type, _vm_addr, _len, frame)) => { + Err(Box::new(EbpfError::StackAccessViolation( + *pc, + *access_type, + self.initial_vm_addr, + self.len, + *frame, + ))) + } + _ => Err(error), + }, + } + } +} + +impl<'a> Iterator for MemoryChunkIterator<'a> { + type Item = Result<(&'a MemoryRegion, u64, usize), Error>; + + fn next(&mut self) -> Option { + if self.vm_addr_start == self.vm_addr_end { + return None; + } + + let region = match self.region(self.vm_addr_start) { + Ok(region) => region, + Err(e) => { + self.vm_addr_start = self.vm_addr_end; + return Some(Err(e)); + } + }; + + let vm_addr = self.vm_addr_start; + + let chunk_len = if region.vm_addr_end <= self.vm_addr_end { + // consume the whole region + let len = region.vm_addr_end.saturating_sub(self.vm_addr_start); + self.vm_addr_start = region.vm_addr_end; + len + } else { + // consume part of the region + let len = self.vm_addr_end.saturating_sub(self.vm_addr_start); + self.vm_addr_start = self.vm_addr_end; + len + }; + + Some(Ok((region, vm_addr, chunk_len as usize))) + } +} + +impl<'a> DoubleEndedIterator for MemoryChunkIterator<'a> { + fn next_back(&mut self) -> Option { + if self.vm_addr_start == self.vm_addr_end { + return None; + } + + let region = match self.region(self.vm_addr_end.saturating_sub(1)) { + Ok(region) => region, + Err(e) => { + self.vm_addr_start = self.vm_addr_end; + return Some(Err(e)); + } + }; + + let chunk_len = if region.vm_addr >= self.vm_addr_start { + // consume the whole region + let len = self.vm_addr_end.saturating_sub(region.vm_addr); + self.vm_addr_end = region.vm_addr; + len + } else { + // consume part of the region + let len = self.vm_addr_end.saturating_sub(self.vm_addr_start); + self.vm_addr_end = self.vm_addr_start; + len + }; + + Some(Ok((region, self.vm_addr_end, chunk_len as usize))) + } +} + +#[cfg(test)] +#[allow(clippy::indexing_slicing)] +mod tests { + use {super::*, solana_rbpf::ebpf::MM_PROGRAM_START}; + + fn to_chunk_vec<'a>( + iter: impl Iterator>, + ) -> Vec<(u64, usize)> { + iter.flat_map(|res| res.map(|(_, vm_addr, len)| (vm_addr, len))) + .collect::>() + } + + #[test] + #[should_panic(expected = "AccessViolation")] + fn test_memory_chunk_iterator_no_regions() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let memory_mapping = MemoryMapping::new(vec![], &config).unwrap(); + + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, 0, 1).unwrap(); + src_chunk_iter.next().unwrap().unwrap(); + } + + #[test] + #[should_panic(expected = "AccessViolation")] + fn test_memory_chunk_iterator_new_out_of_bounds_upper() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let memory_mapping = MemoryMapping::new(vec![], &config).unwrap(); + + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, u64::MAX, 1).unwrap(); + src_chunk_iter.next().unwrap().unwrap(); + } + + #[test] + fn test_memory_chunk_iterator_out_of_bounds() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = vec![0xFF; 42]; + let memory_mapping = MemoryMapping::new( + vec![MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START)], + &config, + ) + .unwrap(); + + // check oob at the lower bound on the first next() + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, MM_PROGRAM_START - 1, 42) + .unwrap(); + assert!(matches!( + src_chunk_iter.next().unwrap().unwrap_err().downcast_ref().unwrap(), + EbpfError::AccessViolation(0, AccessType::Load, addr, 42, "unknown") if *addr == MM_PROGRAM_START - 1 + )); + + // check oob at the upper bound. Since the memory mapping isn't empty, + // this always happens on the second next(). + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, MM_PROGRAM_START, 43) + .unwrap(); + assert!(src_chunk_iter.next().unwrap().is_ok()); + assert!(matches!( + src_chunk_iter.next().unwrap().unwrap_err().downcast_ref().unwrap(), + EbpfError::AccessViolation(0, AccessType::Load, addr, 43, "program") if *addr == MM_PROGRAM_START + )); + + // check oob at the upper bound on the first next_back() + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, MM_PROGRAM_START, 43) + .unwrap() + .rev(); + assert!(matches!( + src_chunk_iter.next().unwrap().unwrap_err().downcast_ref().unwrap(), + EbpfError::AccessViolation(0, AccessType::Load, addr, 43, "program") if *addr == MM_PROGRAM_START + )); + + // check oob at the upper bound on the 2nd next_back() + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, MM_PROGRAM_START - 1, 43) + .unwrap() + .rev(); + assert!(src_chunk_iter.next().unwrap().is_ok()); + assert!(matches!( + src_chunk_iter.next().unwrap().unwrap_err().downcast_ref().unwrap(), + EbpfError::AccessViolation(0, AccessType::Load, addr, 43, "unknown") if *addr == MM_PROGRAM_START - 1 + )); + } + + #[test] + fn test_memory_chunk_iterator_one() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = vec![0xFF; 42]; + let memory_mapping = MemoryMapping::new( + vec![MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START)], + &config, + ) + .unwrap(); + + // check lower bound + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, MM_PROGRAM_START - 1, 1) + .unwrap(); + assert!(src_chunk_iter.next().unwrap().is_err()); + + // check upper bound + let mut src_chunk_iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, MM_PROGRAM_START + 42, 1) + .unwrap(); + assert!(src_chunk_iter.next().unwrap().is_err()); + + for (vm_addr, len) in [ + (MM_PROGRAM_START, 0), + (MM_PROGRAM_START + 42, 0), + (MM_PROGRAM_START, 1), + (MM_PROGRAM_START, 42), + (MM_PROGRAM_START + 41, 1), + ] { + for rev in [true, false] { + let iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, vm_addr, len) + .unwrap(); + let res = if rev { + to_chunk_vec(iter.rev()) + } else { + to_chunk_vec(iter) + }; + if len == 0 { + assert_eq!(res, &[]); + } else { + assert_eq!(res, &[(vm_addr, len as usize)]); + } + } + } + } + + #[test] + fn test_memory_chunk_iterator_two() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = vec![0x11; 8]; + let mem2 = vec![0x22; 4]; + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START), + MemoryRegion::new_readonly(&mem2, MM_PROGRAM_START + 8), + ], + &config, + ) + .unwrap(); + + for (vm_addr, len, mut expected) in [ + (MM_PROGRAM_START, 8, vec![(MM_PROGRAM_START, 8)]), + ( + MM_PROGRAM_START + 7, + 2, + vec![(MM_PROGRAM_START + 7, 1), (MM_PROGRAM_START + 8, 1)], + ), + (MM_PROGRAM_START + 8, 4, vec![(MM_PROGRAM_START + 8, 4)]), + ] { + for rev in [false, true] { + let iter = + MemoryChunkIterator::new(&memory_mapping, AccessType::Load, vm_addr, len) + .unwrap(); + let res = if rev { + expected.reverse(); + to_chunk_vec(iter.rev()) + } else { + to_chunk_vec(iter) + }; + + assert_eq!(res, expected); + } + } + } + + #[test] + fn test_iter_memory_pair_chunks_short() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = vec![0x11; 8]; + let mem2 = vec![0x22; 4]; + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START), + MemoryRegion::new_readonly(&mem2, MM_PROGRAM_START + 8), + ], + &config, + ) + .unwrap(); + + // dst is shorter than src + assert!(matches!( + iter_memory_pair_chunks( + AccessType::Load, + MM_PROGRAM_START, + AccessType::Load, + MM_PROGRAM_START + 8, + 8, + &memory_mapping, + false, + |_src, _dst, _len| Ok::<_, Error>(0), + ).unwrap_err().downcast_ref().unwrap(), + EbpfError::AccessViolation(0, AccessType::Load, addr, 8, "program") if *addr == MM_PROGRAM_START + 8 + )); + + // src is shorter than dst + assert!(matches!( + iter_memory_pair_chunks( + AccessType::Load, + MM_PROGRAM_START + 10, + AccessType::Load, + MM_PROGRAM_START + 2, + 3, + &memory_mapping, + false, + |_src, _dst, _len| Ok::<_, Error>(0), + ).unwrap_err().downcast_ref().unwrap(), + EbpfError::AccessViolation(0, AccessType::Load, addr, 3, "program") if *addr == MM_PROGRAM_START + 10 + )); + } + + #[test] + #[should_panic(expected = "AccessViolation(0, Store, 4294967296, 4")] + fn test_memmove_non_contiguous_readonly() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = vec![0x11; 8]; + let mem2 = vec![0x22; 4]; + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START), + MemoryRegion::new_readonly(&mem2, MM_PROGRAM_START + 8), + ], + &config, + ) + .unwrap(); + + memmove_non_contiguous(MM_PROGRAM_START, MM_PROGRAM_START + 8, 4, &memory_mapping).unwrap(); + } + + #[test] + fn test_overlapping_memmove_non_contiguous_right() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = vec![0x11; 1]; + let mut mem2 = vec![0x22; 2]; + let mut mem3 = vec![0x33; 3]; + let mut mem4 = vec![0x44; 4]; + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START), + MemoryRegion::new_writable(&mut mem2, MM_PROGRAM_START + 1), + MemoryRegion::new_writable(&mut mem3, MM_PROGRAM_START + 3), + MemoryRegion::new_writable(&mut mem4, MM_PROGRAM_START + 6), + ], + &config, + ) + .unwrap(); + + // overlapping memmove right - the implementation will copy backwards + assert_eq!( + memmove_non_contiguous(MM_PROGRAM_START + 1, MM_PROGRAM_START, 7, &memory_mapping) + .unwrap(), + 0 + ); + assert_eq!(&mem1, &[0x11]); + assert_eq!(&mem2, &[0x11, 0x22]); + assert_eq!(&mem3, &[0x22, 0x33, 0x33]); + assert_eq!(&mem4, &[0x33, 0x44, 0x44, 0x44]); + } + + #[test] + fn test_overlapping_memmove_non_contiguous_left() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mut mem1 = vec![0x11; 1]; + let mut mem2 = vec![0x22; 2]; + let mut mem3 = vec![0x33; 3]; + let mut mem4 = vec![0x44; 4]; + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_writable(&mut mem1, MM_PROGRAM_START), + MemoryRegion::new_writable(&mut mem2, MM_PROGRAM_START + 1), + MemoryRegion::new_writable(&mut mem3, MM_PROGRAM_START + 3), + MemoryRegion::new_writable(&mut mem4, MM_PROGRAM_START + 6), + ], + &config, + ) + .unwrap(); + + // overlapping memmove left - the implementation will copy forward + assert_eq!( + memmove_non_contiguous(MM_PROGRAM_START, MM_PROGRAM_START + 1, 7, &memory_mapping) + .unwrap(), + 0 + ); + assert_eq!(&mem1, &[0x22]); + assert_eq!(&mem2, &[0x22, 0x33]); + assert_eq!(&mem3, &[0x33, 0x33, 0x44]); + assert_eq!(&mem4, &[0x44, 0x44, 0x44, 0x44]); + } + + #[test] + #[should_panic(expected = "AccessViolation(0, Store, 4294967296, 9")] + fn test_memset_non_contiguous_readonly() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mut mem1 = vec![0x11; 8]; + let mem2 = vec![0x22; 4]; + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_writable(&mut mem1, MM_PROGRAM_START), + MemoryRegion::new_readonly(&mem2, MM_PROGRAM_START + 8), + ], + &config, + ) + .unwrap(); + + assert_eq!( + memset_non_contiguous(MM_PROGRAM_START, 0x33, 9, &memory_mapping).unwrap(), + 0 + ); + } + + #[test] + fn test_memset_non_contiguous() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = vec![0x11; 1]; + let mut mem2 = vec![0x22; 2]; + let mut mem3 = vec![0x33; 3]; + let mut mem4 = vec![0x44; 4]; + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START), + MemoryRegion::new_writable(&mut mem2, MM_PROGRAM_START + 1), + MemoryRegion::new_writable(&mut mem3, MM_PROGRAM_START + 3), + MemoryRegion::new_writable(&mut mem4, MM_PROGRAM_START + 6), + ], + &config, + ) + .unwrap(); + + assert_eq!( + memset_non_contiguous(MM_PROGRAM_START + 1, 0x55, 7, &memory_mapping).unwrap(), + 0 + ); + assert_eq!(&mem1, &[0x11]); + assert_eq!(&mem2, &[0x55, 0x55]); + assert_eq!(&mem3, &[0x55, 0x55, 0x55]); + assert_eq!(&mem4, &[0x55, 0x55, 0x44, 0x44]); + } + + #[test] + fn test_memcmp_non_contiguous() { + let config = Config { + aligned_memory_mapping: false, + ..Config::default() + }; + let mem1 = b"foo".to_vec(); + let mem2 = b"barbad".to_vec(); + let mem3 = b"foobarbad".to_vec(); + let memory_mapping = MemoryMapping::new( + vec![ + MemoryRegion::new_readonly(&mem1, MM_PROGRAM_START), + MemoryRegion::new_readonly(&mem2, MM_PROGRAM_START + 3), + MemoryRegion::new_readonly(&mem3, MM_PROGRAM_START + 9), + ], + &config, + ) + .unwrap(); + + // non contiguous src + assert_eq!( + memcmp_non_contiguous(MM_PROGRAM_START, MM_PROGRAM_START + 9, 9, &memory_mapping) + .unwrap(), + 0 + ); + + // non contiguous dst + assert_eq!( + memcmp_non_contiguous( + MM_PROGRAM_START + 10, + MM_PROGRAM_START + 1, + 8, + &memory_mapping + ) + .unwrap(), + 0 + ); + + // diff + assert_eq!( + memcmp_non_contiguous( + MM_PROGRAM_START + 1, + MM_PROGRAM_START + 11, + 5, + &memory_mapping + ) + .unwrap(), + unsafe { memcmp(b"oobar", b"obarb", 5) } + ); + } +} diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index e782bda37862e3..cbc7ab3c4fe66b 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -30,6 +30,7 @@ use { big_mod_exp::{big_mod_exp, BigModExpParams}, blake3, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE, SUCCESS}, + feature_set::bpf_account_data_direct_mapping, feature_set::FeatureSet, feature_set::{ self, blake3_syscall_enabled, check_syscall_outputs_do_not_overlap, @@ -174,7 +175,7 @@ pub fn create_loader<'a>( enable_elf_vaddr: false, reject_rodata_stack_overlap: false, new_elf_parser: feature_set.is_active(&switch_to_new_elf_parser::id()), - aligned_memory_mapping: true, + aligned_memory_mapping: !feature_set.is_active(&bpf_account_data_direct_mapping::id()), // Warning, do not use `Config::default()` so that configuration here is explicit. }; diff --git a/programs/loader-v3/src/lib.rs b/programs/loader-v3/src/lib.rs index 4e1042a3e05d0b..07d6d1403c87e3 100644 --- a/programs/loader-v3/src/lib.rs +++ b/programs/loader-v3/src/lib.rs @@ -691,7 +691,7 @@ mod tests { authority_address, }) .unwrap(); - program_account.data_mut()[loader_v3::LoaderV3State::program_data_offset()..] + program_account.data_as_mut_slice()[loader_v3::LoaderV3State::program_data_offset()..] .copy_from_slice(&elf_bytes); program_account } diff --git a/programs/sbf/benches/bpf_loader.rs b/programs/sbf/benches/bpf_loader.rs index a2ba7df20fa841..82ac884767baec 100644 --- a/programs/sbf/benches/bpf_loader.rs +++ b/programs/sbf/benches/bpf_loader.rs @@ -3,7 +3,10 @@ #![allow(clippy::uninlined_format_args)] #![allow(clippy::integer_arithmetic)] -use {solana_rbpf::memory_region::MemoryState, std::slice}; +use { + solana_rbpf::memory_region::MemoryState, + solana_sdk::feature_set::bpf_account_data_direct_mapping, std::slice, +}; extern crate test; @@ -66,7 +69,7 @@ macro_rules! with_mock_invoke_context { index_in_caller: 2, index_in_callee: 0, is_signer: false, - is_writable: false, + is_writable: true, }]; solana_program_runtime::with_mock_invoke_context!( $invoke_context, @@ -223,6 +226,9 @@ fn bench_create_vm(bencher: &mut Bencher) { const BUDGET: u64 = 200_000; invoke_context.mock_set_remaining(BUDGET); + let direct_mapping = invoke_context + .feature_set + .is_active(&bpf_account_data_direct_mapping::id()); let loader = create_loader( &invoke_context.feature_set, &ComputeBudget::default(), @@ -244,7 +250,8 @@ fn bench_create_vm(bencher: &mut Bencher) { .transaction_context .get_current_instruction_context() .unwrap(), - true, // should_cap_ix_accounts + true, // should_cap_ix_accounts + !direct_mapping, // copy_account_data ) .unwrap(); @@ -267,6 +274,10 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) { const BUDGET: u64 = 200_000; invoke_context.mock_set_remaining(BUDGET); + let direct_mapping = invoke_context + .feature_set + .is_active(&bpf_account_data_direct_mapping::id()); + // Serialize account data let (_serialized, regions, account_lengths) = serialize_parameters( invoke_context.transaction_context, @@ -274,7 +285,8 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) { .transaction_context .get_current_instruction_context() .unwrap(), - true, // should_cap_ix_accounts + true, // should_cap_ix_accounts + !direct_mapping, // copy_account_data ) .unwrap(); diff --git a/programs/sbf/rust/realloc/src/instructions.rs b/programs/sbf/rust/realloc/src/instructions.rs index 75e5cfe3db8d83..831a69029ed166 100644 --- a/programs/sbf/rust/realloc/src/instructions.rs +++ b/programs/sbf/rust/realloc/src/instructions.rs @@ -15,6 +15,7 @@ pub const DEALLOC_AND_ASSIGN_TO_CALLER: u8 = 7; pub const CHECK: u8 = 8; pub const ZERO_INIT: u8 = 9; pub const REALLOC_EXTEND_AND_UNDO: u8 = 10; +pub const EXTEND_AND_WRITE_U64: u8 = 11; pub fn realloc(program_id: &Pubkey, address: &Pubkey, size: usize, bump: &mut u8) -> Instruction { let mut instruction_data = vec![REALLOC, *bump]; @@ -88,3 +89,14 @@ pub fn realloc_extend_and_undo( vec![AccountMeta::new(*address, false)], ) } + +pub fn extend_and_write_u64(program_id: &Pubkey, address: &Pubkey, value: u64) -> Instruction { + let mut instruction_data = vec![EXTEND_AND_WRITE_U64]; + instruction_data.extend_from_slice(&value.to_le_bytes()); + + Instruction::new_with_bytes( + *program_id, + &instruction_data, + vec![AccountMeta::new(*address, false)], + ) +} diff --git a/programs/sbf/rust/realloc/src/processor.rs b/programs/sbf/rust/realloc/src/processor.rs index b0b008ad010e9e..782eb1cd505519 100644 --- a/programs/sbf/rust/realloc/src/processor.rs +++ b/programs/sbf/rust/realloc/src/processor.rs @@ -13,7 +13,7 @@ use { pubkey::Pubkey, system_instruction, system_program, }, - std::convert::TryInto, + std::{convert::TryInto, mem}, }; solana_program::entrypoint!(process_instruction); @@ -61,6 +61,42 @@ fn process_instruction( assert_eq!(new_len, account.data_len()); account.try_borrow_mut_data()?[pre_len..].fill(fill); } + // extend the account and do a 8-bytes write across the original account + // length and the realloc region + EXTEND_AND_WRITE_U64 => { + let pre_len = account.data_len(); + let new_len = mem::size_of::(); + assert!(pre_len < new_len); + account.realloc(new_len, false)?; + assert_eq!(new_len, account.data_len()); + + let (bytes, _) = instruction_data[1..].split_at(new_len); + let value = u64::from_le_bytes(bytes.try_into().unwrap()); + msg!( + "write {} to account {:p}", + value, + account.try_borrow_data().unwrap().as_ptr() + ); + // exercise memory write + unsafe { + *account + .try_borrow_mut_data() + .unwrap() + .as_mut_ptr() + .cast::() = value + }; + // exercise memory read + assert_eq!( + unsafe { + *account + .try_borrow_mut_data() + .unwrap() + .as_ptr() + .cast::() + }, + value + ); + } REALLOC_AND_ASSIGN => { msg!("realloc and assign"); account.realloc(MAX_PERMITTED_DATA_INCREASE, false)?; diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 7861b3d0a2935e..754bdf6d403040 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -39,7 +39,7 @@ use { clock::MAX_PROCESSING_AGE, compute_budget::ComputeBudgetInstruction, entrypoint::MAX_PERMITTED_DATA_INCREASE, - feature_set::FeatureSet, + feature_set::{self, FeatureSet}, fee::FeeStructure, loader_instruction, message::{v0::LoadedAddresses, SanitizedMessage}, @@ -1110,7 +1110,7 @@ fn test_program_sbf_invoke_sanity() { .map(|ix| &message.account_keys[ix.instruction.program_id_index as usize]) .cloned() .collect(); - assert_eq!(invoked_programs, vec![system_program::id()]); + assert_eq!(invoked_programs, vec![]); assert_eq!( result.unwrap_err(), TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete) @@ -2761,83 +2761,70 @@ fn test_program_sbf_realloc() { } = create_genesis_config(1_000_000_000_000); let mint_pubkey = mint_keypair.pubkey(); let signer = &[&mint_keypair]; + for direct_mapping in [false, true] { + let mut bank = Bank::new_for_tests(&genesis_config); + let feature_set = Arc::make_mut(&mut bank.feature_set); + // by default test banks have all features enabled, so we only need to + // disable when needed + if !direct_mapping { + feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + } + let bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); - let bank = Bank::new_for_tests(&genesis_config); - let bank = Arc::new(bank); - let bank_client = BankClient::new_shared(&bank); + let program_id = load_program( + &bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_realloc", + ); - let program_id = load_program( - &bank_client, - &bpf_loader::id(), - &mint_keypair, - "solana_sbf_rust_realloc", - ); + let mut bump = 0; + let keypair = Keypair::new(); + let pubkey = keypair.pubkey(); + let account = AccountSharedData::new(START_BALANCE, 5, &program_id); + bank.store_account(&pubkey, &account); - let mut bump = 0; - let keypair = Keypair::new(); - let pubkey = keypair.pubkey(); - let account = AccountSharedData::new(START_BALANCE, 5, &program_id); - bank.store_account(&pubkey, &account); + // Realloc RO account + let mut instruction = realloc(&program_id, &pubkey, 0, &mut bump); + instruction.accounts[0].is_writable = false; + assert_eq!( + bank_client + .send_and_confirm_message(signer, Message::new(&[instruction], Some(&mint_pubkey),),) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified) + ); - // Realloc RO account - let mut instruction = realloc(&program_id, &pubkey, 0, &mut bump); - instruction.accounts[0].is_writable = false; - assert_eq!( - bank_client - .send_and_confirm_message(signer, Message::new(&[instruction], Some(&mint_pubkey),),) - .unwrap_err() - .unwrap(), - TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified) - ); + // Realloc account to overflow + assert_eq!( + bank_client + .send_and_confirm_message( + signer, + Message::new( + &[realloc(&program_id, &pubkey, usize::MAX, &mut bump)], + Some(&mint_pubkey), + ), + ) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::InvalidRealloc) + ); - // Realloc account to overflow - assert_eq!( + // Realloc account to 0 bank_client .send_and_confirm_message( signer, Message::new( - &[realloc(&program_id, &pubkey, usize::MAX, &mut bump)], + &[realloc(&program_id, &pubkey, 0, &mut bump)], Some(&mint_pubkey), ), ) - .unwrap_err() - .unwrap(), - TransactionError::InstructionError(0, InstructionError::InvalidRealloc) - ); - - // Realloc account to 0 - bank_client - .send_and_confirm_message( - signer, - Message::new( - &[realloc(&program_id, &pubkey, 0, &mut bump)], - Some(&mint_pubkey), - ), - ) - .unwrap(); - let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); - assert_eq!(0, data.len()); - - // Realloc account to max then undo - bank_client - .send_and_confirm_message( - signer, - Message::new( - &[realloc_extend_and_undo( - &program_id, - &pubkey, - MAX_PERMITTED_DATA_INCREASE, - &mut bump, - )], - Some(&mint_pubkey), - ), - ) - .unwrap(); - let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); - assert_eq!(0, data.len()); + .unwrap(); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!(0, data.len()); - // Realloc account to max + 1 then undo - assert_eq!( + // Realloc account to max then undo bank_client .send_and_confirm_message( signer, @@ -2845,200 +2832,252 @@ fn test_program_sbf_realloc() { &[realloc_extend_and_undo( &program_id, &pubkey, - MAX_PERMITTED_DATA_INCREASE + 1, + MAX_PERMITTED_DATA_INCREASE, &mut bump, )], Some(&mint_pubkey), ), ) - .unwrap_err() - .unwrap(), - TransactionError::InstructionError(0, InstructionError::InvalidRealloc) - ); + .unwrap(); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!(0, data.len()); - // Realloc to max + 1 - assert_eq!( + // Realloc account to max + 1 then undo + assert_eq!( + bank_client + .send_and_confirm_message( + signer, + Message::new( + &[realloc_extend_and_undo( + &program_id, + &pubkey, + MAX_PERMITTED_DATA_INCREASE + 1, + &mut bump, + )], + Some(&mint_pubkey), + ), + ) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::InvalidRealloc) + ); + + // Realloc to max + 1 + assert_eq!( + bank_client + .send_and_confirm_message( + signer, + Message::new( + &[realloc( + &program_id, + &pubkey, + MAX_PERMITTED_DATA_INCREASE + 1, + &mut bump + )], + Some(&mint_pubkey), + ), + ) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::InvalidRealloc) + ); + + // Realloc to max length in max increase increments + for i in 0..MAX_PERMITTED_DATA_LENGTH as usize / MAX_PERMITTED_DATA_INCREASE { + let mut bump = i as u64; + bank_client + .send_and_confirm_message( + signer, + Message::new( + &[realloc_extend_and_fill( + &program_id, + &pubkey, + MAX_PERMITTED_DATA_INCREASE, + 1, + &mut bump, + )], + Some(&mint_pubkey), + ), + ) + .unwrap(); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!((i + 1) * MAX_PERMITTED_DATA_INCREASE, data.len()); + } + for i in 0..data.len() { + assert_eq!(data[i], 1); + } + + // and one more time should fail + assert_eq!( + bank_client + .send_and_confirm_message( + signer, + Message::new( + &[realloc_extend( + &program_id, + &pubkey, + MAX_PERMITTED_DATA_INCREASE, + &mut bump + )], + Some(&mint_pubkey), + ) + ) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::InvalidRealloc) + ); + + // Realloc to 6 bytes bank_client .send_and_confirm_message( signer, Message::new( - &[realloc( - &program_id, - &pubkey, - MAX_PERMITTED_DATA_INCREASE + 1, - &mut bump - )], + &[realloc(&program_id, &pubkey, 6, &mut bump)], Some(&mint_pubkey), ), ) - .unwrap_err() - .unwrap(), - TransactionError::InstructionError(0, InstructionError::InvalidRealloc) - ); + .unwrap(); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!(6, data.len()); - // Realloc to max length in max increase increments - for i in 0..MAX_PERMITTED_DATA_LENGTH as usize / MAX_PERMITTED_DATA_INCREASE { - let mut bump = i as u64; + // Extend by 2 bytes and write a u64. This ensures that we can do writes that span the original + // account length (6 bytes) and the realloc data (2 bytes). bank_client .send_and_confirm_message( signer, Message::new( - &[realloc_extend_and_fill( + &[extend_and_write_u64( &program_id, &pubkey, - MAX_PERMITTED_DATA_INCREASE, - 1, - &mut bump, + 0x1122334455667788, )], Some(&mint_pubkey), ), ) .unwrap(); let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); - assert_eq!((i + 1) * MAX_PERMITTED_DATA_INCREASE, data.len()); - } - for i in 0..data.len() { - assert_eq!(data[i], 1); - } + assert_eq!(8, data.len()); + assert_eq!(0x1122334455667788, unsafe { *data.as_ptr().cast::() }); - // and one more time should fail - assert_eq!( + // Realloc to 0 bank_client .send_and_confirm_message( signer, Message::new( - &[realloc_extend( - &program_id, - &pubkey, - MAX_PERMITTED_DATA_INCREASE, - &mut bump - )], + &[realloc(&program_id, &pubkey, 0, &mut bump)], Some(&mint_pubkey), - ) + ), ) - .unwrap_err() - .unwrap(), - TransactionError::InstructionError(0, InstructionError::InvalidRealloc) - ); - - // Realloc to 0 - bank_client - .send_and_confirm_message( - signer, - Message::new( - &[realloc(&program_id, &pubkey, 0, &mut bump)], - Some(&mint_pubkey), - ), - ) - .unwrap(); - let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); - assert_eq!(0, data.len()); - - // Realloc and assign - bank_client - .send_and_confirm_message( - signer, - Message::new( - &[Instruction::new_with_bytes( - program_id, - &[REALLOC_AND_ASSIGN], - vec![AccountMeta::new(pubkey, false)], - )], - Some(&mint_pubkey), - ), - ) - .unwrap(); - let account = bank.get_account(&pubkey).unwrap(); - assert_eq!(&solana_sdk::system_program::id(), account.owner()); - let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); - assert_eq!(MAX_PERMITTED_DATA_INCREASE, data.len()); + .unwrap(); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!(0, data.len()); - // Realloc to 0 with wrong owner - assert_eq!( + // Realloc and assign bank_client .send_and_confirm_message( signer, Message::new( - &[realloc(&program_id, &pubkey, 0, &mut bump)], + &[Instruction::new_with_bytes( + program_id, + &[REALLOC_AND_ASSIGN], + vec![AccountMeta::new(pubkey, false)], + )], Some(&mint_pubkey), ), ) - .unwrap_err() - .unwrap(), - TransactionError::InstructionError(0, InstructionError::AccountDataSizeChanged) - ); + .unwrap(); + let account = bank.get_account(&pubkey).unwrap(); + assert_eq!(&solana_sdk::system_program::id(), account.owner()); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!(MAX_PERMITTED_DATA_INCREASE, data.len()); - // realloc and assign to self via cpi - assert_eq!( + // Realloc to 0 with wrong owner + assert_eq!( + bank_client + .send_and_confirm_message( + signer, + Message::new( + &[realloc(&program_id, &pubkey, 0, &mut bump)], + Some(&mint_pubkey), + ), + ) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::AccountDataSizeChanged) + ); + + // realloc and assign to self via cpi + assert_eq!( + bank_client + .send_and_confirm_message( + &[&mint_keypair, &keypair], + Message::new( + &[Instruction::new_with_bytes( + program_id, + &[REALLOC_AND_ASSIGN_TO_SELF_VIA_SYSTEM_PROGRAM], + vec![ + AccountMeta::new(pubkey, true), + AccountMeta::new(solana_sdk::system_program::id(), false), + ], + )], + Some(&mint_pubkey), + ) + ) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::AccountDataSizeChanged) + ); + + // Assign to self and realloc via cpi bank_client .send_and_confirm_message( &[&mint_keypair, &keypair], Message::new( &[Instruction::new_with_bytes( program_id, - &[REALLOC_AND_ASSIGN_TO_SELF_VIA_SYSTEM_PROGRAM], + &[ASSIGN_TO_SELF_VIA_SYSTEM_PROGRAM_AND_REALLOC], vec![ AccountMeta::new(pubkey, true), AccountMeta::new(solana_sdk::system_program::id(), false), ], )], Some(&mint_pubkey), - ) + ), ) - .unwrap_err() - .unwrap(), - TransactionError::InstructionError(0, InstructionError::AccountDataSizeChanged) - ); - - // Assign to self and realloc via cpi - bank_client - .send_and_confirm_message( - &[&mint_keypair, &keypair], - Message::new( - &[Instruction::new_with_bytes( - program_id, - &[ASSIGN_TO_SELF_VIA_SYSTEM_PROGRAM_AND_REALLOC], - vec![ - AccountMeta::new(pubkey, true), - AccountMeta::new(solana_sdk::system_program::id(), false), - ], - )], - Some(&mint_pubkey), - ), - ) - .unwrap(); - let account = bank.get_account(&pubkey).unwrap(); - assert_eq!(&program_id, account.owner()); - let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); - assert_eq!(2 * MAX_PERMITTED_DATA_INCREASE, data.len()); + .unwrap(); + let account = bank.get_account(&pubkey).unwrap(); + assert_eq!(&program_id, account.owner()); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!(2 * MAX_PERMITTED_DATA_INCREASE, data.len()); - // Realloc to 0 - bank_client - .send_and_confirm_message( - signer, - Message::new( - &[realloc(&program_id, &pubkey, 0, &mut bump)], - Some(&mint_pubkey), - ), - ) - .unwrap(); - let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); - assert_eq!(0, data.len()); + // Realloc to 0 + bank_client + .send_and_confirm_message( + signer, + Message::new( + &[realloc(&program_id, &pubkey, 0, &mut bump)], + Some(&mint_pubkey), + ), + ) + .unwrap(); + let data = bank_client.get_account_data(&pubkey).unwrap().unwrap(); + assert_eq!(0, data.len()); - // zero-init - bank_client - .send_and_confirm_message( - &[&mint_keypair, &keypair], - Message::new( - &[Instruction::new_with_bytes( - program_id, - &[ZERO_INIT], - vec![AccountMeta::new(pubkey, true)], - )], - Some(&mint_pubkey), - ), - ) - .unwrap(); + // zero-init + bank_client + .send_and_confirm_message( + &[&mint_keypair, &keypair], + Message::new( + &[Instruction::new_with_bytes( + program_id, + &[ZERO_INIT], + vec![AccountMeta::new(pubkey, true)], + )], + Some(&mint_pubkey), + ), + ) + .unwrap(); + } } #[test] diff --git a/rbpf-cli/src/main.rs b/rbpf-cli/src/main.rs index 30ad73435cfc30..4408603bdc92a4 100644 --- a/rbpf-cli/src/main.rs +++ b/rbpf-cli/src/main.rs @@ -235,6 +235,7 @@ before execting it in the virtual machine.", .get_current_instruction_context() .unwrap(), true, // should_cap_ix_accounts + true, ) .unwrap(); diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index b9bc16612d72ac..705aba3ae19b30 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7445,7 +7445,9 @@ fn test_bank_executor_cache() { upgrade_authority_address: None, }) .unwrap(); - programdata_account.data_mut()[programdata_data_offset..].copy_from_slice(&elf); + let mut data = programdata_account.data().to_vec(); + data[programdata_data_offset..].copy_from_slice(&elf); + programdata_account.set_data_from_slice(&data); programdata_account.set_rent_epoch(1); bank.store_account_and_update_capitalization(&key1, &program_account); bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); @@ -7494,7 +7496,7 @@ fn test_bank_load_program() { upgrade_authority_address: None, }) .unwrap(); - programdata_account.data_mut()[programdata_data_offset..].copy_from_slice(&elf); + programdata_account.data_as_mut_slice()[programdata_data_offset..].copy_from_slice(&elf); programdata_account.set_rent_epoch(1); bank.store_account_and_update_capitalization(&key1, &program_account); bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); diff --git a/sdk/src/account.rs b/sdk/src/account.rs index fca8f8b71f5165..0657a5b17637b8 100644 --- a/sdk/src/account.rs +++ b/sdk/src/account.rs @@ -13,7 +13,9 @@ use { solana_program::{account_info::AccountInfo, debug_account_data::*, sysvar::Sysvar}, std::{ cell::{Ref, RefCell}, - fmt, ptr, + fmt, + mem::MaybeUninit, + ptr, rc::Rc, sync::Arc, }, @@ -174,7 +176,6 @@ pub trait WritableAccount: ReadableAccount { fn saturating_sub_lamports(&mut self, lamports: u64) { self.set_lamports(self.lamports().saturating_sub(lamports)) } - fn data_mut(&mut self) -> &mut Vec; fn data_as_mut_slice(&mut self) -> &mut [u8]; fn set_owner(&mut self, owner: Pubkey); fn copy_into_owner_from_slice(&mut self, source: &[u8]); @@ -228,9 +229,6 @@ impl WritableAccount for Account { fn set_lamports(&mut self, lamports: u64) { self.lamports = lamports; } - fn data_mut(&mut self) -> &mut Vec { - &mut self.data - } fn data_as_mut_slice(&mut self) -> &mut [u8] { &mut self.data } @@ -267,9 +265,6 @@ impl WritableAccount for AccountSharedData { fn set_lamports(&mut self, lamports: u64) { self.lamports = lamports; } - fn data_mut(&mut self) -> &mut Vec { - Arc::make_mut(&mut self.data) - } fn data_as_mut_slice(&mut self) -> &mut [u8] { &mut self.data_mut()[..] } @@ -540,6 +535,30 @@ impl Account { } impl AccountSharedData { + pub fn is_shared(&self) -> bool { + Arc::strong_count(&self.data) > 1 + } + + pub fn reserve(&mut self, additional: usize) { + self.data_mut().reserve(additional) + } + + pub fn capacity(&self) -> usize { + self.data.capacity() + } + + fn data_mut(&mut self) -> &mut Vec { + Arc::make_mut(&mut self.data) + } + + pub fn resize(&mut self, new_len: usize, value: u8) { + self.data_mut().resize(new_len, value) + } + + pub fn extend_from_slice(&mut self, data: &[u8]) { + self.data_mut().extend_from_slice(data) + } + pub fn set_data_from_slice(&mut self, new_data: &[u8]) { let data = match Arc::get_mut(&mut self.data) { // The buffer isn't shared, so we're going to memcpy in place. @@ -584,6 +603,10 @@ impl AccountSharedData { self.data = Arc::new(data); } + pub fn spare_data_capacity_mut(&mut self) -> &mut [MaybeUninit] { + self.data_mut().spare_capacity_mut() + } + pub fn new(lamports: u64, space: usize, owner: &Pubkey) -> Self { shared_new(lamports, space, owner) } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 25dfacec6e8e5f..37b1e4dfb704b0 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -617,6 +617,9 @@ pub mod delay_visibility_of_program_deployment { pub mod apply_cost_tracker_during_replay { solana_sdk::declare_id!("2ry7ygxiYURULZCrypHhveanvP5tzZ4toRwVp89oCNSj"); } +pub mod bpf_account_data_direct_mapping { + solana_sdk::declare_id!("9gwzizfABsKUereT6phZZxbTzuAnovkgwpVVpdcSxv9h"); +} pub mod add_set_tx_loaded_accounts_data_size_instruction { solana_sdk::declare_id!("G6vbf1UBok8MWb8m25ex86aoQHeKTzDKzuZADHkShqm6"); @@ -822,6 +825,7 @@ lazy_static! { (clean_up_delegation_errors::id(), "Return InsufficientDelegation instead of InsufficientFunds or InsufficientStake where applicable #31206"), (vote_state_add_vote_latency::id(), "replace Lockout with LandedVote (including vote latency) in vote state #31264"), (checked_arithmetic_in_fee_validation::id(), "checked arithmetic in fee validation #31273"), + (bpf_account_data_direct_mapping::id(), "use memory regions to map account data into the rbpf vm instead of copying the data"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 023607374ed9bc..8f8d1bb038bc20 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -4,12 +4,16 @@ #[cfg(all(not(target_os = "solana"), debug_assertions))] use crate::signature::Signature; #[cfg(not(target_os = "solana"))] -use crate::{ - account::WritableAccount, - rent::Rent, - system_instruction::{ - MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION, MAX_PERMITTED_DATA_LENGTH, +use { + crate::{ + account::WritableAccount, + rent::Rent, + system_instruction::{ + MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION, MAX_PERMITTED_DATA_LENGTH, + }, }, + solana_program::entrypoint::MAX_PERMITTED_DATA_INCREASE, + std::mem::MaybeUninit, }; use { crate::{ @@ -18,9 +22,10 @@ use { pubkey::Pubkey, }, std::{ - cell::{RefCell, RefMut}, + cell::{Ref, RefCell, RefMut}, collections::HashSet, pin::Pin, + sync::Arc, }, }; @@ -51,15 +56,93 @@ pub struct InstructionAccount { /// An account key and the matching account pub type TransactionAccount = (Pubkey, AccountSharedData); +#[derive(Clone, Debug, PartialEq)] +pub struct TransactionAccounts { + accounts: Vec>, + touched_flags: RefCell>, + is_early_verification_of_account_modifications_enabled: bool, +} + +impl TransactionAccounts { + #[cfg(not(target_os = "solana"))] + fn new( + accounts: Vec>, + is_early_verification_of_account_modifications_enabled: bool, + ) -> TransactionAccounts { + TransactionAccounts { + touched_flags: RefCell::new(vec![false; accounts.len()].into_boxed_slice()), + accounts, + is_early_verification_of_account_modifications_enabled, + } + } + + fn len(&self) -> usize { + self.accounts.len() + } + + pub fn get(&self, index: IndexOfAccount) -> Option<&RefCell> { + self.accounts.get(index as usize) + } + + #[cfg(not(target_os = "solana"))] + pub fn touch(&self, index: IndexOfAccount) -> Result<(), InstructionError> { + if self.is_early_verification_of_account_modifications_enabled { + *self + .touched_flags + .borrow_mut() + .get_mut(index as usize) + .ok_or(InstructionError::NotEnoughAccountKeys)? = true; + } + Ok(()) + } + + #[cfg(not(target_os = "solana"))] + pub fn touched_count(&self) -> usize { + self.touched_flags + .borrow() + .iter() + .fold(0usize, |accumulator, was_touched| { + accumulator.saturating_add(*was_touched as usize) + }) + } + + pub fn try_borrow( + &self, + index: IndexOfAccount, + ) -> Result, InstructionError> { + self.accounts + .get(index as usize) + .ok_or(InstructionError::MissingAccount)? + .try_borrow() + .map_err(|_| InstructionError::AccountBorrowFailed) + } + + pub fn try_borrow_mut( + &self, + index: IndexOfAccount, + ) -> Result, InstructionError> { + self.accounts + .get(index as usize) + .ok_or(InstructionError::MissingAccount)? + .try_borrow_mut() + .map_err(|_| InstructionError::AccountBorrowFailed) + } + + pub fn into_accounts(self) -> Vec { + self.accounts + .into_iter() + .map(|account| account.into_inner()) + .collect() + } +} + /// Loaded transaction shared between runtime and programs. /// /// This context is valid for the entire duration of a transaction being processed. #[derive(Debug, Clone, PartialEq)] pub struct TransactionContext { account_keys: Pin>, - accounts: Pin]>>, - #[cfg(not(target_os = "solana"))] - account_touched_flags: RefCell>>, + accounts: Arc, instruction_stack_capacity: usize, instruction_trace_capacity: usize, instruction_stack: Vec, @@ -84,16 +167,13 @@ impl TransactionContext { instruction_stack_capacity: usize, instruction_trace_capacity: usize, ) -> Self { - let (account_keys, accounts): (Vec, Vec>) = - transaction_accounts - .into_iter() - .map(|(key, account)| (key, RefCell::new(account))) - .unzip(); - let account_touched_flags = vec![false; accounts.len()]; + let (account_keys, accounts): (Vec<_>, Vec<_>) = transaction_accounts + .into_iter() + .map(|(key, account)| (key, RefCell::new(account))) + .unzip(); Self { account_keys: Pin::new(account_keys.into_boxed_slice()), - accounts: Pin::new(accounts.into_boxed_slice()), - account_touched_flags: RefCell::new(Pin::new(account_touched_flags.into_boxed_slice())), + accounts: Arc::new(TransactionAccounts::new(accounts, rent.is_some())), instruction_stack_capacity, instruction_trace_capacity, instruction_stack: Vec::with_capacity(instruction_stack_capacity), @@ -113,10 +193,15 @@ impl TransactionContext { if !self.instruction_stack.is_empty() { return Err(InstructionError::CallDepth); } - Ok(Vec::from(Pin::into_inner(self.accounts)) - .into_iter() - .map(|account| account.into_inner()) - .collect()) + + Ok(Arc::try_unwrap(self.accounts) + .expect("transaction_context.accounts has unexpected outstanding refs") + .into_accounts()) + } + + #[cfg(not(target_os = "solana"))] + pub fn accounts(&self) -> &Arc { + &self.accounts } /// Returns true if `enable_early_verification_of_account_modifications` is active @@ -159,7 +244,7 @@ impl TransactionContext { index_in_transaction: IndexOfAccount, ) -> Result<&RefCell, InstructionError> { self.accounts - .get(index_in_transaction as usize) + .get(index_in_transaction) .ok_or(InstructionError::NotEnoughAccountKeys) } @@ -553,7 +638,7 @@ impl InstructionContext { ) -> Result, InstructionError> { let account = transaction_context .accounts - .get(index_in_transaction as usize) + .get(index_in_transaction) .ok_or(InstructionError::MissingAccount)? .try_borrow_mut() .map_err(|_| InstructionError::AccountBorrowFailed)?; @@ -663,6 +748,11 @@ pub struct BorrowedAccount<'a> { } impl<'a> BorrowedAccount<'a> { + /// Returns the transaction context + pub fn transaction_context(&self) -> &TransactionContext { + self.transaction_context + } + /// Returns the index of this account (transaction wide) #[inline] pub fn get_index_in_transaction(&self) -> IndexOfAccount { @@ -782,23 +872,35 @@ impl<'a> BorrowedAccount<'a> { pub fn get_data_mut(&mut self) -> Result<&mut [u8], InstructionError> { self.can_data_be_changed()?; self.touch()?; + self.make_data_mut(); Ok(self.account.data_as_mut_slice()) } + /// Returns the spare capacity of the vector backing the account data. + /// + /// This method should only ever be used during CPI, where after a shrinking + /// realloc we want to zero the spare capacity. + #[cfg(not(target_os = "solana"))] + pub fn spare_data_capacity_mut(&mut self) -> Result<&mut [MaybeUninit], InstructionError> { + debug_assert!(!self.account.is_shared()); + Ok(self.account.spare_data_capacity_mut()) + } + /// Overwrites the account data and size (transaction wide). /// - /// Call this when you have an owned buffer and want to replace the account - /// data with it. + /// You should always prefer set_data_from_slice(). Calling this method is + /// currently safe but requires some special casing during CPI when direct + /// account mapping is enabled. /// - /// If you have a slice, use [`Self::set_data_from_slice()`]. + /// Currently only used by tests and the program-test crate. #[cfg(not(target_os = "solana"))] pub fn set_data(&mut self, data: Vec) -> Result<(), InstructionError> { self.can_data_be_resized(data.len())?; self.can_data_be_changed()?; self.touch()?; + self.update_accounts_resize_delta(data.len())?; self.account.set_data(data); - Ok(()) } @@ -806,14 +908,19 @@ impl<'a> BorrowedAccount<'a> { /// /// Call this when you have a slice of data you do not own and want to /// replace the account data with it. - /// - /// If you have an owned buffer (eg [`Vec`]), use [`Self::set_data()`]. #[cfg(not(target_os = "solana"))] pub fn set_data_from_slice(&mut self, data: &[u8]) -> Result<(), InstructionError> { self.can_data_be_resized(data.len())?; self.can_data_be_changed()?; self.touch()?; self.update_accounts_resize_delta(data.len())?; + // Calling make_data_mut() here guarantees that set_data_from_slice() + // copies in places, extending the account capacity if necessary but + // never reducing it. This is required as the account migh be directly + // mapped into a MemoryRegion, and therefore reducing capacity would + // leave a hole in the vm address space. After CPI or upon program + // termination, the runtime will zero the extra capacity. + self.make_data_mut(); self.account.set_data_from_slice(data); Ok(()) @@ -832,7 +939,7 @@ impl<'a> BorrowedAccount<'a> { } self.touch()?; self.update_accounts_resize_delta(new_length)?; - self.account.data_mut().resize(new_length, 0); + self.account.resize(new_length, 0); Ok(()) } @@ -849,10 +956,58 @@ impl<'a> BorrowedAccount<'a> { self.touch()?; self.update_accounts_resize_delta(new_len)?; - self.account.data_mut().extend_from_slice(data); + // Even if extend_from_slice never reduces capacity, still realloc using + // make_data_mut() if necessary so that we grow the account of the full + // max realloc length in one go, avoiding smaller reallocations. + self.make_data_mut(); + self.account.extend_from_slice(data); Ok(()) } + /// Reserves capacity for at least additional more elements to be inserted + /// in the given account. Does nothing if capacity is already sufficient. + #[cfg(not(target_os = "solana"))] + pub fn reserve(&mut self, additional: usize) -> Result<(), InstructionError> { + self.can_data_be_changed()?; + self.make_data_mut(); + self.account.reserve(additional); + + Ok(()) + } + + /// Returns the number of bytes the account can hold without reallocating. + #[cfg(not(target_os = "solana"))] + pub fn capacity(&self) -> usize { + self.account.capacity() + } + + /// Returns whether the underlying AccountSharedData is shared. + /// + /// The data is shared if the account has been loaded from the accounts database and has never + /// been written to. Writing to an account unshares it. + /// + /// During account serialization, if an account is shared it'll get mapped as CoW, else it'll + /// get mapped directly as writable. + #[cfg(not(target_os = "solana"))] + pub fn is_shared(&self) -> bool { + self.account.is_shared() + } + + #[cfg(not(target_os = "solana"))] + fn make_data_mut(&mut self) { + // if the account is still shared, it means this is the first time we're + // about to write into it. Make the account mutable by copying it in a + // buffer with MAX_PERMITTED_DATA_INCREASE capacity so that if the + // transaction reallocs, we don't have to copy the whole account data a + // second time to fullfill the realloc. + // + // NOTE: The account memory region CoW code in Serializer::push_account_region() implements + // the same logic and must be kept in sync. + if self.account.is_shared() { + self.account.reserve(MAX_PERMITTED_DATA_INCREASE); + } + } + /// Deserializes the account data into a state #[cfg(not(target_os = "solana"))] pub fn get_state(&self) -> Result { @@ -1023,19 +1178,9 @@ impl<'a> BorrowedAccount<'a> { #[cfg(not(target_os = "solana"))] fn touch(&self) -> Result<(), InstructionError> { - if self - .transaction_context - .is_early_verification_of_account_modifications_enabled() - { - *self - .transaction_context - .account_touched_flags - .try_borrow_mut() - .map_err(|_| InstructionError::GenericError)? - .get_mut(self.index_in_transaction as usize) - .ok_or(InstructionError::NotEnoughAccountKeys)? = true; - } - Ok(()) + self.transaction_context + .accounts() + .touch(self.index_in_transaction) } #[cfg(not(target_os = "solana"))] @@ -1064,23 +1209,14 @@ pub struct ExecutionRecord { #[cfg(not(target_os = "solana"))] impl From for ExecutionRecord { fn from(context: TransactionContext) -> Self { - let account_touched_flags = context - .account_touched_flags - .try_borrow() - .expect("borrowing transaction_context.account_touched_flags failed"); - let touched_account_count = account_touched_flags - .iter() - .fold(0u64, |accumulator, was_touched| { - accumulator.saturating_add(*was_touched as u64) - }); + let accounts = Arc::try_unwrap(context.accounts) + .expect("transaction_context.accounts has unexpectd outstanding refs"); + let touched_account_count = accounts.touched_count() as u64; + let accounts = accounts.into_accounts(); Self { accounts: Vec::from(Pin::into_inner(context.account_keys)) .into_iter() - .zip( - Vec::from(Pin::into_inner(context.accounts)) - .into_iter() - .map(|account| account.into_inner()), - ) + .zip(accounts) .collect(), return_data: context.return_data, touched_account_count,