Fix alignment check (#25351)

program-runtime/src/invoke_context.rs

@@ -206,6 +206,13 @@ impl<'a> StackFrame<'a> {
     }
 }
 
+struct SyscallContext {
+    check_aligned: bool,
+    check_size: bool,
+    orig_account_lengths: Vec<usize>,
+    allocator: Rc<RefCell<dyn Alloc>>,
+}
+
 pub struct InvokeContext<'a> {
     pub transaction_context: &'a mut TransactionContext,
     #[allow(deprecated)]
@@ -224,10 +231,7 @@ pub struct InvokeContext<'a> {
     pub timings: ExecuteDetailsTimings,
     pub blockhash: Hash,
     pub lamports_per_signature: u64,
-    check_aligned: bool,
+    syscall_context: Vec<Option<SyscallContext>>,
-    check_size: bool,
-    orig_account_lengths: Vec<Option<Vec<usize>>>,
-    allocators: Vec<Option<Rc<RefCell<dyn Alloc>>>>,
 }
 
 impl<'a> InvokeContext<'a> {
@@ -262,10 +266,7 @@ impl<'a> InvokeContext<'a> {
             timings: ExecuteDetailsTimings::default(),
             blockhash,
             lamports_per_signature,
-            check_aligned: true,
+            syscall_context: Vec::new(),
-            check_size: true,
-            orig_account_lengths: Vec::new(),
-            allocators: Vec::new(),
         }
     }
 
@@ -454,8 +455,7 @@ impl<'a> InvokeContext<'a> {
                 std::mem::transmute(keyed_accounts.as_slice())
             }),
         ));
-        self.orig_account_lengths.push(None);
+        self.syscall_context.push(None);
-        self.allocators.push(None);
         self.transaction_context.push(
             program_indices,
             instruction_accounts,
@@ -467,8 +467,7 @@ impl<'a> InvokeContext<'a> {
 
     /// Pop a stack frame from the invocation stack
     pub fn pop(&mut self) -> Result<(), InstructionError> {
-        self.orig_account_lengths.pop();
+        self.syscall_context.pop();
-        self.allocators.pop();
         self.invoke_stack.pop();
         self.transaction_context.pop()
     }
@@ -1077,66 +1076,61 @@ impl<'a> InvokeContext<'a> {
             .get_key_of_account_at_index(index_in_transaction)
     }
 
-    /// Set the original account lengths
+    // Set this instruction syscall context
-    pub fn set_orig_account_lengths(
+    pub fn set_syscall_context(
         &mut self,
+        check_aligned: bool,
+        check_size: bool,
         orig_account_lengths: Vec<usize>,
+        allocator: Rc<RefCell<dyn Alloc>>,
     ) -> Result<(), InstructionError> {
         *self
-            .orig_account_lengths
+            .syscall_context
             .last_mut()
-            .ok_or(InstructionError::CallDepth)? = Some(orig_account_lengths);
+            .ok_or(InstructionError::CallDepth)? = Some(SyscallContext {
+            check_aligned,
+            check_size,
+            orig_account_lengths,
+            allocator,
+        });
         Ok(())
     }
 
-    /// Get the original account lengths
-    pub fn get_orig_account_lengths(&self) -> Result<&[usize], InstructionError> {
-        self.orig_account_lengths
-            .last()
-            .and_then(|orig_account_lengths| orig_account_lengths.as_ref())
-            .map(|orig_account_lengths| orig_account_lengths.as_slice())
-            .ok_or(InstructionError::CallDepth)
-    }
-
-    // Set should alignment be enforced during user pointer translation
-    pub fn set_check_aligned(&mut self, check_aligned: bool) {
-        self.check_aligned = check_aligned;
-    }
-
     // Should alignment be enforced during user pointer translation
     pub fn get_check_aligned(&self) -> bool {
-        self.check_aligned
+        self.syscall_context
-    }
+            .last()
-
+            .and_then(|context| context.as_ref())
-    // Set should type size be checked during user pointer translation
+            .map(|context| context.check_aligned)
-    pub fn set_check_size(&mut self, check_size: bool) {
+            .unwrap_or(true)
-        self.check_size = check_size;
     }
 
     // Set should type size be checked during user pointer translation
     pub fn get_check_size(&self) -> bool {
-        self.check_size
+        self.syscall_context
+            .last()
+            .and_then(|context| context.as_ref())
+            .map(|context| context.check_size)
+            .unwrap_or(true)
+    }
+
+    /// Get the original account lengths
+    pub fn get_orig_account_lengths(&self) -> Result<&[usize], InstructionError> {
+        self.syscall_context
+            .last()
+            .and_then(|context| context.as_ref())
+            .map(|context| context.orig_account_lengths.as_slice())
+            .ok_or(InstructionError::CallDepth)
     }
 
     // Get this instruction's memory allocator
     pub fn get_allocator(&self) -> Result<Rc<RefCell<dyn Alloc>>, InstructionError> {
-        self.allocators
+        self.syscall_context
             .last()
-            .and_then(|allocator| allocator.clone())
+            .and_then(|context| context.as_ref())
+            .map(|context| context.allocator.clone())
             .ok_or(InstructionError::CallDepth)
     }
-
-    // Set this instruction's memory allocator
-    pub fn set_allocator(
-        &mut self,
-        allocator: Rc<RefCell<dyn Alloc>>,
-    ) -> Result<(), InstructionError> {
-        *self
-            .allocators
-            .last_mut()
-            .ok_or(InstructionError::CallDepth)? = Some(allocator);
-        Ok(())
-    }
 }
 
 pub struct MockInvokeContextPreparation {

programs/bpf/Cargo.lock

@@ -3983,6 +3983,13 @@ dependencies = [
  "solana-program 1.11.0",
 ]
 
+[[package]]
+name = "solana-bpf-rust-inner_instruction_alignment_check"
+version = "1.11.0"
+dependencies = [
+ "solana-program 1.11.0",
+]
+
 [[package]]
 name = "solana-bpf-rust-instruction-introspection"
 version = "1.11.0"

programs/bpf/Cargo.toml

@@ -59,6 +59,7 @@ members = [
     "rust/external_spend",
     "rust/get_minimum_delegation",
     "rust/finalize",
+    "rust/inner_instruction_alignment_check",
     "rust/instruction_introspection",
     "rust/invoke",
     "rust/invoke_and_error",

programs/bpf/benches/bpf_loader.rs

@@ -115,8 +115,7 @@ fn bench_program_alu(bencher: &mut Bencher) {
         Executable::<BpfError, ThisInstructionMeter>::jit_compile(&mut executable).unwrap();
         let compute_meter = invoke_context.get_compute_meter();
         let mut instruction_meter = ThisInstructionMeter { compute_meter };
-        invoke_context.set_orig_account_lengths(vec![]).unwrap();
+        let mut vm = create_vm(&executable, &mut inner_iter, vec![], invoke_context).unwrap();
-        let mut vm = create_vm(&executable, &mut inner_iter, invoke_context).unwrap();
 
         println!("Interpreted:");
         assert_eq!(
@@ -220,9 +219,6 @@ fn bench_create_vm(bencher: &mut Bencher) {
                 .unwrap(),
         )
         .unwrap();
-        invoke_context
-            .set_orig_account_lengths(account_lengths)
-            .unwrap();
 
         let executable = Executable::<BpfError, ThisInstructionMeter>::from_elf(
             &elf,
@@ -233,7 +229,13 @@ fn bench_create_vm(bencher: &mut Bencher) {
         .unwrap();
 
         bencher.iter(|| {
-            let _ = create_vm(&executable, serialized.as_slice_mut(), invoke_context).unwrap();
+            let _ = create_vm(
+                &executable,
+                serialized.as_slice_mut(),
+                account_lengths.clone(),
+                invoke_context,
+            )
+            .unwrap();
         });
     });
 }
@@ -268,10 +270,13 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) {
         .unwrap();
         let compute_meter = invoke_context.get_compute_meter();
         let mut instruction_meter = ThisInstructionMeter { compute_meter };
-        invoke_context
+        let mut vm = create_vm(
-            .set_orig_account_lengths(account_lengths)
+            &executable,
+            serialized.as_slice_mut(),
+            account_lengths,
+            invoke_context,
+        )
         .unwrap();
-        let mut vm = create_vm(&executable, serialized.as_slice_mut(), invoke_context).unwrap();
 
         let mut measure = Measure::start("tune");
         let _ = vm.execute_program_interpreted(&mut instruction_meter);

programs/bpf/build.rs

@@ -71,6 +71,7 @@ fn main() {
             "external_spend",
             "finalize",
             "get_minimum_delegation",
+            "inner_instruction_alignment_check",
             "instruction_introspection",
             "invoke",
             "invoke_and_error",

programs/bpf/rust/inner_instruction_alignment_check/Cargo.toml

@@ -0,0 +1,19 @@
+[package]
+name = "solana-bpf-rust-inner_instruction_alignment_check"
+version = "1.11.0"
+description = "Solana BPF test program written in Rust"
+authors = ["Solana Maintainers <maintainers@solana.foundation>"]
+repository = "https://github.com/solana-labs/solana"
+license = "Apache-2.0"
+homepage = "https://solana.com/"
+documentation = "https://docs.rs/solana-bpf-rust-inner_instruction_alignment_check"
+edition = "2021"
+
+[dependencies]
+solana-program = { path = "../../../../sdk/program", version = "=1.11.0" }
+
+[lib]
+crate-type = ["cdylib"]
+
+[package.metadata.docs.rs]
+targets = ["x86_64-unknown-linux-gnu"]

programs/bpf/rust/inner_instruction_alignment_check/src/lib.rs

@@ -0,0 +1,41 @@
+//! Example Rust-based BPF noop program
+
+use solana_program::{
+    account_info::AccountInfo,
+    entrypoint_deprecated::ProgramResult,
+    instruction::{AccountMeta, Instruction},
+    msg,
+    program::invoke,
+    pubkey::Pubkey,
+};
+
+#[no_mangle]
+fn custom_panic(info: &core::panic::PanicInfo<'_>) {
+    // Full panic reporting
+    msg!(&format!("{}", info));
+}
+
+solana_program::entrypoint_deprecated!(process_instruction);
+#[allow(clippy::unnecessary_wraps)]
+fn process_instruction(
+    _program_id: &Pubkey,
+    accounts: &[AccountInfo],
+    instruction_data: &[u8],
+) -> ProgramResult {
+    let to_call = accounts[0].key;
+    let infos = accounts;
+    let instruction = Instruction {
+        accounts: vec![AccountMeta {
+            pubkey: *accounts[1].key,
+            is_signer: accounts[1].is_signer,
+            is_writable: accounts[1].is_writable,
+        }],
+        data: instruction_data.to_owned(),
+        program_id: *to_call,
+    };
+
+    let _ = invoke(&instruction, infos);
+    let _ = invoke(&instruction, infos);
+
+    Ok(())
+}

programs/bpf/rust/invoke_and_ok/src/lib.rs

@@ -1,4 +1,4 @@
-//! Invokes an instruction and returns an error, the instruction invoked
+//! Invokes an instruction and return ok no matter what, the instruction invoked
 //! uses the instruction data provided and all the accounts
 
 use solana_program::{

programs/bpf/rust/invoke_and_return/src/lib.rs

@@ -1,4 +1,4 @@
-//! Invokes an instruction and returns an error, the instruction invoked
+//! Invokes an instruction and returns the invoke result, the instruction invoked
 //! uses the instruction data provided and all the accounts
 
 use solana_program::{

programs/bpf/tests/programs.rs

@@ -246,11 +246,13 @@ fn run_program(name: &str) -> u64 {
                 .unwrap();
             let mut parameter_bytes = parameter_bytes.clone();
             {
-                invoke_context
+                let mut vm = create_vm(
-                    .set_orig_account_lengths(account_lengths.clone())
+                    &executable,
+                    parameter_bytes.as_slice_mut(),
+                    account_lengths.clone(),
+                    invoke_context,
+                )
                 .unwrap();
-                let mut vm =
-                    create_vm(&executable, parameter_bytes.as_slice_mut(), invoke_context).unwrap();
                 let result = if i == 0 {
                     vm.execute_program_interpreted(&mut instruction_meter)
                 } else {
@@ -3555,3 +3557,56 @@ fn test_get_minimum_delegation() {
     let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
     assert!(result.is_ok());
 }
+
+#[cfg(feature = "bpf_rust")]
+#[test]
+fn test_program_bpf_inner_instruction_alignment_checks() {
+    solana_logger::setup();
+
+    let GenesisConfigInfo {
+        mut genesis_config,
+        mint_keypair,
+        ..
+    } = create_genesis_config(50);
+    genesis_config
+        .accounts
+        .remove(&solana_sdk::feature_set::disable_deprecated_loader::id())
+        .unwrap();
+    let mut bank = Bank::new_for_tests(&genesis_config);
+    let (name, id, entrypoint) = solana_bpf_loader_program!();
+    bank.add_builtin(&name, &id, entrypoint);
+    let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!();
+    bank.add_builtin(&name, &id, entrypoint);
+    let bank_client = BankClient::new(bank);
+
+    // load aligned program
+    let noop = load_bpf_program(
+        &bank_client,
+        &bpf_loader::id(),
+        &mint_keypair,
+        "solana_bpf_rust_noop",
+    );
+
+    // Load unaligned program
+    let inner_instruction_alignment_check = load_bpf_program(
+        &bank_client,
+        &bpf_loader_deprecated::id(),
+        &mint_keypair,
+        "solana_bpf_rust_inner_instruction_alignment_check",
+    );
+
+    // invoke unaligned program, which will call aligned program twice,
+    // unaligned should be allowed once invoke completes
+    let mut instruction = Instruction::new_with_bytes(
+        inner_instruction_alignment_check,
+        &[0],
+        vec![
+            AccountMeta::new_readonly(noop, false),
+            AccountMeta::new_readonly(mint_keypair.pubkey(), false),
+        ],
+    );
+
+    instruction.data[0] += 1;
+    let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction.clone());
+    assert!(result.is_ok());
+}

programs/bpf_loader/src/lib.rs

@@ -249,6 +249,7 @@ fn check_loader_id(id: &Pubkey) -> bool {
 pub fn create_vm<'a, 'b>(
     program: &'a Pin<Box<Executable<BpfError, ThisInstructionMeter>>>,
     parameter_bytes: &mut [u8],
+    orig_account_lengths: Vec<usize>,
     invoke_context: &'a mut InvokeContext<'b>,
 ) -> Result<EbpfVm<'a, BpfError, ThisInstructionMeter>, EbpfError<BpfError>> {
     let compute_budget = invoke_context.get_compute_budget();
@@ -267,7 +268,7 @@ pub fn create_vm<'a, 'b>(
         AlignedMemory::new_with_size(compute_budget.heap_size.unwrap_or(HEAP_LENGTH), HOST_ALIGN);
     let parameter_region = MemoryRegion::new_writable(parameter_bytes, MM_INPUT_START);
     let mut vm = EbpfVm::new(program, heap.as_slice_mut(), vec![parameter_region])?;
-    syscalls::bind_syscall_context_objects(&mut vm, invoke_context, heap)?;
+    syscalls::bind_syscall_context_objects(&mut vm, invoke_context, heap, orig_account_lengths)?;
     Ok(vm)
 }
 
@@ -1165,13 +1166,14 @@ impl Executor for BpfExecutor {
         let (mut parameter_bytes, account_lengths) =
             serialize_parameters(invoke_context.transaction_context, instruction_context)?;
         serialize_time.stop();
-        invoke_context.set_orig_account_lengths(account_lengths)?;
+
         let mut create_vm_time = Measure::start("create_vm");
         let mut execute_time;
         let execution_result = {
             let mut vm = match create_vm(
                 &self.executable,
                 parameter_bytes.as_slice_mut(),
+                account_lengths,
                 invoke_context,
             ) {
                 Ok(info) => info,

programs/bpf_loader/src/syscalls.rs

@@ -384,30 +384,28 @@ pub fn bind_syscall_context_objects<'a, 'b>(
     vm: &mut EbpfVm<'a, BpfError, crate::ThisInstructionMeter>,
     invoke_context: &'a mut InvokeContext<'b>,
     heap: AlignedMemory,
+    orig_account_lengths: Vec<usize>,
 ) -> Result<(), EbpfError<BpfError>> {
-    invoke_context.set_check_aligned(
+    let check_aligned = bpf_loader_deprecated::id()
-        bpf_loader_deprecated::id()
         != invoke_context
             .transaction_context
             .get_current_instruction_context()
             .and_then(|instruction_context| {
-                    instruction_context
+                instruction_context.try_borrow_program_account(invoke_context.transaction_context)
-                        .try_borrow_program_account(invoke_context.transaction_context)
             })
             .map(|program_account| *program_account.get_owner())
-                .map_err(SyscallError::InstructionError)?,
+            .map_err(SyscallError::InstructionError)?;
-    );
+    let check_size = invoke_context
-    invoke_context.set_check_size(
-        invoke_context
         .feature_set
-            .is_active(&check_slice_translation_size::id()),
+        .is_active(&check_slice_translation_size::id());
-    );
 
     invoke_context
-        .set_allocator(Rc::new(RefCell::new(BpfAllocator::new(
+        .set_syscall_context(
-            heap,
+            check_aligned,
-            ebpf::MM_HEAP_START,
+            check_size,
-        ))))
+            orig_account_lengths,
+            Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))),
+        )
         .map_err(SyscallError::InstructionError)?;
 
     let invoke_context = Rc::new(RefCell::new(invoke_context));
@@ -3253,7 +3251,7 @@ fn call<'a, 'b: 'a>(
                     memory_mapping,
                     caller_account.vm_data_addr,
                     new_len as u64,
-                    invoke_context.get_check_aligned(),
+                    false, // Don't care since it is byte aligned
                     invoke_context.get_check_size(),
                 )?;
                 *caller_account.ref_to_len_in_vm = new_len as u64;
@@ -4330,10 +4328,12 @@ mod tests {
             )
             .unwrap();
             invoke_context
-                .set_allocator(Rc::new(RefCell::new(BpfAllocator::new(
+                .set_syscall_context(
-                    heap,
+                    true,
-                    ebpf::MM_HEAP_START,
+                    true,
-                ))))
+                    vec![],
+                    Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))),
+                )
                 .unwrap();
             let mut syscall = SyscallAllocFree {
                 invoke_context: Rc::new(RefCell::new(&mut invoke_context)),
@@ -4370,12 +4370,13 @@ mod tests {
             )
             .unwrap();
             invoke_context
-                .set_allocator(Rc::new(RefCell::new(BpfAllocator::new(
+                .set_syscall_context(
-                    heap,
+                    false,
-                    ebpf::MM_HEAP_START,
+                    true,
-                ))))
+                    vec![],
+                    Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))),
+                )
                 .unwrap();
-            invoke_context.set_check_aligned(false);
             let mut syscall = SyscallAllocFree {
                 invoke_context: Rc::new(RefCell::new(&mut invoke_context)),
             };
@@ -4410,10 +4411,12 @@ mod tests {
             )
             .unwrap();
             invoke_context
-                .set_allocator(Rc::new(RefCell::new(BpfAllocator::new(
+                .set_syscall_context(
-                    heap,
+                    true,
-                    ebpf::MM_HEAP_START,
+                    true,
-                ))))
+                    vec![],
+                    Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))),
+                )
                 .unwrap();
             let mut syscall = SyscallAllocFree {
                 invoke_context: Rc::new(RefCell::new(&mut invoke_context)),
@@ -4451,10 +4454,12 @@ mod tests {
             )
             .unwrap();
             invoke_context
-                .set_allocator(Rc::new(RefCell::new(BpfAllocator::new(
+                .set_syscall_context(
-                    heap,
+                    true,
-                    ebpf::MM_HEAP_START,
+                    true,
-                ))))
+                    vec![],
+                    Rc::new(RefCell::new(BpfAllocator::new(heap, ebpf::MM_HEAP_START))),
+                )
                 .unwrap();
             let mut syscall = SyscallAllocFree {
                 invoke_context: Rc::new(RefCell::new(&mut invoke_context)),

rbpf-cli/src/main.rs

@@ -313,12 +313,10 @@ native machine code before execting it in the virtual machine.",
         _ => {}
     }
 
-    invoke_context
-        .set_orig_account_lengths(account_lengths)
-        .unwrap();
     let mut vm = create_vm(
         &executable,
         parameter_bytes.as_slice_mut(),
+        account_lengths,
         &mut invoke_context,
     )
     .unwrap();