diff --git a/programs/bpf_loader/src/syscalls/mem_ops.rs b/programs/bpf_loader/src/syscalls/mem_ops.rs index 2420d78b92d893..c89afd622db24a 100644 --- a/programs/bpf_loader/src/syscalls/mem_ops.rs +++ b/programs/bpf_loader/src/syscalls/mem_ops.rs @@ -490,18 +490,24 @@ impl<'a> Iterator for MemoryChunkIterator<'a> { let region_is_account; - let mut account_index = self.account_index.unwrap_or_default(); - self.account_index = Some(account_index); + let account_index = self.account_index.get_or_insert_default(); + // Do not allow iteration over account/non-account boundary. + + // First check whether the region is account data or not + // Note: that if an account has empty account data, it still has a resize area + // Note: for deprecated unaligned programs, there is no resize area an empty data accounts do NOT count as accounts loop { - if let Some(account) = self.accounts.get(account_index) { + if let Some(account) = self.accounts.get(*account_index) { + // region is either account data or 10k resize region let account_addr = account.vm_data_addr; let resize_addr = account_addr.saturating_add(account.original_data_len as u64); if resize_addr < region.vm_addr { - // region is after this account, move on next one - account_index = account_index.saturating_add(1); - self.account_index = Some(account_index); + // region is not this account, move onto the next account + + // note that we iterate in the same linear direction as regions + *account_index = account_index.saturating_add(1); } else { region_is_account = region.vm_addr == account_addr || region.vm_addr == resize_addr; @@ -556,25 +562,31 @@ impl DoubleEndedIterator for MemoryChunkIterator<'_> { let region_is_account; - let mut account_index = self + let account_index = self .account_index - .unwrap_or_else(|| self.accounts.len().saturating_sub(1)); - self.account_index = Some(account_index); + .get_or_insert_with(|| self.accounts.len().saturating_sub(1)); + + // Do not allow iteration over account/non-account boundary. + // First check whether the region is account data or not + // Note: that if an account has empty account data, it still has a resize area + // Note: for deprecated unaligned programs, there is no resize area an empty data accounts do NOT count as accounts loop { - let Some(account) = self.accounts.get(account_index) else { + let Some(account) = self.accounts.get(*account_index) else { // address is after all the accounts region_is_account = false; break; }; + // region is either account data or 10k resize region let account_addr = account.vm_data_addr; let resize_addr = account_addr.saturating_add(account.original_data_len as u64); - if account_index > 0 && account_addr > region.vm_addr { - account_index = account_index.saturating_sub(1); + if *account_index > 0 && account_addr > region.vm_addr { + // region is not this account, move onto the previous account - self.account_index = Some(account_index); + // note that we iterate in the same linear direction as regions + *account_index = account_index.saturating_sub(1); } else { region_is_account = region.vm_addr == account_addr || region.vm_addr == resize_addr; break; diff --git a/programs/sbf/rust/account_mem/src/lib.rs b/programs/sbf/rust/account_mem/src/lib.rs index fe343b17dd9bef..008155fa3b4aa6 100644 --- a/programs/sbf/rust/account_mem/src/lib.rs +++ b/programs/sbf/rust/account_mem/src/lib.rs @@ -105,7 +105,7 @@ pub fn process_instruction( sol_memcpy(too_early(3), &buf, 10); } 13 => { - // memmov dst overlaps begin of account + // memmove dst overlaps begin of account unsafe { sol_memmove(too_early(3).as_mut_ptr(), buf.as_ptr(), 10) }; } 14 => { @@ -122,6 +122,55 @@ pub fn process_instruction( ) }; } + 16 => { + // memmove at the lower boundary fails (by 1) - reverse + unsafe { sol_memmove(data.as_mut_ptr(), data.as_ptr().wrapping_sub(1), 100) }; + } + 17 => { + // memmove at the lower boundary fails (by 1) - forward + unsafe { sol_memmove(data.as_mut_ptr().wrapping_sub(1), data.as_ptr(), 100) }; + } + 18 => { + // memmove at the higher account boundary fails (by 1) - forward + unsafe { + sol_memmove( + data.as_mut_ptr().wrapping_add(data_len.wrapping_sub(100)), + data.as_ptr().wrapping_add(data_len.wrapping_sub(99)), + 100, + ) + }; + } + 19 => { + // memmove at the higher account boundary fails (by 1) - reverse + unsafe { + sol_memmove( + data.as_mut_ptr().wrapping_add(data_len.wrapping_sub(99)), + data.as_ptr().wrapping_add(data_len.wrapping_sub(100)), + 100, + ) + }; + } + 100 => { + // memmove at the lower boundary succeeds + unsafe { sol_memmove(data.as_mut_ptr().wrapping_add(50), data.as_ptr(), 100) }; + + // memmove at the higher boundary succeeds + unsafe { + sol_memmove( + data.as_mut_ptr().wrapping_add(data_len.wrapping_sub(101)), + data.as_ptr().wrapping_sub(data_len.wrapping_sub(100)), + 100, + ) + }; + + unsafe { + sol_memmove( + data.as_mut_ptr().wrapping_add(data_len.wrapping_sub(100)), + data.as_ptr().wrapping_sub(data_len.wrapping_sub(101)), + 100, + ) + }; + } _ => {} } diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 52fb4efab0759b..65c9bf958b143e 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -5179,7 +5179,7 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { let account = AccountSharedData::new(42, 1024, &program_id); bank.store_account(&account_keypair.pubkey(), &account); - for instr in 0..=15 { + for instr in 0..=19 { println!("Testing direct_mapping:{direct_mapping} instruction:{instr}"); let instruction = Instruction::new_with_bytes(program_id, &[instr], account_metas.clone()); @@ -5195,5 +5195,15 @@ fn test_mem_syscalls_overlap_account_begin_or_end() { assert!(!logs.last().unwrap().ends_with(" failed: InvalidLength")); } } + + // test success + println!("Testing direct_mapping:{direct_mapping} success"); + let instruction = Instruction::new_with_bytes(program_id, &[199], account_metas.clone()); + + 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); + + assert!(result.is_ok(), "{logs:?}"); } }