Bump SBPF to v0.11.0 (#5839)

* Bump to SBPF v0.11.0

* Cleanup - EbpfError::InvalidVirtualAddress

* Refactor - Return region index

* Refactor - Simplify CoW in MemoryMapping

* Inlines Serializer::push_account_data_region().

Author: Lichtso

Cargo.lock

@@ -527,7 +527,7 @@ solana-rpc-client-types = { path = "rpc-client-types", version = "=2.3.0" }
 solana-rpc-client-nonce-utils = { path = "rpc-client-nonce-utils", version = "=2.3.0" }
 solana-runtime = { path = "runtime", version = "=2.3.0" }
 solana-runtime-transaction = { path = "runtime-transaction", version = "=2.3.0" }
-solana-sbpf = "=0.10.0"
+solana-sbpf = "=0.11.0"
 solana-sdk = "2.2.2"
 solana-sdk-ids = "2.2.1"
 solana-sdk-macro = "2.2.1"

Cargo.toml

@@ -8,7 +8,7 @@ use {
     solana_sbpf::{
         aligned_memory::{AlignedMemory, Pod},
         ebpf::{HOST_ALIGN, MM_INPUT_START},
-        memory_region::{MemoryRegion, MemoryState},
+        memory_region::MemoryRegion,
     },
     solana_sdk_ids::bpf_loader_deprecated,
     solana_system_interface::MAX_PERMITTED_DATA_LENGTH,
@@ -99,7 +99,20 @@ impl Serializer {
         } else {
             self.push_region(true);
             let vaddr = self.vaddr;
-            self.push_account_data_region(account)?;
+            if !account.get_data().is_empty() {
+                let writable = account.can_data_be_changed().is_ok();
+                let shared = account.is_shared();
+                let mut new_region = if writable && !shared {
+                    MemoryRegion::new_writable(account.get_data_mut()?, self.vaddr)
+                } else {
+                    MemoryRegion::new_readonly(account.get_data(), self.vaddr)
+                };
+                if writable && shared {
+                    new_region.cow_callback_payload = account.get_index_in_transaction() as u32;
+                }
+                self.vaddr += new_region.len;
+                self.regions.push(new_region);
+            }
             vaddr
         };
 
@@ -126,27 +139,6 @@ impl Serializer {
         Ok(vm_data_addr)
     }
 
-    fn push_account_data_region(
-        &mut self,
-        account: &mut BorrowedAccount<'_>,
-    ) -> Result<(), InstructionError> {
-        if !account.get_data().is_empty() {
-            let region = match account_data_region_memory_state(account) {
-                MemoryState::Readable => MemoryRegion::new_readonly(account.get_data(), self.vaddr),
-                MemoryState::Writable => {
-                    MemoryRegion::new_writable(account.get_data_mut()?, self.vaddr)
-                }
-                MemoryState::Cow(index_in_transaction) => {
-                    MemoryRegion::new_cow(account.get_data(), self.vaddr, index_in_transaction)
-                }
-            };
-            self.vaddr += region.len;
-            self.regions.push(region);
-        }
-
-        Ok(())
-    }
-
     fn push_region(&mut self, writable: bool) {
         let range = self.region_start..self.buffer.len();
         let region = if writable {
@@ -620,18 +612,6 @@ fn deserialize_parameters_aligned<I: IntoIterator<Item = usize>>(
     Ok(())
 }
 
-pub fn account_data_region_memory_state(account: &BorrowedAccount<'_>) -> MemoryState {
-    if account.can_data_be_changed().is_ok() {
-        if account.is_shared() {
-            MemoryState::Cow(account.get_index_in_transaction() as u64)
-        } else {
-            MemoryState::Writable
-        }
-    } else {
-        MemoryState::Readable
-    }
-}
-
 #[cfg(test)]
 #[allow(clippy::indexing_slicing)]
 mod tests {

program-runtime/src/serialization.rs

@@ -357,9 +357,9 @@ fn create_memory_mapping<'a, 'b, C: ContextObject>(
 
     Ok(MemoryMapping::new_with_cow(
         regions,
-        transaction_context.account_data_write_access_handler(),
         config,
         sbpf_version,
+        transaction_context.account_data_write_access_handler(),
     )?)
 }
 

programs/bpf_loader/src/lib.rs

@@ -4,13 +4,8 @@ use {
     scopeguard::defer,
     solana_loader_v3_interface::instruction as bpf_loader_upgradeable,
     solana_measure::measure::Measure,
-    solana_program_runtime::{
-        invoke_context::SerializedAccountMetadata, serialization::account_data_region_memory_state,
-    },
-    solana_sbpf::{
-        ebpf,
-        memory_region::{MemoryRegion, MemoryState},
-    },
+    solana_program_runtime::invoke_context::SerializedAccountMetadata,
+    solana_sbpf::{ebpf, memory_region::MemoryRegion},
     solana_stable_layout::stable_instruction::StableInstruction,
     solana_transaction_context::BorrowedAccount,
     std::{mem, ptr},
@@ -365,7 +360,7 @@ impl<'a> CallerAccount<'a> {
         &self,
         memory_mapping: &'a MemoryMapping<'_>,
         is_loader_deprecated: bool,
-    ) -> Result<Option<&'a MemoryRegion>, Error> {
+    ) -> Result<Option<(usize, &'a MemoryRegion)>, Error> {
         account_realloc_region(
             memory_mapping,
             self.vm_data_addr,
@@ -1076,7 +1071,7 @@ fn cpi_common<S: SyscallInvokeSigned>(
     account_infos_len: u64,
     signers_seeds_addr: u64,
     signers_seeds_len: u64,
-    memory_mapping: &MemoryMapping,
+    memory_mapping: &mut MemoryMapping,
 ) -> Result<u64, Error> {
     // CPI entry.
     //
@@ -1148,12 +1143,12 @@ fn cpi_common<S: SyscallInvokeSigned>(
         // it's better to be safe than sorry.
         for (index_in_caller, caller_account) in accounts.iter() {
             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_perms(
                     memory_mapping,
                     caller_account,
-                    &callee_account,
+                    &mut callee_account,
                     is_loader_deprecated,
                 )?;
             }
@@ -1266,9 +1261,9 @@ fn update_callee_account(
 }
 
 fn update_caller_account_perms(
-    memory_mapping: &MemoryMapping,
+    memory_mapping: &mut MemoryMapping,
     caller_account: &CallerAccount,
-    callee_account: &BorrowedAccount<'_>,
+    callee_account: &mut BorrowedAccount<'_>,
     is_loader_deprecated: bool,
 ) -> Result<(), Error> {
     let CallerAccount {
@@ -1277,26 +1272,66 @@ fn update_caller_account_perms(
         ..
     } = caller_account;
 
-    let data_region = account_data_region(memory_mapping, *vm_data_addr, *original_data_len)?;
-    if let Some(region) = data_region {
-        region
-            .state
-            .set(account_data_region_memory_state(callee_account));
+    if let Some((region_index, region)) =
+        account_data_region(memory_mapping, *vm_data_addr, *original_data_len)?
+    {
+        let writable = callee_account.can_data_be_changed().is_ok();
+        let shared = callee_account.is_shared();
+        let mut new_region = if writable && !shared {
+            MemoryRegion::new_writable(
+                unsafe {
+                    std::slice::from_raw_parts_mut(
+                        region.host_addr.get() as *mut u8,
+                        region.len as usize,
+                    )
+                },
+                region.vm_addr,
+            )
+        } else {
+            MemoryRegion::new_readonly(
+                unsafe {
+                    std::slice::from_raw_parts(
+                        region.host_addr.get() as *const u8,
+                        region.len as usize,
+                    )
+                },
+                region.vm_addr,
+            )
+        };
+        if writable && shared {
+            new_region.cow_callback_payload = callee_account.get_index_in_transaction() as u32;
+        }
+        memory_mapping.replace_region(region_index, new_region)?;
     }
-    let realloc_region = account_realloc_region(
+
+    if let Some((region_index, region)) = account_realloc_region(
         memory_mapping,
         *vm_data_addr,
         *original_data_len,
         is_loader_deprecated,
-    )?;
-    if let Some(region) = realloc_region {
-        region
-            .state
-            .set(if callee_account.can_data_be_changed().is_ok() {
-                MemoryState::Writable
-            } else {
-                MemoryState::Readable
-            });
+    )? {
+        let new_region = if callee_account.can_data_be_changed().is_ok() {
+            MemoryRegion::new_writable(
+                unsafe {
+                    std::slice::from_raw_parts_mut(
+                        region.host_addr.get() as *mut u8,
+                        region.len as usize,
+                    )
+                },
+                region.vm_addr,
+            )
+        } else {
+            MemoryRegion::new_readonly(
+                unsafe {
+                    std::slice::from_raw_parts(
+                        region.host_addr.get() as *const u8,
+                        region.len as usize,
+                    )
+                },
+                region.vm_addr,
+            )
+        };
+        memory_mapping.replace_region(region_index, new_region)?;
     }
 
     Ok(())
@@ -1323,7 +1358,7 @@ fn update_caller_account(
 
     let mut zero_all_mapped_spare_capacity = false;
     if direct_mapping {
-        if let Some(region) = account_data_region(
+        if let Some((_region_index, region)) = account_data_region(
             memory_mapping,
             caller_account.vm_data_addr,
             caller_account.original_data_len,
@@ -1412,12 +1447,12 @@ fn update_caller_account(
 
                     // Temporarily configure the realloc region as writable then set it back to
                     // whatever state it had.
-                    let realloc_region = caller_account
+                    let (_realloc_region_index, realloc_region) = caller_account
                         .realloc_region(memory_mapping, is_loader_deprecated)?
                         .unwrap(); // unwrapping here is fine, we already asserted !is_loader_deprecated
-                    let original_state = realloc_region.state.replace(MemoryState::Writable);
+                    let original_state = realloc_region.writable.replace(true);
                     defer! {
-                        realloc_region.state.set(original_state);
+                        realloc_region.writable.set(original_state);
                     };
 
                     // We need to zero the unused space in the realloc region, starting after the
@@ -1521,12 +1556,12 @@ fn update_caller_account(
                 //
                 // Therefore we temporarily configure the realloc region as writable
                 // then set it back to whatever state it had.
-                let realloc_region = caller_account
+                let (_realloc_region_index, realloc_region) = caller_account
                     .realloc_region(memory_mapping, is_loader_deprecated)?
                     .unwrap(); // unwrapping here is fine, we asserted !is_loader_deprecated
-                let original_state = realloc_region.state.replace(MemoryState::Writable);
+                let original_state = realloc_region.writable.replace(true);
                 defer! {
-                    realloc_region.state.set(original_state);
+                    realloc_region.writable.set(original_state);
                 };
 
                 translate_slice_mut::<u8>(
@@ -1566,37 +1601,38 @@ fn account_data_region<'a>(
     memory_mapping: &'a MemoryMapping<'_>,
     vm_data_addr: u64,
     original_data_len: usize,
-) -> Result<Option<&'a MemoryRegion>, Error> {
+) -> Result<Option<(usize, &'a MemoryRegion)>, Error> {
     if original_data_len == 0 {
         return Ok(None);
     }
 
     // We can trust vm_data_addr to point to the correct region because we
     // enforce that in CallerAccount::from_(sol_)account_info.
-    let data_region = memory_mapping.region(AccessType::Load, vm_data_addr)?;
+    let (data_region_index, data_region) = memory_mapping.region(AccessType::Load, vm_data_addr)?;
     // vm_data_addr must always point to the beginning of the region
     debug_assert_eq!(data_region.vm_addr, vm_data_addr);
-    Ok(Some(data_region))
+    Ok(Some((data_region_index, data_region)))
 }
 
 fn account_realloc_region<'a>(
     memory_mapping: &'a MemoryMapping<'_>,
     vm_data_addr: u64,
     original_data_len: usize,
     is_loader_deprecated: bool,
-) -> Result<Option<&'a MemoryRegion>, Error> {
+) -> Result<Option<(usize, &'a MemoryRegion)>, Error> {
     if is_loader_deprecated {
         return Ok(None);
     }
 
     let realloc_vm_addr = vm_data_addr.saturating_add(original_data_len as u64);
-    let realloc_region = memory_mapping.region(AccessType::Load, realloc_vm_addr)?;
+    let (realloc_region_index, realloc_region) =
+        memory_mapping.region(AccessType::Load, realloc_vm_addr)?;
     debug_assert_eq!(realloc_region.vm_addr, realloc_vm_addr);
     debug_assert!((MAX_PERMITTED_DATA_INCREASE
         ..MAX_PERMITTED_DATA_INCREASE.saturating_add(BPF_ALIGN_OF_U128))
         .contains(&(realloc_region.len as usize)));
-    debug_assert!(!matches!(realloc_region.state.get(), MemoryState::Cow(_)));
-    Ok(Some(realloc_region))
+    debug_assert_eq!(realloc_region.cow_callback_payload, u32::MAX);
+    Ok(Some((realloc_region_index, realloc_region)))
 }
 
 #[allow(clippy::indexing_slicing)]