diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs index 362f67ef21817b..bd26f9b52033f0 100644 --- a/feature-set/src/lib.rs +++ b/feature-set/src/lib.rs @@ -106,7 +106,8 @@ impl FeatureSet { .is_active(&move_precompile_verification_to_svm::id()), remove_accounts_executable_flag_checks: self .is_active(&remove_accounts_executable_flag_checks::id()), - bpf_account_data_direct_mapping: self.is_active(&bpf_account_data_direct_mapping::id()), + stricter_abi_and_runtime_constraints: self + .is_active(&stricter_abi_and_runtime_constraints::id()), enable_bpf_loader_set_authority_checked_ix: self .is_active(&enable_bpf_loader_set_authority_checked_ix::id()), enable_loader_v4: self.is_active(&enable_loader_v4::id()), @@ -752,8 +753,8 @@ pub mod apply_cost_tracker_during_replay { solana_pubkey::declare_id!("2ry7ygxiYURULZCrypHhveanvP5tzZ4toRwVp89oCNSj"); } -pub mod bpf_account_data_direct_mapping { - solana_pubkey::declare_id!("1ncomp1ete111111111111111111111111111111111"); +pub mod stricter_abi_and_runtime_constraints { + solana_pubkey::declare_id!("C37iaPi6VE4CZDueU1vL8y6pGp5i8amAbEsF31xzz723"); } pub mod add_set_tx_loaded_accounts_data_size_instruction { @@ -1279,7 +1280,7 @@ pub static FEATURE_NAMES: LazyLock> = LazyLock::n (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"), + (stricter_abi_and_runtime_constraints::id(), "use memory regions to map account data into the rbpf vm instead of copying the data"), (last_restart_slot_sysvar::id(), "enable new sysvar last_restart_slot"), (reduce_stake_warmup_cooldown::id(), "reduce stake warmup cooldown from 25% to 9%"), (revise_turbine_epoch_stakes::id(), "revise turbine epoch stakes"), diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index 47040df6b5b2fb..b76379cb6f9364 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -535,7 +535,8 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { .transaction_context .get_current_instruction_context() .unwrap(), - false, // direct_mapping + false, // stricter_abi_and_runtime_constraints + false, // account_data_direct_mapping true, // for mask_out_rent_epoch_in_vm_serialization ) .unwrap(); diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index b4ca250f84ed14..22a02d58d48a32 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -202,6 +202,8 @@ pub struct InvokeContext<'a> { pub timings: ExecuteDetailsTimings, pub syscall_context: Vec>, traces: Vec>, + /// Stops copying account data if stricter_abi_and_runtime_constraints is enabled + pub account_data_direct_mapping: bool, } impl<'a> InvokeContext<'a> { @@ -226,6 +228,7 @@ impl<'a> InvokeContext<'a> { timings: ExecuteDetailsTimings::default(), syscall_context: Vec::new(), traces: Vec::new(), + account_data_direct_mapping: false, } } diff --git a/program-runtime/src/serialization.rs b/program-runtime/src/serialization.rs index c1e263ef51ed5d..eb3e3c99d83afd 100644 --- a/program-runtime/src/serialization.rs +++ b/program-runtime/src/serialization.rs @@ -22,7 +22,22 @@ use { /// SBF VM. const MAX_INSTRUCTION_ACCOUNTS: u8 = NON_DUP_MARKER; -/// Creates the account data direct mapping in serialization and CPI return +/// Modifies the memory mapping in serialization and CPI return for stricter_abi_and_runtime_constraints +pub fn modify_memory_region_of_account( + account: &mut BorrowedAccount<'_>, + region: &mut MemoryRegion, +) { + region.len = account.get_data().len() as u64; + if account.can_data_be_changed().is_ok() { + region.writable = true; + region.access_violation_handler_payload = Some(account.get_index_in_transaction()); + } else { + region.writable = false; + region.access_violation_handler_payload = None; + } +} + +/// Creates the memory mapping in serialization and CPI return for account_data_direct_mapping pub fn create_memory_region_of_account( account: &mut BorrowedAccount<'_>, vaddr: u64, @@ -51,18 +66,26 @@ struct Serializer { vaddr: u64, region_start: usize, is_loader_v1: bool, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, } impl Serializer { - fn new(size: usize, start_addr: u64, is_loader_v1: bool, direct_mapping: bool) -> Serializer { + fn new( + size: usize, + start_addr: u64, + is_loader_v1: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, + ) -> Serializer { Serializer { buffer: AlignedMemory::with_capacity(size), regions: Vec::new(), region_start: 0, vaddr: start_addr, is_loader_v1, - direct_mapping, + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, } } @@ -109,13 +132,26 @@ impl Serializer { &mut self, account: &mut BorrowedAccount<'_>, ) -> Result { - let vm_data_addr = if !self.direct_mapping { + if !self.stricter_abi_and_runtime_constraints { let vm_data_addr = self.vaddr.saturating_add(self.buffer.len() as u64); self.write_all(account.get_data()); - vm_data_addr + if !self.is_loader_v1 { + 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)?; + } + Ok(vm_data_addr) } else { self.push_region(); - let vaddr = self.vaddr; + let vm_data_addr = self.vaddr; + if !self.account_data_direct_mapping { + self.write_all(account.get_data()); + if !self.is_loader_v1 { + self.fill_write(MAX_PERMITTED_DATA_INCREASE, 0) + .map_err(|_| InstructionError::InvalidArgument)?; + } + } let address_space_reserved_for_account = if !self.is_loader_v1 { account .get_data() @@ -125,32 +161,35 @@ impl Serializer { account.get_data().len() }; if address_space_reserved_for_account > 0 { - let new_region = create_memory_region_of_account(account, self.vaddr)?; - self.vaddr += address_space_reserved_for_account as u64; - self.regions.push(new_region); + if !self.account_data_direct_mapping { + self.push_region(); + let region = self.regions.last_mut().unwrap(); + modify_memory_region_of_account(account, region); + } else { + let new_region = create_memory_region_of_account(account, self.vaddr)?; + self.vaddr += address_space_reserved_for_account as u64; + self.regions.push(new_region); + } } - vaddr - }; - - if !self.is_loader_v1 { - let align_offset = - (account.get_data().len() as *const u8).align_offset(BPF_ALIGN_OF_U128); - if !self.direct_mapping { - 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(BPF_ALIGN_OF_U128, 0) - .map_err(|_| InstructionError::InvalidArgument)?; - self.region_start += BPF_ALIGN_OF_U128.saturating_sub(align_offset); + if !self.is_loader_v1 { + let align_offset = + (account.get_data().len() as *const u8).align_offset(BPF_ALIGN_OF_U128); + if !self.account_data_direct_mapping { + self.fill_write(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(BPF_ALIGN_OF_U128, 0) + .map_err(|_| InstructionError::InvalidArgument)?; + self.region_start += BPF_ALIGN_OF_U128.saturating_sub(align_offset); + } } + Ok(vm_data_addr) } - - Ok(vm_data_addr) } fn push_region(&mut self) { @@ -186,7 +225,8 @@ impl Serializer { pub fn serialize_parameters( transaction_context: &TransactionContext, instruction_context: &InstructionContext, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, mask_out_rent_epoch_in_vm_serialization: bool, ) -> Result< ( @@ -235,7 +275,8 @@ pub fn serialize_parameters( accounts, instruction_context.get_instruction_data(), &program_id, - direct_mapping, + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, mask_out_rent_epoch_in_vm_serialization, ) } else { @@ -243,7 +284,8 @@ pub fn serialize_parameters( accounts, instruction_context.get_instruction_data(), &program_id, - direct_mapping, + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, mask_out_rent_epoch_in_vm_serialization, ) } @@ -252,7 +294,8 @@ pub fn serialize_parameters( pub fn deserialize_parameters( transaction_context: &TransactionContext, instruction_context: &InstructionContext, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, buffer: &[u8], accounts_metadata: &[SerializedAccountMetadata], ) -> Result<(), InstructionError> { @@ -265,7 +308,8 @@ pub fn deserialize_parameters( deserialize_parameters_unaligned( transaction_context, instruction_context, - direct_mapping, + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, buffer, account_lengths, ) @@ -273,7 +317,8 @@ pub fn deserialize_parameters( deserialize_parameters_aligned( transaction_context, instruction_context, - direct_mapping, + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, buffer, account_lengths, ) @@ -284,7 +329,8 @@ fn serialize_parameters_unaligned( accounts: Vec, instruction_data: &[u8], program_id: &Pubkey, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, mask_out_rent_epoch_in_vm_serialization: bool, ) -> Result< ( @@ -309,7 +355,7 @@ fn serialize_parameters_unaligned( + size_of::() // owner + size_of::() // executable + size_of::(); // rent_epoch - if !direct_mapping { + if !(stricter_abi_and_runtime_constraints && account_data_direct_mapping) { size += account.get_data().len(); } } @@ -319,7 +365,13 @@ fn serialize_parameters_unaligned( + instruction_data.len() // instruction data + size_of::(); // program id - let mut s = Serializer::new(size, MM_INPUT_START, true, direct_mapping); + let mut s = Serializer::new( + size, + MM_INPUT_START, + true, + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, + ); let mut accounts_metadata: Vec = Vec::with_capacity(accounts.len()); s.write::((accounts.len() as u64).to_le()); @@ -367,7 +419,8 @@ fn serialize_parameters_unaligned( fn deserialize_parameters_unaligned>( transaction_context: &TransactionContext, instruction_context: &InstructionContext, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, buffer: &[u8], account_lengths: I, ) -> Result<(), InstructionError> { @@ -396,7 +449,7 @@ fn deserialize_parameters_unaligned>( } start += size_of::() // lamports + size_of::(); // data length - if !direct_mapping { + if !stricter_abi_and_runtime_constraints { let data = buffer .get(start..start + pre_len) .ok_or(InstructionError::InvalidArgument)?; @@ -406,10 +459,18 @@ fn deserialize_parameters_unaligned>( Err(err) if borrowed_account.get_data() != data => return Err(err), _ => {} } - start += pre_len; // data + } else if !account_data_direct_mapping && borrowed_account.can_data_be_changed().is_ok() + { + let data = buffer + .get(start..start + pre_len) + .ok_or(InstructionError::InvalidArgument)?; + borrowed_account.set_data_from_slice(data)?; } else if borrowed_account.get_data().len() != pre_len { borrowed_account.set_data_length(pre_len)?; } + if !(stricter_abi_and_runtime_constraints && account_data_direct_mapping) { + start += pre_len; // data + } start += size_of::() // owner + size_of::() // executable + size_of::(); // rent_epoch @@ -422,7 +483,8 @@ fn serialize_parameters_aligned( accounts: Vec, instruction_data: &[u8], program_id: &Pubkey, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, mask_out_rent_epoch_in_vm_serialization: bool, ) -> Result< ( @@ -450,7 +512,7 @@ fn serialize_parameters_aligned( + size_of::() // lamports + size_of::() // data len + size_of::(); // rent epoch - if !direct_mapping { + if !(stricter_abi_and_runtime_constraints && account_data_direct_mapping) { size += data_len + MAX_PERMITTED_DATA_INCREASE + (data_len as *const u8).align_offset(BPF_ALIGN_OF_U128); @@ -464,7 +526,13 @@ fn serialize_parameters_aligned( + instruction_data.len() + size_of::(); // program id; - let mut s = Serializer::new(size, MM_INPUT_START, false, direct_mapping); + let mut s = Serializer::new( + size, + MM_INPUT_START, + false, + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, + ); // Serialize into the buffer s.write::((accounts.len() as u64).to_le()); @@ -514,7 +582,8 @@ fn serialize_parameters_aligned( fn deserialize_parameters_aligned>( transaction_context: &TransactionContext, instruction_context: &InstructionContext, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, buffer: &[u8], account_lengths: I, ) -> Result<(), InstructionError> { @@ -562,7 +631,7 @@ fn deserialize_parameters_aligned>( { return Err(InstructionError::InvalidRealloc); } - if !direct_mapping { + if !stricter_abi_and_runtime_constraints { let data = buffer .get(start..start + post_len) .ok_or(InstructionError::InvalidArgument)?; @@ -572,10 +641,16 @@ fn deserialize_parameters_aligned>( Err(err) if borrowed_account.get_data() != data => return Err(err), _ => {} } + } else if !account_data_direct_mapping && borrowed_account.can_data_be_changed().is_ok() + { + let data = buffer + .get(start..start + post_len) + .ok_or(InstructionError::InvalidArgument)?; + borrowed_account.set_data_from_slice(data)?; } else if borrowed_account.get_data().len() != post_len { borrowed_account.set_data_length(post_len)?; } - start += if !direct_mapping { + start += if !(stricter_abi_and_runtime_constraints && account_data_direct_mapping) { let alignment_offset = (pre_len as *const u8).align_offset(BPF_ALIGN_OF_U128); pre_len // data .saturating_add(MAX_PERMITTED_DATA_INCREASE) // realloc padding @@ -649,7 +724,7 @@ mod tests { name: &'static str, } - for direct_mapping in [false] { + for stricter_abi_and_runtime_constraints in [false, true] { for TestCase { num_ix_accounts, append_dup_account, @@ -728,8 +803,9 @@ mod tests { let serialization_result = serialize_parameters( invoke_context.transaction_context, instruction_context, - direct_mapping, - true, // mask_out_rent_epoch_in_vm_serialization + stricter_abi_and_runtime_constraints, + false, // account_data_direct_mapping + true, // mask_out_rent_epoch_in_vm_serialization ); assert_eq!( serialization_result.as_ref().err(), @@ -744,7 +820,7 @@ mod tests { let mut serialized_regions = concat_regions(®ions); let (de_program_id, de_accounts, de_instruction_data) = unsafe { deserialize( - if !direct_mapping { + if !stricter_abi_and_runtime_constraints { serialized.as_slice_mut() } else { serialized_regions.as_slice_mut() @@ -777,7 +853,7 @@ mod tests { #[test] fn test_serialize_parameters() { - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints in [false, true] { let program_id = solana_pubkey::new_rand(); let transaction_accounts = vec![ ( @@ -872,18 +948,19 @@ mod tests { let (mut serialized, regions, accounts_metadata) = serialize_parameters( invoke_context.transaction_context, instruction_context, - direct_mapping, - true, // mask_out_rent_epoch_in_vm_serialization + stricter_abi_and_runtime_constraints, + false, // account_data_direct_mapping + true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); let mut serialized_regions = concat_regions(®ions); - if !direct_mapping { + if !stricter_abi_and_runtime_constraints { assert_eq!(serialized.as_slice(), serialized_regions.as_slice()); } let (de_program_id, de_accounts, de_instruction_data) = unsafe { deserialize( - if !direct_mapping { + if !stricter_abi_and_runtime_constraints { serialized.as_slice_mut() } else { serialized_regions.as_slice_mut() @@ -932,7 +1009,8 @@ mod tests { deserialize_parameters( invoke_context.transaction_context, instruction_context, - direct_mapping, + stricter_abi_and_runtime_constraints, + false, // account_data_direct_mapping serialized.as_slice(), &accounts_metadata, ) @@ -965,15 +1043,16 @@ mod tests { let (mut serialized, regions, account_lengths) = serialize_parameters( invoke_context.transaction_context, instruction_context, - direct_mapping, - true, // mask_out_rent_epoch_in_vm_serialization + stricter_abi_and_runtime_constraints, + false, // account_data_direct_mapping + true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); let mut serialized_regions = concat_regions(®ions); let (de_program_id, de_accounts, de_instruction_data) = unsafe { deserialize_unaligned( - if !direct_mapping { + if !stricter_abi_and_runtime_constraints { serialized.as_slice_mut() } else { serialized_regions.as_slice_mut() @@ -1004,7 +1083,8 @@ mod tests { deserialize_parameters( invoke_context.transaction_context, instruction_context, - direct_mapping, + stricter_abi_and_runtime_constraints, + false, // account_data_direct_mapping serialized.as_slice(), &account_lengths, ) @@ -1119,6 +1199,7 @@ mod tests { invoke_context.transaction_context, instruction_context, true, + false, // account_data_direct_mapping mask_out_rent_epoch_in_vm_serialization, ) .unwrap(); @@ -1164,6 +1245,7 @@ mod tests { invoke_context.transaction_context, instruction_context, true, + false, // account_data_direct_mapping mask_out_rent_epoch_in_vm_serialization, ) .unwrap(); @@ -1411,7 +1493,7 @@ mod tests { regions, &config, SBPFVersion::V3, - transaction_context.access_violation_handler(), + transaction_context.access_violation_handler(true, true), ) .unwrap(); diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index a00642665a8233..2cf8c37b70f868 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -133,7 +133,8 @@ pub fn invoke_builtin_function( let (mut parameter_bytes, _regions, _account_lengths) = serialize_parameters( transaction_context, instruction_context, - false, // direct_mapping // There is no VM so direct mapping can not be implemented here + false, // There is no VM so stricter_abi_and_runtime_constraints can not be implemented here + false, // There is no VM so account_data_direct_mapping can not be implemented here mask_out_rent_epoch_in_vm_serialization, )?; diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index 917a530eb18e35..f603279b133b72 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -123,7 +123,8 @@ fn bench_serialize_unaligned(c: &mut Criterion) { let _ = serialize_parameters( &transaction_context, instruction_context, - true, // direct_mapping + true, // stricter_abi_and_runtime_constraints + true, // account_data_direct_mapping true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); @@ -141,7 +142,8 @@ fn bench_serialize_unaligned_copy_account_data(c: &mut Criterion) { let _ = serialize_parameters( &transaction_context, instruction_context, - false, // direct_mapping + false, // stricter_abi_and_runtime_constraints + false, // account_data_direct_mapping true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); @@ -160,7 +162,8 @@ fn bench_serialize_aligned(c: &mut Criterion) { let _ = serialize_parameters( &transaction_context, instruction_context, - true, // direct_mapping + true, // stricter_abi_and_runtime_constraints + true, // account_data_direct_mapping true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); @@ -179,7 +182,8 @@ fn bench_serialize_aligned_copy_account_data(c: &mut Criterion) { let _ = serialize_parameters( &transaction_context, instruction_context, - false, // direct_mapping + false, // stricter_abi_and_runtime_constraints + false, // account_data_direct_mapping true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); @@ -198,7 +202,8 @@ fn bench_serialize_unaligned_max_accounts(c: &mut Criterion) { let _ = serialize_parameters( &transaction_context, instruction_context, - true, // direct_mapping + true, // stricter_abi_and_runtime_constraints + true, // account_data_direct_mapping true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); @@ -217,7 +222,8 @@ fn bench_serialize_aligned_max_accounts(c: &mut Criterion) { let _ = serialize_parameters( &transaction_context, instruction_context, - true, // direct_mapping + true, // stricter_abi_and_runtime_constraints + true, // account_data_direct_mapping true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index b194dcbf719c3b..e06e5c811aabf1 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -269,6 +269,10 @@ fn create_vm<'a, 'b>( heap, regions, invoke_context.transaction_context, + invoke_context + .get_feature_set() + .stricter_abi_and_runtime_constraints, + invoke_context.account_data_direct_mapping, )?; invoke_context.set_syscall_context(SyscallContext { allocator: BpfAllocator::new(heap_size as u64), @@ -322,6 +326,8 @@ fn create_memory_mapping<'a, 'b, C: ContextObject>( heap: &'b mut [u8], additional_regions: Vec, transaction_context: &TransactionContext, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, ) -> Result, Box> { let config = executable.get_config(); let sbpf_version = executable.get_sbpf_version(); @@ -346,7 +352,10 @@ fn create_memory_mapping<'a, 'b, C: ContextObject>( regions, config, sbpf_version, - transaction_context.access_violation_handler(), + transaction_context.access_violation_handler( + stricter_abi_and_runtime_constraints, + account_data_direct_mapping, + ), )?) } @@ -1587,9 +1596,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_compiled_program().is_some(); - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; let mask_out_rent_epoch_in_vm_serialization = invoke_context .get_feature_set() .mask_out_rent_epoch_in_vm_serialization; @@ -1598,7 +1607,8 @@ fn execute<'a, 'b: 'a>( let (parameter_bytes, regions, accounts_metadata) = serialization::serialize_parameters( invoke_context.transaction_context, instruction_context, - direct_mapping, + stricter_abi_and_runtime_constraints, + invoke_context.account_data_direct_mapping, mask_out_rent_epoch_in_vm_serialization, )?; serialize_time.stop(); @@ -1679,7 +1689,7 @@ fn execute<'a, 'b: 'a>( invoke_context.consume(invoke_context.get_remaining()); } - if direct_mapping { + if stricter_abi_and_runtime_constraints { if let EbpfError::SyscallError(err) = error { error = err .downcast::() @@ -1689,7 +1699,7 @@ fn execute<'a, 'b: 'a>( if let EbpfError::AccessViolation(access_type, vm_addr, len, _section_name) = error { - // If direct_mapping is enabled and a program tries to write to a readonly + // If stricter_abi_and_runtime_constraints is enabled and a program tries to write to a readonly // region we'll get a memory access violation. Map it to a more specific // error so it's easier for developers to see what happened. if let Some((instruction_account_index, vm_addr_range)) = @@ -1757,14 +1767,15 @@ fn execute<'a, 'b: 'a>( fn deserialize_parameters( invoke_context: &mut InvokeContext, parameter_bytes: &[u8], - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, ) -> Result<(), InstructionError> { serialization::deserialize_parameters( invoke_context.transaction_context, invoke_context .transaction_context .get_current_instruction_context()?, - direct_mapping, + stricter_abi_and_runtime_constraints, + invoke_context.account_data_direct_mapping, parameter_bytes, &invoke_context.get_syscall_context()?.accounts_metadata, ) @@ -1772,8 +1783,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(), direct_mapping) - .map_err(|error| Box::new(error) as Box) + deserialize_parameters( + invoke_context, + parameter_bytes.as_slice(), + stricter_abi_and_runtime_constraints, + ) + .map_err(|error| Box::new(error) as Box) }); deserialize_time.stop(); diff --git a/programs/sbf/benches/bpf_loader.rs b/programs/sbf/benches/bpf_loader.rs index 2ede3d5e7207dd..4866f53213881e 100644 --- a/programs/sbf/benches/bpf_loader.rs +++ b/programs/sbf/benches/bpf_loader.rs @@ -224,9 +224,9 @@ fn bench_create_vm(bencher: &mut Bencher) { const BUDGET: u64 = 200_000; invoke_context.mock_set_remaining(BUDGET); - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; let raise_cpi_nesting_limit_to_8 = invoke_context .get_feature_set() .raise_cpi_nesting_limit_to_8; @@ -249,8 +249,9 @@ fn bench_create_vm(bencher: &mut Bencher) { .transaction_context .get_current_instruction_context() .unwrap(), - direct_mapping, - true, // mask_out_rent_epoch_in_vm_serialization + stricter_abi_and_runtime_constraints, + false, // account_data_direct_mapping + true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); @@ -273,9 +274,9 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) { const BUDGET: u64 = 200_000; invoke_context.mock_set_remaining(BUDGET); - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; // Serialize account data let (_serialized, regions, account_lengths) = serialize_parameters( @@ -284,8 +285,9 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) { .transaction_context .get_current_instruction_context() .unwrap(), - direct_mapping, - true, // mask_out_rent_epoch_in_vm_serialization + stricter_abi_and_runtime_constraints, + false, // account_data_direct_mapping + true, // mask_out_rent_epoch_in_vm_serialization ) .unwrap(); diff --git a/programs/sbf/rust/deprecated_loader/src/lib.rs b/programs/sbf/rust/deprecated_loader/src/lib.rs index 516bf7f23cb492..872fd73aa7ae17 100644 --- a/programs/sbf/rust/deprecated_loader/src/lib.rs +++ b/programs/sbf/rust/deprecated_loader/src/lib.rs @@ -74,9 +74,9 @@ fn process_instruction( let expected = { let data = &instruction_data[1..]; let prev_len = account.data_len(); - // when direct mapping is off, this will accidentally clobber + // when stricter_abi_and_runtime_constraints is off, this will accidentally clobber // whatever comes after the data slice (owner, executable, rent - // epoch etc). When direct mapping is on, you get an + // epoch etc). When stricter_abi_and_runtime_constraints is on, you get an // InvalidRealloc error. account.resize(prev_len + data.len())?; account.data.borrow_mut()[prev_len..].copy_from_slice(data); @@ -144,7 +144,7 @@ fn process_instruction( let realloc_program_id = accounts[REALLOC_PROGRAM_INDEX].key; let realloc_program_owner = accounts[REALLOC_PROGRAM_INDEX].owner; let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; - let direct_mapping = instruction_data[1]; + let stricter_abi_and_runtime_constraints = instruction_data[1]; let new_len = usize::from_le_bytes(instruction_data[2..10].try_into().unwrap()); let prev_len = account.data_len(); let expected = account.data.borrow()[..new_len].to_vec(); @@ -167,7 +167,7 @@ fn process_instruction( // deserialize_parameters_unaligned predates realloc support, and // hardcodes the account data length to the original length. if !solana_program::bpf_loader_deprecated::check_id(realloc_program_owner) - && direct_mapping == 0 + && stricter_abi_and_runtime_constraints == 0 { assert_eq!(&*account.data.borrow(), &expected); assert_eq!( @@ -208,7 +208,7 @@ fn process_instruction( }; let mut expected = account.data.borrow().to_vec(); - let direct_mapping = instruction_data[1]; + let stricter_abi_and_runtime_constraints = instruction_data[1]; let new_len = usize::from_le_bytes(instruction_data[2..10].try_into().unwrap()); expected.extend_from_slice(&instruction_data[10..]); let mut instruction_data = @@ -228,7 +228,7 @@ fn process_instruction( .unwrap(); assert_eq!(*account.data.borrow(), &prev_data[..new_len]); - if direct_mapping == 0 { + if stricter_abi_and_runtime_constraints == 0 { assert_eq!( unsafe { std::slice::from_raw_parts( diff --git a/programs/sbf/rust/invoke/src/lib.rs b/programs/sbf/rust/invoke/src/lib.rs index 8e5795ed838e89..0949e58d0a0564 100644 --- a/programs/sbf/rust/invoke/src/lib.rs +++ b/programs/sbf/rust/invoke/src/lib.rs @@ -877,7 +877,7 @@ fn process_instruction<'a>( // TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE_MOVING_DATA_POINTER // is that we don't move the data pointer past the // RcBox. This is needed to avoid the "Invalid account - // info pointer" check when direct mapping is enabled. + // info pointer" check when stricter_abi_and_runtime_constraints is enabled. // This also means we don't need to update the // serialized len like we do in the other test. value: RefCell::new(slice::from_raw_parts_mut( @@ -1032,7 +1032,7 @@ fn process_instruction<'a>( let realloc_program_id = accounts[REALLOC_PROGRAM_INDEX].key; let realloc_program_owner = accounts[REALLOC_PROGRAM_INDEX].owner; let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; - let direct_mapping = instruction_data[1]; + let stricter_abi_and_runtime_constraints = instruction_data[1]; let new_len = usize::from_le_bytes(instruction_data[2..10].try_into().unwrap()); let prev_len = account.data_len(); let expected = account.data.borrow()[..new_len].to_vec(); @@ -1054,7 +1054,9 @@ fn process_instruction<'a>( // deserialize_parameters_unaligned predates realloc support, and // hardcodes the account data length to the original length. - if !bpf_loader_deprecated::check_id(realloc_program_owner) && direct_mapping == 0 { + if !bpf_loader_deprecated::check_id(realloc_program_owner) + && stricter_abi_and_runtime_constraints == 0 + { assert_eq!(&*account.data.borrow(), &expected); assert_eq!( unsafe { @@ -1093,7 +1095,7 @@ fn process_instruction<'a>( }; let mut expected = account.data.borrow().to_vec(); - let direct_mapping = instruction_data[1]; + let stricter_abi_and_runtime_constraints = instruction_data[1]; let new_len = usize::from_le_bytes(instruction_data[2..10].try_into().unwrap()); expected.extend_from_slice(&instruction_data[10..]); let mut instruction_data = @@ -1113,7 +1115,7 @@ fn process_instruction<'a>( .unwrap(); assert_eq!(*account.data.borrow(), &prev_data[..new_len]); - if direct_mapping == 0 { + if stricter_abi_and_runtime_constraints == 0 { assert_eq!( unsafe { slice::from_raw_parts( @@ -1302,7 +1304,7 @@ fn process_instruction<'a>( } if !invoke_struction.is_empty() { - // Invoke another program. With direct mapping, before CPI the callee will update the accounts (incl resizing) + // Invoke another program. With stricter_abi_and_runtime_constraints, before CPI the callee will update the accounts (incl resizing) // so the pointer may change. let invoked_program_id = accounts[INVOKED_PROGRAM_INDEX].key; diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 32758a09344a58..7ceaf621ac4bda 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -2445,13 +2445,13 @@ fn test_program_sbf_realloc() { let mint_pubkey = mint_keypair.pubkey(); let signer = &[&mint_keypair]; - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints 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()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let mut bank_client = BankClient::new_shared(bank.clone()); @@ -3738,7 +3738,7 @@ fn test_program_sbf_inner_instruction_alignment_checks() { fn test_cpi_account_ownership_writability() { solana_logger::setup(); - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints in [false, true] { let GenesisConfigInfo { genesis_config, mint_keypair, @@ -3747,8 +3747,8 @@ fn test_cpi_account_ownership_writability() { let mut bank = Bank::new_for_tests(&genesis_config); let mut feature_set = FeatureSet::all_enabled(); - if !direct_mapping { - feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } bank.feature_set = Arc::new(feature_set); @@ -3815,7 +3815,7 @@ fn test_cpi_account_ownership_writability() { let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); - if (byte_index as usize) < account_size || direct_mapping { + if (byte_index as usize) < account_size || stricter_abi_and_runtime_constraints { assert_eq!( result.unwrap_err().unwrap(), TransactionError::InstructionError( @@ -3824,7 +3824,7 @@ fn test_cpi_account_ownership_writability() { ) ); } else { - // without direct mapping, changes to the realloc padding + // without stricter_abi_and_runtime_constraints, changes to the realloc padding // outside the account length are ignored assert!(result.is_ok(), "{result:?}"); } @@ -3832,11 +3832,11 @@ fn test_cpi_account_ownership_writability() { } // Test that the CPI code that updates `ref_to_len_in_vm` fails if we // make it write to an invalid location. This is the first variant which - // correctly triggers ExternalAccountDataModified when direct mapping is - // disabled. When direct mapping is enabled this tests fails early + // correctly triggers ExternalAccountDataModified when stricter_abi_and_runtime_constraints is + // disabled. When stricter_abi_and_runtime_constraints is enabled this tests fails early // because we move the account data pointer. // TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE is able to make more - // progress when direct mapping is on. + // progress when stricter_abi_and_runtime_constraints is on. let account = AccountSharedData::new(42, 0, &invoke_program_id); bank.store_account(&account_keypair.pubkey(), &account); let instruction_data = vec![ @@ -3853,8 +3853,8 @@ fn test_cpi_account_ownership_writability() { let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); assert_eq!( result.unwrap_err().unwrap(), - if direct_mapping { - // We move the data pointer, direct mapping doesn't allow it + if stricter_abi_and_runtime_constraints { + // We move the data pointer, stricter_abi_and_runtime_constraints doesn't allow it // anymore so it errors out earlier. See // test_cpi_invalid_account_info_pointers. TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete) @@ -3873,7 +3873,7 @@ fn test_cpi_account_ownership_writability() { for target_account in [1, account_metas.len() as u8 - 1] { // Similar to the test above where we try to make CPI write into account - // data. This variant is for when direct mapping is enabled. + // data. This variant is for when stricter_abi_and_runtime_constraints is enabled. let account = AccountSharedData::new(42, 0, &invoke_program_id); bank.store_account(&account_keypair.pubkey(), &account); let account = AccountSharedData::new(42, 0, &invoke_program_id); @@ -3892,7 +3892,7 @@ fn test_cpi_account_ownership_writability() { let message = Message::new(&[instruction], Some(&mint_pubkey)); let tx = Transaction::new(&[&mint_keypair], message.clone(), bank.last_blockhash()); let (result, _, logs, _) = process_transaction_and_record_inner(&bank, tx); - if direct_mapping { + if stricter_abi_and_runtime_constraints { assert_eq!( result.unwrap_err(), TransactionError::InstructionError( @@ -3934,7 +3934,7 @@ fn test_cpi_account_ownership_writability() { fn test_cpi_account_data_updates() { solana_logger::setup(); - for (deprecated_callee, deprecated_caller, direct_mapping) in + for (deprecated_callee, deprecated_caller, stricter_abi_and_runtime_constraints) in [false, true].into_iter().flat_map(move |z| { [false, true] .into_iter() @@ -3948,8 +3948,8 @@ fn test_cpi_account_data_updates() { } = create_genesis_config(100_123_456_789); let mut bank = Bank::new_for_tests(&genesis_config); let mut feature_set = FeatureSet::all_enabled(); - if !direct_mapping { - feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } bank.feature_set = Arc::new(feature_set); @@ -4019,7 +4019,7 @@ fn test_cpi_account_data_updates() { result.unwrap_err().unwrap(), TransactionError::InstructionError( 0, - if direct_mapping { + if stricter_abi_and_runtime_constraints { InstructionError::ProgramFailedToComplete } else { InstructionError::ModifiedProgramId @@ -4058,7 +4058,7 @@ fn test_cpi_account_data_updates() { result.unwrap_err().unwrap(), TransactionError::InstructionError( 0, - if direct_mapping { + if stricter_abi_and_runtime_constraints { InstructionError::InvalidRealloc } else { InstructionError::AccountDataSizeChanged @@ -4082,7 +4082,7 @@ fn test_cpi_account_data_updates() { bank.store_account(&account_keypair.pubkey(), &account); let mut instruction_data = vec![ TEST_CPI_ACCOUNT_UPDATE_CALLEE_SHRINKS_SMALLER_THAN_ORIGINAL_LEN, - direct_mapping as u8, + stricter_abi_and_runtime_constraints as u8, ]; instruction_data.extend_from_slice(4usize.to_le_bytes().as_ref()); let instruction = Instruction::new_with_bytes( @@ -4101,7 +4101,7 @@ fn test_cpi_account_data_updates() { result.unwrap_err().unwrap(), TransactionError::InstructionError( 0, - if direct_mapping && deprecated_callee { + if stricter_abi_and_runtime_constraints && deprecated_callee { InstructionError::InvalidRealloc } else { InstructionError::AccountDataSizeChanged @@ -4124,7 +4124,7 @@ fn test_cpi_account_data_updates() { bank.store_account(&account_keypair.pubkey(), &account); let mut instruction_data = vec![ TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS, - direct_mapping as u8, + stricter_abi_and_runtime_constraints as u8, ]; // realloc to "foobazbad" then shrink to "foobazb" instruction_data.extend_from_slice(7usize.to_le_bytes().as_ref()); @@ -4140,7 +4140,7 @@ fn test_cpi_account_data_updates() { result.unwrap_err().unwrap(), TransactionError::InstructionError( 0, - if direct_mapping { + if stricter_abi_and_runtime_constraints { InstructionError::ProgramFailedToComplete } else { InstructionError::ModifiedProgramId @@ -4161,7 +4161,7 @@ fn test_cpi_account_data_updates() { bank.store_account(&account_keypair.pubkey(), &account); let mut instruction_data = vec![ TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS, - direct_mapping as u8, + stricter_abi_and_runtime_constraints as u8, ]; // realloc to "foobazbad" then shrink to "f" instruction_data.extend_from_slice(1usize.to_le_bytes().as_ref()); @@ -4177,7 +4177,7 @@ fn test_cpi_account_data_updates() { result.unwrap_err().unwrap(), TransactionError::InstructionError( 0, - if direct_mapping { + if stricter_abi_and_runtime_constraints { InstructionError::ProgramFailedToComplete } else { InstructionError::ModifiedProgramId @@ -4394,13 +4394,13 @@ fn test_deny_access_beyond_current_length() { .. } = create_genesis_config(100_123_456_789); - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints 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()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let mut bank_client = BankClient::new_shared(bank); @@ -4439,7 +4439,7 @@ fn test_deny_access_beyond_current_length() { account_metas.clone(), ); let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); - if direct_mapping { + if stricter_abi_and_runtime_constraints { assert_eq!( result.unwrap_err().unwrap(), TransactionError::InstructionError(0, expected_error) @@ -4462,13 +4462,13 @@ fn test_deny_executable_write() { .. } = create_genesis_config(100_123_456_789); - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints 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()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let mut bank_client = BankClient::new_shared(bank); @@ -4517,13 +4517,13 @@ fn test_update_callee_account() { .. } = create_genesis_config(100_123_456_789); - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints 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()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let mut bank_client = BankClient::new_shared(bank.clone()); @@ -4547,7 +4547,7 @@ fn test_update_callee_account() { AccountMeta::new_readonly(invoke_program_id, false), ]; - // I. do CPI with account in read only (separate code path with direct mapping) + // I. do CPI with account in read only (separate code path with stricter_abi_and_runtime_constraints) let mut account = AccountSharedData::new(42, 10240, &invoke_program_id); let data: Vec = (0..10240).map(|n| n as u8).collect(); account.set_data(data); @@ -4753,7 +4753,7 @@ fn test_update_callee_account() { ); let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); - if direct_mapping { + if stricter_abi_and_runtime_constraints { // changing the data pointer is not permitted assert!(result.is_err()); } else { @@ -4801,13 +4801,13 @@ fn test_account_info_in_account() { } for program in programs { - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints 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()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); @@ -4843,7 +4843,7 @@ fn test_account_info_in_account() { bank.store_account(&account_keypair.pubkey(), &account); let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); - if direct_mapping { + if stricter_abi_and_runtime_constraints { assert!(result.is_err()); } else { assert!(result.is_ok()); @@ -4862,13 +4862,13 @@ fn test_account_info_rc_in_account() { .. } = create_genesis_config(100_123_456_789); - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints 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()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); @@ -4909,7 +4909,7 @@ fn test_account_info_rc_in_account() { let tx = Transaction::new(&[&mint_keypair], message.clone(), bank.last_blockhash()); let (result, _, logs, _) = process_transaction_and_record_inner(&bank, tx); - if direct_mapping { + if stricter_abi_and_runtime_constraints { assert!( logs.last().unwrap().ends_with(" failed: Invalid pointer"), "{logs:?}" @@ -4932,7 +4932,7 @@ fn test_account_info_rc_in_account() { let tx = Transaction::new(&[&mint_keypair], message.clone(), bank.last_blockhash()); let (result, _, logs, _) = process_transaction_and_record_inner(&bank, tx); - if direct_mapping { + if stricter_abi_and_runtime_constraints { assert!( logs.last().unwrap().ends_with(" failed: Invalid pointer"), "{logs:?}" @@ -4958,7 +4958,7 @@ fn test_clone_account_data() { let mut bank = Bank::new_for_tests(&genesis_config); let feature_set = Arc::make_mut(&mut bank.feature_set); - feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let mut bank_client = BankClient::new_shared(bank.clone()); @@ -5291,7 +5291,7 @@ fn test_function_call_args() { fn test_mem_syscalls_overlap_account_begin_or_end() { solana_logger::setup(); - for direct_mapping in [false, true] { + for stricter_abi_and_runtime_constraints in [false, true] { let GenesisConfigInfo { genesis_config, mint_keypair, @@ -5300,8 +5300,8 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { let mut bank = Bank::new_for_tests(&genesis_config); let mut feature_set = FeatureSet::all_enabled(); - if !direct_mapping { - feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + if !stricter_abi_and_runtime_constraints { + feature_set.deactivate(&feature_set::stricter_abi_and_runtime_constraints::id()); } let account_keypair = Keypair::new(); @@ -5344,7 +5344,7 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { bank.store_account(&account_keypair.pubkey(), &account); for instr in 0..=15 { - println!("Testing deprecated:{deprecated} direct_mapping:{direct_mapping} instruction:{instr}"); + println!("Testing deprecated:{deprecated} stricter_abi_and_runtime_constraints:{stricter_abi_and_runtime_constraints} instruction:{instr}"); let instruction = Instruction::new_with_bytes(program_id, &[instr], account_metas.clone()); @@ -5353,7 +5353,7 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { let (result, _, logs, _) = process_transaction_and_record_inner(&bank, tx); let last_line = logs.last().unwrap(); - if direct_mapping { + if stricter_abi_and_runtime_constraints { assert!(last_line.contains(" failed: Access violation"), "{logs:?}"); } else { assert!(result.is_ok(), "{logs:?}"); @@ -5364,7 +5364,7 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { bank.store_account(&account_keypair.pubkey(), &account); for instr in 0..=15 { - println!("Testing deprecated:{deprecated} direct_mapping:{direct_mapping} instruction:{instr} zero-length account"); + println!("Testing deprecated:{deprecated} stricter_abi_and_runtime_constraints:{stricter_abi_and_runtime_constraints} instruction:{instr} zero-length account"); let instruction = Instruction::new_with_bytes(program_id, &[instr, 0], account_metas.clone()); @@ -5373,7 +5373,7 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { let (result, _, logs, _) = process_transaction_and_record_inner(&bank, tx); let last_line = logs.last().unwrap(); - if direct_mapping && (!deprecated || instr < 8) { + if stricter_abi_and_runtime_constraints && (!deprecated || instr < 8) { assert!( last_line.contains(" failed: account data too small") || last_line.contains(" failed: Failed to reallocate account data") @@ -5381,7 +5381,7 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { "{logs:?}", ); } else { - // direct_mapping && deprecated && instr >= 8 succeeds with zero-length accounts + // stricter_abi_and_runtime_constraints && deprecated && instr >= 8 succeeds with zero-length accounts // because there is no MemoryRegion for the account, // so there can be no error when leaving that non-existent region. assert!(result.is_ok(), "{logs:?}"); diff --git a/svm-feature-set/src/lib.rs b/svm-feature-set/src/lib.rs index 425d0f2ed139a8..d2046761869e83 100644 --- a/svm-feature-set/src/lib.rs +++ b/svm-feature-set/src/lib.rs @@ -2,7 +2,7 @@ pub struct SVMFeatureSet { pub move_precompile_verification_to_svm: bool, pub remove_accounts_executable_flag_checks: bool, - pub bpf_account_data_direct_mapping: bool, + pub stricter_abi_and_runtime_constraints: bool, pub enable_bpf_loader_set_authority_checked_ix: bool, pub enable_loader_v4: bool, pub deplete_cu_meter_on_vm_failure: bool, @@ -45,7 +45,7 @@ impl SVMFeatureSet { Self { move_precompile_verification_to_svm: true, remove_accounts_executable_flag_checks: true, - bpf_account_data_direct_mapping: true, + stricter_abi_and_runtime_constraints: true, enable_bpf_loader_set_authority_checked_ix: true, enable_loader_v4: true, deplete_cu_meter_on_vm_failure: true, diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index c79c27d642dd83..a15c55ebf34941 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -24,7 +24,7 @@ use { solana_nonce::{self as nonce, state::DurableNonce}, solana_program_entrypoint::MAX_PERMITTED_DATA_INCREASE, solana_program_runtime::execution_budget::SVMTransactionExecutionAndFeeBudgetLimits, - solana_pubkey::{pubkey, Pubkey}, + solana_pubkey::Pubkey, solana_sdk_ids::{bpf_loader_upgradeable, native_loader}, solana_signer::Signer, solana_svm::{ @@ -296,7 +296,7 @@ impl SvmTestEnvironment<'_> { } // container for a transaction batch and all data needed to run and verify it against svm -#[derive(Clone, Debug)] +#[derive(Clone, Default, Debug)] pub struct SvmTestEntry { // features are enabled by default; these will be disabled pub disabled_features: Vec, @@ -497,21 +497,6 @@ impl SvmTestEntry { } } -// NOTE `1ncomp1ete111111111111111111111111111111111` corresponds to `bpf_account_data_direct_mapping::id()` -// by hardcoding the string, we ensure when the feature is finished, it will automatically be tested -impl Default for SvmTestEntry { - fn default() -> Self { - Self { - disabled_features: vec![pubkey!("1ncomp1ete111111111111111111111111111111111")], - with_loader_v4: false, - initial_programs: vec![], - initial_accounts: AccountsMap::default(), - transaction_batch: vec![], - final_accounts: AccountsMap::default(), - } - } -} - // one transaction in a batch plus check results for svm and asserts for tests #[derive(Clone, Debug)] pub struct TransactionBatchItem { diff --git a/syscalls/src/cpi.rs b/syscalls/src/cpi.rs index e1e6a895b2062e..eb7836940483af 100644 --- a/syscalls/src/cpi.rs +++ b/syscalls/src/cpi.rs @@ -5,7 +5,8 @@ use { solana_loader_v3_interface::instruction as bpf_loader_upgradeable, solana_measure::measure::Measure, solana_program_runtime::{ - invoke_context::SerializedAccountMetadata, serialization::create_memory_region_of_account, + invoke_context::SerializedAccountMetadata, + serialization::{create_memory_region_of_account, modify_memory_region_of_account}, }, solana_sbpf::ebpf, solana_stable_layout::stable_instruction::StableInstruction, @@ -77,8 +78,7 @@ struct CallerAccount<'a> { // mapped inside the vm (see serialize_parameters() in // BpfExecutor::execute). // - // This is only set when direct mapping is off (see the relevant comment in - // CallerAccount::from_account_info). + // This is only set when account_data_direct_mapping is off. 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. @@ -87,6 +87,41 @@ struct CallerAccount<'a> { } impl<'a> CallerAccount<'a> { + fn get_serialized_data( + memory_mapping: &MemoryMapping<'_>, + vm_addr: u64, + len: u64, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, + ) -> Result<&'a mut [u8], Error> { + if stricter_abi_and_runtime_constraints && account_data_direct_mapping { + Ok(&mut []) + } else if stricter_abi_and_runtime_constraints { + // Workaround the memory permissions (as these are from the PoV of being inside the VM) + let serialization_ptr = translate_slice_mut::( + memory_mapping, + solana_sbpf::ebpf::MM_INPUT_START, + 1, + false, // Don't care since it is byte aligned + )? + .as_mut_ptr(); + unsafe { + Ok(std::slice::from_raw_parts_mut( + serialization_ptr + .add(vm_addr.saturating_sub(solana_sbpf::ebpf::MM_INPUT_START) as usize), + len as usize, + )) + } + } else { + translate_slice_mut::( + memory_mapping, + vm_addr, + len, + false, // Don't care since it is byte aligned + ) + } + } + // Create a CallerAccount given an AccountInfo. fn from_account_info( invoke_context: &InvokeContext, @@ -96,11 +131,11 @@ impl<'a> CallerAccount<'a> { account_info: &AccountInfo, account_metadata: &SerializedAccountMetadata, ) -> Result, Error> { - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; - if direct_mapping { + if stricter_abi_and_runtime_constraints { check_account_info_pointer( invoke_context, account_info.key as *const _ as u64, @@ -124,7 +159,7 @@ impl<'a> CallerAccount<'a> { account_info.lamports.as_ptr() as u64, check_aligned, )?; - if direct_mapping { + if stricter_abi_and_runtime_constraints { if account_info.lamports.as_ptr() as u64 >= ebpf::MM_INPUT_START { return Err(SyscallError::InvalidPointer.into()); } @@ -146,7 +181,9 @@ impl<'a> CallerAccount<'a> { )?; let (serialized_data, vm_data_addr, ref_to_len_in_vm) = { - if direct_mapping && account_info.data.as_ptr() as u64 >= ebpf::MM_INPUT_START { + if stricter_abi_and_runtime_constraints + && account_info.data.as_ptr() as u64 >= ebpf::MM_INPUT_START + { return Err(SyscallError::InvalidPointer.into()); } @@ -156,7 +193,7 @@ impl<'a> CallerAccount<'a> { account_info.data.as_ptr() as *const _ as u64, check_aligned, )?; - if direct_mapping { + if stricter_abi_and_runtime_constraints { check_account_info_pointer( invoke_context, data.as_ptr() as u64, @@ -174,7 +211,7 @@ impl<'a> CallerAccount<'a> { let vm_len_addr = (account_info.data.as_ptr() as *const u64 as u64) .saturating_add(size_of::() as u64); - if direct_mapping { + if stricter_abi_and_runtime_constraints { // In the same vein as the other check_account_info_pointer() checks, we don't lock // this pointer to a specific address but we don't want it to be inside accounts, or // callees might be able to write to the pointed memory. @@ -184,29 +221,13 @@ impl<'a> CallerAccount<'a> { } let ref_to_len_in_vm = translate_type_mut::(memory_mapping, vm_len_addr, false)?; let vm_data_addr = data.as_ptr() as u64; - - let serialized_data = if direct_mapping { - // when direct mapping is enabled, the permissions on the - // realloc region can change during CPI so we must delay - // translating until when we know whether we're going to mutate - // the realloc region or not. Consider this case: - // - // [caller can't write to an account] <- we are here - // [callee grows and assigns account to the caller] - // [caller can now write to the account] - // - // If we always translated the realloc area here, we'd get a - // memory access violation since we can't write to the account - // _yet_, but we will be able to once the caller returns. - &mut [] - } else { - translate_slice_mut::( - memory_mapping, - vm_data_addr, - data.len() as u64, - check_aligned, - )? - }; + let serialized_data = CallerAccount::get_serialized_data( + memory_mapping, + vm_data_addr, + data.len() as u64, + stricter_abi_and_runtime_constraints, + invoke_context.account_data_direct_mapping, + )?; (serialized_data, vm_data_addr, ref_to_len_in_vm) }; @@ -229,11 +250,11 @@ impl<'a> CallerAccount<'a> { account_info: &SolAccountInfo, account_metadata: &SerializedAccountMetadata, ) -> Result, Error> { - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; - if direct_mapping { + if stricter_abi_and_runtime_constraints { check_account_info_pointer( invoke_context, account_info.key_addr, @@ -278,17 +299,13 @@ impl<'a> CallerAccount<'a> { .unwrap_or(u64::MAX), )?; - let serialized_data = if direct_mapping { - // See comment in CallerAccount::from_account_info() - &mut [] - } else { - translate_slice_mut::( - memory_mapping, - account_info.data_addr, - account_info.data_len, - check_aligned, - )? - }; + let serialized_data = CallerAccount::get_serialized_data( + memory_mapping, + account_info.data_addr, + account_info.data_len, + stricter_abi_and_runtime_constraints, + invoke_context.account_data_direct_mapping, + )?; // we already have the host addr we want: &mut account_info.data_len. // The account info might be read only in the vm though, so we translate @@ -715,14 +732,14 @@ fn translate_account_infos<'a, T, F>( where F: Fn(&T) -> u64, { - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; // In the same vein as the other check_account_info_pointer() checks, we don't lock // this pointer to a specific address but we don't want it to be inside accounts, or // callees might be able to write to the pointed memory. - if direct_mapping + if stricter_abi_and_runtime_constraints && account_infos_addr .saturating_add(account_infos_len.saturating_mul(std::mem::size_of::() as u64)) >= ebpf::MM_INPUT_START @@ -786,9 +803,9 @@ where .unwrap() .accounts_metadata; - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; for (instruction_account_index, instruction_account) in next_instruction_accounts.iter().enumerate() @@ -851,10 +868,11 @@ where // BorrowedAccount (callee_account) so the callee can see the // changes. let update_caller = update_callee_account( + check_aligned, &caller_account, callee_account, - direct_mapping, - check_aligned, + stricter_abi_and_runtime_constraints, + invoke_context.account_data_direct_mapping, )?; accounts.push(TranslatedAccount { @@ -1039,9 +1057,9 @@ fn cpi_common( // CPI exit. // // Synchronize the callee's account changes so the caller can see them. - let direct_mapping = invoke_context + let stricter_abi_and_runtime_constraints = invoke_context .get_feature_set() - .bpf_account_data_direct_mapping; + .stricter_abi_and_runtime_constraints; for translate_account in accounts.iter_mut() { let mut callee_account = instruction_context.try_borrow_instruction_account( @@ -1055,12 +1073,12 @@ fn cpi_common( check_aligned, &mut translate_account.caller_account, &mut callee_account, - direct_mapping, + stricter_abi_and_runtime_constraints, )?; } } - if direct_mapping { + if stricter_abi_and_runtime_constraints { for translate_account in accounts.iter() { let mut callee_account = instruction_context.try_borrow_instruction_account( transaction_context, @@ -1072,6 +1090,7 @@ fn cpi_common( check_aligned, &translate_account.caller_account, &mut callee_account, + invoke_context.account_data_direct_mapping, )?; } } @@ -1091,12 +1110,13 @@ fn cpi_common( // changes. // // When true is returned, the caller account must be updated after CPI. This -// is only set for direct mapping when the pointer may have changed. +// is only set for stricter_abi_and_runtime_constraints when the pointer may have changed. fn update_callee_account( + check_aligned: bool, caller_account: &CallerAccount, mut callee_account: BorrowedAccount<'_>, - direct_mapping: bool, - check_aligned: bool, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, ) -> Result { let mut must_update_caller = false; @@ -1104,7 +1124,7 @@ fn update_callee_account( callee_account.set_lamports(*caller_account.lamports)?; } - if direct_mapping { + if stricter_abi_and_runtime_constraints { let prev_len = callee_account.get_data().len(); let post_len = *caller_account.ref_to_len_in_vm as usize; if prev_len != post_len { @@ -1123,6 +1143,9 @@ fn update_callee_account( // pointer to data may have changed, so caller must be updated must_update_caller = true; } + if !account_data_direct_mapping && callee_account.can_data_be_changed().is_ok() { + callee_account.set_data_from_slice(caller_account.serialized_data)?; + } } 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()) { @@ -1149,6 +1172,7 @@ fn update_caller_account_region( check_aligned: bool, caller_account: &CallerAccount, callee_account: &mut BorrowedAccount<'_>, + account_data_direct_mapping: bool, ) -> Result<(), Error> { let is_caller_loader_deprecated = !check_aligned; let address_space_reserved_for_account = if is_caller_loader_deprecated { @@ -1167,7 +1191,13 @@ fn update_caller_account_region( .ok_or_else(|| Box::new(InstructionError::MissingAccount))?; // vm_data_addr must always point to the beginning of the region debug_assert_eq!(region.vm_addr, caller_account.vm_data_addr); - let new_region = create_memory_region_of_account(callee_account, region.vm_addr)?; + let mut new_region; + if !account_data_direct_mapping { + new_region = region.clone(); + modify_memory_region_of_account(callee_account, &mut new_region); + } else { + new_region = create_memory_region_of_account(callee_account, region.vm_addr)?; + } memory_mapping.replace_region(region_index, new_region)?; } @@ -1183,7 +1213,7 @@ fn update_caller_account_region( // This method updates caller_account so the CPI caller can see the callee's // changes. // -// Safety: Once `direct_mapping` is enabled all fields of [CallerAccount] used +// Safety: Once `stricter_abi_and_runtime_constraints` is enabled all fields of [CallerAccount] used // in this function should never point inside the address space reserved for // accounts (regardless of the current size of an account). fn update_caller_account( @@ -1192,7 +1222,7 @@ fn update_caller_account( check_aligned: bool, caller_account: &mut CallerAccount<'_>, callee_account: &mut BorrowedAccount<'_>, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, ) -> Result<(), Error> { *caller_account.lamports = callee_account.get_lamports(); *caller_account.owner = *callee_account.get_owner(); @@ -1200,15 +1230,18 @@ fn update_caller_account( let prev_len = *caller_account.ref_to_len_in_vm as usize; let post_len = callee_account.get_data().len(); let is_caller_loader_deprecated = !check_aligned; - let address_space_reserved_for_account = if direct_mapping && is_caller_loader_deprecated { - caller_account.original_data_len - } else { - caller_account - .original_data_len - .saturating_add(MAX_PERMITTED_DATA_INCREASE) - }; + let address_space_reserved_for_account = + if stricter_abi_and_runtime_constraints && is_caller_loader_deprecated { + caller_account.original_data_len + } else { + caller_account + .original_data_len + .saturating_add(MAX_PERMITTED_DATA_INCREASE) + }; - if post_len > address_space_reserved_for_account && (direct_mapping || prev_len != post_len) { + if post_len > address_space_reserved_for_account + && (stricter_abi_and_runtime_constraints || prev_len != post_len) + { let max_increase = address_space_reserved_for_account.saturating_sub(caller_account.original_data_len); ic_msg!( @@ -1219,9 +1252,9 @@ fn update_caller_account( } if prev_len != post_len { - // when direct mapping is enabled we don't cache the serialized data in + // when stricter_abi_and_runtime_constraints is enabled we don't cache the serialized data in // caller_account.serialized_data. See CallerAccount::from_account_info. - if !direct_mapping { + if !(stricter_abi_and_runtime_constraints && invoke_context.account_data_direct_mapping) { // If the account has been shrunk, we're going to zero the unused memory // *that was previously used*. if post_len < prev_len { @@ -1232,11 +1265,12 @@ fn update_caller_account( .fill(0); } // Set the length of caller_account.serialized_data to post_len. - caller_account.serialized_data = translate_slice_mut::( + caller_account.serialized_data = CallerAccount::get_serialized_data( memory_mapping, caller_account.vm_data_addr, post_len as u64, - false, // Don't care since it is byte aligned + stricter_abi_and_runtime_constraints, + invoke_context.account_data_direct_mapping, )?; } // this is the len field in the AccountInfo::data slice @@ -1253,7 +1287,7 @@ fn update_caller_account( *serialized_len_ptr = post_len as u64; } - if !direct_mapping { + if !(stricter_abi_and_runtime_constraints && invoke_context.account_data_direct_mapping) { // Propagate changes in the callee up to the caller. let to_slice = &mut caller_account.serialized_data; let from_slice = callee_account @@ -1322,7 +1356,7 @@ mod tests { .map(|a| (a.0, a.1)) .collect::>(); let mut feature_set = SVMFeatureSet::all_enabled(); - feature_set.bpf_account_data_direct_mapping = false; + feature_set.stricter_abi_and_runtime_constraints = false; let feature_set = &feature_set; with_mock_invoke_context_with_feature_set!( $invoke_context, @@ -1480,7 +1514,7 @@ mod tests { } #[test_matrix([false, true])] - fn test_update_caller_account_lamports_owner(direct_mapping: bool) { + fn test_update_caller_account_lamports_owner(stricter_abi_and_runtime_constraints: bool) { let transaction_accounts = transaction_with_one_writable_instruction_account(vec![]); let account = transaction_accounts[1].1.clone(); mock_invoke_context!( @@ -1521,7 +1555,7 @@ mod tests { true, // check_aligned &mut caller_account, &mut callee_account, - direct_mapping, + stricter_abi_and_runtime_constraints, ) .unwrap(); @@ -1650,7 +1684,7 @@ mod tests { } #[test_matrix([false, true])] - fn test_update_callee_account_lamports_owner(direct_mapping: bool) { + fn test_update_callee_account_lamports_owner(stricter_abi_and_runtime_constraints: bool) { let transaction_accounts = transaction_with_one_writable_instruction_account(vec![]); let account = transaction_accounts[1].1.clone(); @@ -1673,7 +1707,14 @@ mod tests { *caller_account.lamports = 42; *caller_account.owner = Pubkey::new_unique(); - update_callee_account(&caller_account, callee_account, direct_mapping, true).unwrap(); + update_callee_account( + true, // check_aligned + &caller_account, + callee_account, + stricter_abi_and_runtime_constraints, + true, // account_data_direct_mapping + ) + .unwrap(); let callee_account = borrow_instruction_account!(invoke_context, 0); assert_eq!(callee_account.get_lamports(), 42); @@ -1681,7 +1722,7 @@ mod tests { } #[test_matrix([false, true])] - fn test_update_callee_account_data_writable(direct_mapping: bool) { + fn test_update_callee_account_data_writable(stricter_abi_and_runtime_constraints: bool) { let transaction_accounts = transaction_with_one_writable_instruction_account(b"foobar".to_vec()); let account = transaction_accounts[1].1.clone(); @@ -1701,9 +1742,16 @@ mod tests { let mut caller_account = mock_caller_account.caller_account(); let callee_account = borrow_instruction_account!(invoke_context, 0); - // direct mapping does not copy data in update_callee_account() + // stricter_abi_and_runtime_constraints does not copy data in update_callee_account() caller_account.serialized_data[0] = b'b'; - update_callee_account(&caller_account, callee_account, false, true).unwrap(); + update_callee_account( + true, // check_aligned + &caller_account, + callee_account, + false, // stricter_abi_and_runtime_constraints + false, // account_data_direct_mapping + ) + .unwrap(); let callee_account = borrow_instruction_account!(invoke_context, 0); assert_eq!(callee_account.get_data(), b"boobar"); @@ -1712,8 +1760,15 @@ mod tests { *caller_account.ref_to_len_in_vm = data.len() as u64; caller_account.serialized_data = &mut data; assert_eq!( - update_callee_account(&caller_account, callee_account, direct_mapping, true).unwrap(), - direct_mapping, + update_callee_account( + true, // check_aligned + &caller_account, + callee_account, + stricter_abi_and_runtime_constraints, + true, // account_data_direct_mapping + ) + .unwrap(), + stricter_abi_and_runtime_constraints, ); // truncating resize @@ -1722,8 +1777,15 @@ mod tests { caller_account.serialized_data = &mut data; let callee_account = borrow_instruction_account!(invoke_context, 0); assert_eq!( - update_callee_account(&caller_account, callee_account, direct_mapping, true).unwrap(), - direct_mapping, + update_callee_account( + true, // check_aligned + &caller_account, + callee_account, + stricter_abi_and_runtime_constraints, + true, // account_data_direct_mapping + ) + .unwrap(), + stricter_abi_and_runtime_constraints, ); // close the account @@ -1733,14 +1795,27 @@ mod tests { let mut owner = system_program::id(); caller_account.owner = &mut owner; let callee_account = borrow_instruction_account!(invoke_context, 0); - update_callee_account(&caller_account, callee_account, direct_mapping, true).unwrap(); + update_callee_account( + true, // check_aligned + &caller_account, + callee_account, + stricter_abi_and_runtime_constraints, + true, // account_data_direct_mapping + ) + .unwrap(); let callee_account = borrow_instruction_account!(invoke_context, 0); assert_eq!(callee_account.get_data(), b""); // growing beyond address_space_reserved_for_account *caller_account.ref_to_len_in_vm = (7 + MAX_PERMITTED_DATA_INCREASE) as u64; - let result = update_callee_account(&caller_account, callee_account, direct_mapping, true); - if direct_mapping { + let result = update_callee_account( + true, // check_aligned + &caller_account, + callee_account, + stricter_abi_and_runtime_constraints, + true, // account_data_direct_mapping + ); + if stricter_abi_and_runtime_constraints { assert_matches!( result, Err(error) if error.downcast_ref::().unwrap() == &InstructionError::InvalidRealloc @@ -1751,7 +1826,7 @@ mod tests { } #[test_matrix([false, true])] - fn test_update_callee_account_data_readonly(direct_mapping: bool) { + fn test_update_callee_account_data_readonly(stricter_abi_and_runtime_constraints: bool) { let transaction_accounts = transaction_with_one_readonly_instruction_account(b"foobar".to_vec()); let account = transaction_accounts[1].1.clone(); @@ -1770,14 +1845,15 @@ mod tests { let mut caller_account = mock_caller_account.caller_account(); let callee_account = borrow_instruction_account!(invoke_context, 0); - // direct mapping does not copy data in update_callee_account() + // stricter_abi_and_runtime_constraints does not copy data in update_callee_account() caller_account.serialized_data[0] = b'b'; assert_matches!( update_callee_account( + true, // check_aligned &caller_account, callee_account, - false, - true, + false, // stricter_abi_and_runtime_constraints + false, // account_data_direct_mapping ), Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ExternalAccountDataModified ); @@ -1789,10 +1865,11 @@ mod tests { let callee_account = borrow_instruction_account!(invoke_context, 0); assert_matches!( update_callee_account( + true, // check_aligned &caller_account, callee_account, - direct_mapping, - true, + stricter_abi_and_runtime_constraints, + true, // account_data_direct_mapping ), Err(error) if error.downcast_ref::().unwrap() == &InstructionError::AccountDataSizeChanged ); @@ -1804,10 +1881,11 @@ mod tests { let callee_account = borrow_instruction_account!(invoke_context, 0); assert_matches!( update_callee_account( + true, // check_aligned &caller_account, callee_account, - direct_mapping, - true, + stricter_abi_and_runtime_constraints, + true, // account_data_direct_mapping ), Err(error) if error.downcast_ref::().unwrap() == &InstructionError::AccountDataSizeChanged ); @@ -1882,7 +1960,7 @@ mod tests { data: Vec, len: u64, regions: Vec, - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, } impl MockCallerAccount { @@ -1890,12 +1968,12 @@ mod tests { lamports: u64, owner: Pubkey, data: &[u8], - direct_mapping: bool, + stricter_abi_and_runtime_constraints: bool, ) -> MockCallerAccount { let vm_addr = MM_INPUT_START; let mut region_addr = vm_addr; let region_len = mem::size_of::() - + if direct_mapping { + + if stricter_abi_and_runtime_constraints { 0 } else { data.len() + MAX_PERMITTED_DATA_INCREASE @@ -1903,19 +1981,19 @@ mod tests { let mut d = vec![0; region_len]; let mut regions = vec![]; - // always write the [len] part even when direct mapping + // always write the [len] part even when stricter_abi_and_runtime_constraints unsafe { ptr::write_unaligned::(d.as_mut_ptr().cast(), data.len() as u64) }; - // write the account data when not direct mapping - if !direct_mapping { + // write the account data when not stricter_abi_and_runtime_constraints + if !stricter_abi_and_runtime_constraints { d[mem::size_of::()..][..data.len()].copy_from_slice(data); } - // create a region for [len][data+realloc if !direct_mapping] + // create a region for [len][data+realloc if !stricter_abi_and_runtime_constraints] regions.push(MemoryRegion::new_writable(&mut d[..region_len], vm_addr)); region_addr += region_len as u64; - if direct_mapping { + if stricter_abi_and_runtime_constraints { // create a region for the directly mapped data regions.push(MemoryRegion::new_readonly(data, region_addr)); region_addr += data.len() as u64; @@ -1937,7 +2015,7 @@ mod tests { data: d, len: data.len() as u64, regions, - direct_mapping, + stricter_abi_and_runtime_constraints, } } @@ -1952,7 +2030,7 @@ mod tests { } fn caller_account(&mut self) -> CallerAccount { - let data = if self.direct_mapping { + let data = if self.stricter_abi_and_runtime_constraints { &mut [] } else { &mut self.data[mem::size_of::()..] diff --git a/syscalls/src/lib.rs b/syscalls/src/lib.rs index 70f32de4ae7d4e..91640df50477ec 100644 --- a/syscalls/src/lib.rs +++ b/syscalls/src/lib.rs @@ -345,7 +345,7 @@ pub fn create_program_runtime_environment_v1<'a>( sanitize_user_provided_values: true, enabled_sbpf_versions: min_sbpf_version..=max_sbpf_version, optimize_rodata: false, - aligned_memory_mapping: !feature_set.bpf_account_data_direct_mapping, + aligned_memory_mapping: !feature_set.stricter_abi_and_runtime_constraints, // Warning, do not use `Config::default()` so that configuration here is explicit. }; let mut result = BuiltinProgram::new_loader(config); diff --git a/transaction-context/src/lib.rs b/transaction-context/src/lib.rs index 623f0c845d83ef..c6a74463bf73b1 100644 --- a/transaction-context/src/lib.rs +++ b/transaction-context/src/lib.rs @@ -31,7 +31,7 @@ static_assertions::const_assert_eq!( #[cfg(not(target_os = "solana"))] const MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION: i64 = MAX_PERMITTED_DATA_LENGTH as i64 * 2; -// Note: With direct mapping programs can grow accounts faster than they intend to, +// Note: With stricter_abi_and_runtime_constraints programs can grow accounts faster than they intend to, // because the AccessViolationHandler might grow an account up to // MAX_PERMITTED_DATA_LENGTH at once. #[cfg(test)] @@ -526,7 +526,11 @@ impl TransactionContext { } /// Returns a new account data write access handler - pub fn access_violation_handler(&self) -> AccessViolationHandler { + pub fn access_violation_handler( + &self, + stricter_abi_and_runtime_constraints: bool, + account_data_direct_mapping: bool, + ) -> AccessViolationHandler { let accounts = Rc::clone(&self.accounts); Box::new( move |region: &mut MemoryRegion, @@ -595,8 +599,10 @@ impl TransactionContext { } // Potentially unshare / make the account shared data unique (CoW logic). - region.host_addr = account.data_as_mut_slice().as_mut_ptr() as u64; - region.writable = true; + if stricter_abi_and_runtime_constraints && account_data_direct_mapping { + region.host_addr = account.data_as_mut_slice().as_mut_ptr() as u64; + region.writable = true; + } }, ) }