direct mapping: misc fixes (#32649)

* transaction_context: update make_data_mut comment

* bpf_loader: cpi: pass SerializeAccountMetadata to CallerAccount::from*

We now have a way to provide CallerAccount with trusted values coming
from our internal serialization code and not from untrusted vm space

* bpf_loader: direct_mapping: enforce account info pointers to be immutable

When direct mapping is enabled, we might need to update account data
memory regions across CPI calls. Since the only way we have to retrieve
the regions is based on their vm addresses, we enforce vm addresses to
be stable.  Accounts can still be mutated and resized of course, but it
must be done in place.

This also locks all other AccountInfo pointers, since there's no legitimate
reason to make them point to anything else.

* bpf_loader: cpi: access ref_to_len_in_vm through VmValue

Direct mapping needs to translate vm values at each access since
permissions of the underlying memory might have changed.

* direct mapping: improve memory permission tracking across CPI calls

Ensure that the data and realloc regions of an account always track the
account's permissions. In order to do this, we also need to split
realloc regions in their own self contained regions, where before we
had:

[account fields][account data][account realloc + more account fields + next account fields][next account data][...]

we now have:

[account fields][account data][account realloc][more account fields + next account fields][next account data][...]

Tested in TEST_[FORBID|ALLOW]_WRITE_AFTER_OWNERSHIP_CHANGE*

Additionally when direct mapping is on, we must update all perms at once before
doing account data updates. Otherwise, updating an account might write into
another account whose perms we haven't updated yet. Tested in
TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE.

* bpf_loader: serialization: address review comment don't return vm_addr from push_account_region

* bpf_loader: rename push_account_region to push_account_data_region

* cpi: fix slow edge case zeroing extra account capacity after shrinking an account

When returning from CPI we need to zero all the account memory up to the
original length only if we know we're potentially dealing with uninitialized
memory.

When we know that the spare capacity has deterministic content, we only need to
zero new_len..prev_len.

This fixes a slow edge case that was triggerable by the following scenario:

- load a large account (say 10MB) into the vm
- shrink to 10 bytes - would memset 10..10MB
- shrink to 9 bytes - would memset 9..10MB
- shrink to 8 bytes - would memset 8..10MB
- ...

Now instead in the scenario above the following will happen:

- load a large account (say 10MB) into the vm
- shrink to 10 bytes - memsets 10..10MB
- shrink to 9 bytes - memsets 9..10
- shrink to 8 bytes - memset 8..9
- ...

* bpf_loader: add account_data_region_memory_state helper

Shared between serialization and CPI to figure out the MemoryState of an
account.

* cpi: direct_mapping: error out if ref_to_len_in_vm points to account memory

If ref_to_len_in_vm is allowed to be in account memory, calles could mutate it,
essentially letting callees directly mutate callers memory.

* bpf_loader: direct_mapping: map AccessViolation -> InstructionError

Return the proper ReadonlyDataModified / ExecutableDataModified /
ExternalAccountDataModified depending on where the violation occurs

* bpf_loader: cpi: remove unnecessary infallible slice::get call

program-runtime/src/invoke_context.rs

@@ -153,6 +153,10 @@ pub struct SyscallContext {
 #[derive(Debug, Clone)]
 pub struct SerializedAccountMetadata {
     pub original_data_len: usize,
+    pub vm_data_addr: u64,
+    pub vm_key_addr: u64,
+    pub vm_lamports_addr: u64,
+    pub vm_owner_addr: u64,
 }
 
 pub struct InvokeContext<'a> {

programs/bpf_loader/Cargo.toml

@@ -15,6 +15,7 @@ byteorder = { workspace = true }
 libsecp256k1 = { workspace = true }
 log = { workspace = true }
 rand = { workspace = true }
+scopeguard = { workspace = true }
 solana-measure = { workspace = true }
 solana-program-runtime = { workspace = true }
 solana-sdk = { workspace = true }

programs/bpf_loader/src/lib.rs

@@ -1495,12 +1495,19 @@ fn execute<'a, 'b: 'a>(
     let log_collector = invoke_context.get_log_collector();
     let transaction_context = &invoke_context.transaction_context;
     let instruction_context = transaction_context.get_current_instruction_context()?;
-    let program_id = *instruction_context.get_last_program_key(transaction_context)?;
+    let (program_id, is_loader_deprecated) = {
+        let program_account =
+            instruction_context.try_borrow_last_program_account(transaction_context)?;
+        (
+            *program_account.get_key(),
+            *program_account.get_owner() == bpf_loader_deprecated::id(),
+        )
+    };
     #[cfg(any(target_os = "windows", not(target_arch = "x86_64")))]
     let use_jit = false;
     #[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))]
     let use_jit = executable.get_compiled_program().is_some();
-    let bpf_account_data_direct_mapping = invoke_context
+    let direct_mapping = invoke_context
         .feature_set
         .is_active(&bpf_account_data_direct_mapping::id());
 
@@ -1511,18 +1518,26 @@ fn execute<'a, 'b: 'a>(
         invoke_context
             .feature_set
             .is_active(&cap_bpf_program_instruction_accounts::ID),
-        !bpf_account_data_direct_mapping,
+        !direct_mapping,
     )?;
     serialize_time.stop();
 
-    // save the account addresses so in case of AccessViolation below we can
-    // map to InstructionError::ReadonlyDataModified, which is easier to
-    // diagnose from developers
-    let account_region_addrs = regions
+    // save the account addresses so in case we hit an AccessViolation error we
+    // can map to a more specific error
+    let account_region_addrs = accounts_metadata
         .iter()
-        .map(|r| r.vm_addr..r.vm_addr.saturating_add(r.len))
+        .map(|m| {
+            let vm_end = m
+                .vm_data_addr
+                .saturating_add(m.original_data_len as u64)
+                .saturating_add(if !is_loader_deprecated {
+                    MAX_PERMITTED_DATA_INCREASE as u64
+                } else {
+                    0
+                });
+            m.vm_data_addr..vm_end
+        })
         .collect::<Vec<_>>();
-    let addr_is_account_data = |addr: u64| account_region_addrs.iter().any(|r| r.contains(&addr));
 
     let mut create_vm_time = Measure::start("create_vm");
     let mut execute_time;
@@ -1574,22 +1589,43 @@ fn execute<'a, 'b: 'a>(
                 };
                 Err(Box::new(error) as Box<dyn std::error::Error>)
             }
-            ProgramResult::Err(error) => {
-                let error = match error.downcast_ref() {
-                    Some(EbpfError::AccessViolation(
+            ProgramResult::Err(mut error) => {
+                if direct_mapping {
+                    if let Some(EbpfError::AccessViolation(
                         _pc,
                         AccessType::Store,
                         address,
                         _size,
                         _section_name,
-                    )) if addr_is_account_data(*address) => {
-                        // We can get here if direct_mapping is enabled and a program tries to
-                        // write to a readonly account. Map the error to ReadonlyDataModified so
-                        // it's easier for devs to diagnose what happened.
-                        Box::new(InstructionError::ReadonlyDataModified)
+                    )) = error.downcast_ref()
+                    {
+                        // If direct_mapping 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, _)) = account_region_addrs
+                            .iter()
+                            .enumerate()
+                            .find(|(_, vm_region)| vm_region.contains(address))
+                        {
+                            let transaction_context = &invoke_context.transaction_context;
+                            let instruction_context =
+                                transaction_context.get_current_instruction_context()?;
+
+                            let account = instruction_context.try_borrow_instruction_account(
+                                transaction_context,
+                                instruction_account_index as IndexOfAccount,
+                            )?;
+
+                            error = Box::new(if account.is_executable() {
+                                InstructionError::ExecutableDataModified
+                            } else if account.is_writable() {
+                                InstructionError::ExternalAccountDataModified
+                            } else {
+                                InstructionError::ReadonlyDataModified
+                            })
+                        }
                     }
-                    _ => error,
-                };
+                }
                 Err(error)
             }
             _ => Ok(()),
@@ -1615,12 +1651,8 @@ 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(),
-            !bpf_account_data_direct_mapping,
-        )
-        .map_err(|error| Box::new(error) as Box<dyn std::error::Error>)
+        deserialize_parameters(invoke_context, parameter_bytes.as_slice(), !direct_mapping)
+            .map_err(|error| Box::new(error) as Box<dyn std::error::Error>)
     });
     deserialize_time.stop();