From 6eeca9c6f1f987b9fe3401b10a0a6e8399980b39 Mon Sep 17 00:00:00 2001 From: Jack May Date: Thu, 24 Oct 2019 22:38:57 -0700 Subject: [PATCH] Make instruction data opaque to runtime (#6470) --- programs/bpf/Cargo.lock | 1 + programs/bpf/tests/programs.rs | 33 ++++++----- programs/bpf_loader_api/src/lib.rs | 8 ++- programs/failure_program/tests/failure.rs | 21 ++++--- runtime/benches/bank.rs | 9 ++- runtime/src/append_vec.rs | 4 +- runtime/src/loader_utils.rs | 25 ++++---- runtime/src/message_processor.rs | 72 ++--------------------- runtime/tests/noop.rs | 19 +++--- sdk/src/loader_instruction.rs | 16 +++++ 10 files changed, 94 insertions(+), 114 deletions(-) diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index cc61de70e..225e80f99 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -1777,6 +1777,7 @@ dependencies = [ name = "solana-runtime" version = "0.20.0" dependencies = [ + "backtrace 0.3.37 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "bv 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 6303fb4e6..ae6f0117f 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -3,9 +3,12 @@ mod bpf { use solana_runtime::bank::Bank; use solana_runtime::bank_client::BankClient; use solana_runtime::genesis_utils::{create_genesis_block, GenesisBlockInfo}; - use solana_runtime::loader_utils::load_program; + use solana_runtime::loader_utils::{load_program, run_program}; + use solana_sdk::bpf_loader; + use solana_sdk::instruction::AccountMeta; use std::env; use std::fs::File; + use std::io::Read; use std::path::PathBuf; /// BPF program file extension @@ -26,11 +29,7 @@ mod bpf { #[cfg(feature = "bpf_c")] mod bpf_c { use super::*; - use solana_runtime::loader_utils::create_invoke_instruction; - use solana_sdk::bpf_loader; - use solana_sdk::client::SyncClient; use solana_sdk::signature::KeypairUtil; - use std::io::Read; #[test] fn test_program_bpf_c() { @@ -62,9 +61,14 @@ mod bpf { // Call user program let program_id = load_program(&bank_client, &mint_keypair, &bpf_loader::id(), elf); - let instruction = - create_invoke_instruction(mint_keypair.pubkey(), program_id, &1u8); - let result = bank_client.send_instruction(&mint_keypair, instruction); + let account_metas = vec![AccountMeta::new(mint_keypair.pubkey(), true)]; + let result = run_program( + &bank_client, + &mint_keypair, + &program_id, + account_metas, + &1u8, + ); if program.1 { assert!(result.is_ok()); } else { @@ -77,14 +81,10 @@ mod bpf { #[cfg(feature = "bpf_rust")] mod bpf_rust { use super::*; - use solana_sdk::bpf_loader; - use solana_sdk::client::SyncClient; use solana_sdk::clock::DEFAULT_SLOTS_PER_EPOCH; - use solana_sdk::instruction::{AccountMeta, Instruction}; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, KeypairUtil}; use solana_sdk::sysvar::{clock, fees, rent, rewards, slot_hashes, stake_history}; - use std::io::Read; use std::sync::Arc; #[test] @@ -133,8 +133,13 @@ mod bpf { AccountMeta::new(stake_history::id(), false), AccountMeta::new(rent::id(), false), ]; - let instruction = Instruction::new(program_id, &1u8, account_metas); - let result = bank_client.send_instruction(&mint_keypair, instruction); + let result = run_program( + &bank_client, + &mint_keypair, + &program_id, + account_metas, + &1u8, + ); if program.1 { assert!(result.is_ok()); } else { diff --git a/programs/bpf_loader_api/src/lib.rs b/programs/bpf_loader_api/src/lib.rs index 029933ade..90b676051 100644 --- a/programs/bpf_loader_api/src/lib.rs +++ b/programs/bpf_loader_api/src/lib.rs @@ -167,8 +167,12 @@ pub fn process_instruction( } } } else { - warn!("Invalid instruction data: {:?}", ix_data); - return Err(InstructionError::GenericError); + warn!( + "Invalid instruction data ({:?}): {:?}", + ix_data.len(), + ix_data + ); + return Err(InstructionError::InvalidInstructionData); } Ok(()) } diff --git a/programs/failure_program/tests/failure.rs b/programs/failure_program/tests/failure.rs index 441560a30..01049cba6 100644 --- a/programs/failure_program/tests/failure.rs +++ b/programs/failure_program/tests/failure.rs @@ -1,8 +1,8 @@ use solana_runtime::bank::Bank; use solana_runtime::bank_client::BankClient; -use solana_runtime::loader_utils::create_invoke_instruction; -use solana_sdk::client::SyncClient; +use solana_runtime::loader_utils::run_program; use solana_sdk::genesis_block::create_genesis_block; +use solana_sdk::instruction::AccountMeta; use solana_sdk::instruction::InstructionError; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::KeypairUtil; @@ -14,15 +14,20 @@ fn test_program_native_failure() { let program_id = Pubkey::new_rand(); let bank = Bank::new(&genesis_block); bank.register_native_instruction_processor("solana_failure_program", &program_id); + let bank_client = BankClient::new(bank); // Call user program - let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8); - let bank_client = BankClient::new(bank); + let account_metas = vec![AccountMeta::new(alice_keypair.pubkey(), true)]; assert_eq!( - bank_client - .send_instruction(&alice_keypair, instruction) - .unwrap_err() - .unwrap(), + run_program( + &bank_client, + &alice_keypair, + &program_id, + account_metas, + &1u8, + ) + .unwrap_err() + .unwrap(), TransactionError::InstructionError(0, InstructionError::GenericError) ); } diff --git a/runtime/benches/bank.rs b/runtime/benches/bank.rs index db13f66e2..04c5b2f60 100644 --- a/runtime/benches/bank.rs +++ b/runtime/benches/bank.rs @@ -5,12 +5,13 @@ extern crate test; use log::*; use solana_runtime::bank::*; use solana_runtime::bank_client::BankClient; -use solana_runtime::loader_utils::create_invoke_instruction; use solana_sdk::account::KeyedAccount; use solana_sdk::client::AsyncClient; use solana_sdk::client::SyncClient; use solana_sdk::genesis_block::create_genesis_block; +use solana_sdk::instruction::AccountMeta; use solana_sdk::instruction::InstructionError; +use solana_sdk::loader_instruction; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, KeypairUtil}; use solana_sdk::transaction::Transaction; @@ -52,7 +53,8 @@ pub fn create_builtin_transactions( .transfer(10_000, &mint_keypair, &rando0.pubkey()) .expect(&format!("{}:{}", line!(), file!())); - let instruction = create_invoke_instruction(rando0.pubkey(), program_id, &1u8); + let account_metas = vec![AccountMeta::new(rando0.pubkey(), true)]; + let instruction = loader_instruction::invoke_main(&program_id, &1u8, account_metas); let (blockhash, _fee_calculator) = bank_client.get_recent_blockhash().unwrap(); Transaction::new_signed_instructions(&[&rando0], vec![instruction], blockhash) }) @@ -74,7 +76,8 @@ pub fn create_native_loader_transactions( .transfer(10_000, &mint_keypair, &rando0.pubkey()) .expect(&format!("{}:{}", line!(), file!())); - let instruction = create_invoke_instruction(rando0.pubkey(), program_id, &1u8); + let account_metas = vec![AccountMeta::new(rando0.pubkey(), true)]; + let instruction = loader_instruction::invoke_main(&program_id, &1u8, account_metas); let (blockhash, _fee_calculator) = bank_client.get_recent_blockhash().unwrap(); Transaction::new_signed_instructions(&[&rando0], vec![instruction], blockhash) }) diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 674b44e2d..14d04cb66 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -13,8 +13,8 @@ use std::{ sync::Mutex, }; -//Data is aligned at the next 64 byte offset. Without alignment loading the memory may -//crash on some architectures. +// Data is aligned at the next 64 byte offset. Without alignment loading the memory may +// crash on some architectures. macro_rules! align_up { ($addr: expr, $align: expr) => { ($addr + ($align - 1)) & !($align - 1) diff --git a/runtime/src/loader_utils.rs b/runtime/src/loader_utils.rs index 3e4065867..211204d27 100644 --- a/runtime/src/loader_utils.rs +++ b/runtime/src/loader_utils.rs @@ -1,11 +1,12 @@ use serde::Serialize; use solana_sdk::client::Client; -use solana_sdk::instruction::{AccountMeta, Instruction}; +use solana_sdk::instruction::AccountMeta; use solana_sdk::loader_instruction; use solana_sdk::message::Message; use solana_sdk::pubkey::Pubkey; -use solana_sdk::signature::{Keypair, KeypairUtil}; +use solana_sdk::signature::{Keypair, KeypairUtil, Signature}; use solana_sdk::system_instruction; +use solana_sdk::transport::Result; pub fn load_program( bank_client: &T, @@ -27,7 +28,7 @@ pub fn load_program( .send_instruction(&from_keypair, instruction) .unwrap(); - let chunk_size = 256; // Size of chunk just needs to fit into tx + let chunk_size = 256; // Size of the chunk needs to fit into the transaction let mut offset = 0; for chunk in program.chunks(chunk_size) { let instruction = @@ -48,13 +49,13 @@ pub fn load_program( program_pubkey } -// Return an Instruction that invokes `program_id` with `data` and required -// a signature from `from_pubkey`. -pub fn create_invoke_instruction( - from_pubkey: Pubkey, - program_id: Pubkey, - data: &T, -) -> Instruction { - let account_metas = vec![AccountMeta::new(from_pubkey, true)]; - Instruction::new(program_id, data, account_metas) +pub fn run_program( + bank_client: &T, + from_keypair: &Keypair, + loader_pubkey: &Pubkey, + account_metas: Vec, + data: &D, +) -> Result { + let instruction = loader_instruction::invoke_main(loader_pubkey, data, account_metas); + bank_client.send_instruction(from_keypair, instruction) } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 5ecbccca0..0605e7b83 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -6,13 +6,11 @@ use solana_sdk::account::{ }; use solana_sdk::instruction::{CompiledInstruction, InstructionError}; use solana_sdk::instruction_processor_utils; -use solana_sdk::loader_instruction::LoaderInstruction; use solana_sdk::message::Message; use solana_sdk::pubkey::Pubkey; use solana_sdk::system_program; use solana_sdk::transaction::TransactionError; use std::collections::HashMap; -use std::io::Write; use std::sync::RwLock; #[cfg(unix)] @@ -137,27 +135,6 @@ fn verify_instruction( Ok(()) } -/// Return instruction data to pass to process_instruction(). -/// When a loader is detected, the instruction data is wrapped with a LoaderInstruction -/// to signal to the loader that the instruction data should be used as arguments when -/// invoking a "main()" function. -fn get_loader_instruction_data<'a>( - loaders: &[(Pubkey, Account)], - ix_data: &'a [u8], - loader_ix_data: &'a mut Vec, -) -> &'a [u8] { - if loaders.len() > 1 { - let ix = LoaderInstruction::InvokeMain { - data: ix_data.to_vec(), - }; - let ix_data = bincode::serialize(&ix).unwrap(); - loader_ix_data.write_all(&ix_data).unwrap(); - loader_ix_data - } else { - ix_data - } -} - pub type ProcessInstruction = fn(&Pubkey, &mut [KeyedAccount], &[u8]) -> Result<(), InstructionError>; @@ -206,14 +183,6 @@ impl MessageProcessor { program_accounts: &mut [&mut Account], ) -> Result<(), InstructionError> { let program_id = instruction.program_id(&message.account_keys); - - let mut loader_ix_data = vec![]; - let ix_data = get_loader_instruction_data( - executable_accounts, - &instruction.data, - &mut loader_ix_data, - ); - let mut keyed_accounts = create_keyed_credit_only_accounts(executable_accounts); let mut keyed_accounts2: Vec<_> = instruction .accounts @@ -247,14 +216,18 @@ impl MessageProcessor { let loader_id = keyed_accounts[0].unsigned_key(); for (id, process_instruction) in &self.instruction_processors { if id == loader_id { - return process_instruction(&program_id, &mut keyed_accounts[1..], &ix_data); + return process_instruction( + &program_id, + &mut keyed_accounts[1..], + &instruction.data, + ); } } native_loader::invoke_entrypoint( &program_id, &mut keyed_accounts, - ix_data, + &instruction.data, &self.symbol_cache, ) } @@ -809,37 +782,4 @@ mod tests { )) ); } - - #[test] - fn test_get_loader_instruction_data() { - // First ensure the ix_data is unaffected if not invoking via a loader. - let ix_data = [1]; - let mut loader_ix_data = vec![]; - - let native_pubkey = Pubkey::new_rand(); - let native_loader = (native_pubkey, Account::new(0, 0, &native_pubkey)); - assert_eq!( - get_loader_instruction_data(&[native_loader.clone()], &ix_data, &mut loader_ix_data), - &ix_data - ); - - // Now ensure the ix_data is wrapped when there's a loader present. - let acme_pubkey = Pubkey::new_rand(); - let acme_loader = (acme_pubkey, Account::new(0, 0, &native_pubkey)); - let expected_ix = LoaderInstruction::InvokeMain { - data: ix_data.to_vec(), - }; - let expected_ix_data = bincode::serialize(&expected_ix).unwrap(); - assert_eq!( - get_loader_instruction_data( - &[native_loader.clone(), acme_loader.clone()], - &ix_data, - &mut loader_ix_data - ), - &expected_ix_data[..] - ); - - // Note there was an allocation in the input vector. - assert_eq!(loader_ix_data, expected_ix_data); - } } diff --git a/runtime/tests/noop.rs b/runtime/tests/noop.rs index d7627fa4d..549a10abb 100644 --- a/runtime/tests/noop.rs +++ b/runtime/tests/noop.rs @@ -1,8 +1,8 @@ use solana_runtime::bank::Bank; use solana_runtime::bank_client::BankClient; -use solana_runtime::loader_utils::create_invoke_instruction; -use solana_sdk::client::SyncClient; +use solana_runtime::loader_utils::run_program; use solana_sdk::genesis_block::create_genesis_block; +use solana_sdk::instruction::AccountMeta; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::KeypairUtil; @@ -14,11 +14,16 @@ fn test_program_native_noop() { let program_id = Pubkey::new_rand(); let bank = Bank::new(&genesis_block); bank.register_native_instruction_processor("solana_noop_program", &program_id); + let bank_client = BankClient::new(bank); // Call user program - let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8); - let bank_client = BankClient::new(bank); - bank_client - .send_instruction(&alice_keypair, instruction) - .unwrap(); + let account_metas = vec![AccountMeta::new(alice_keypair.pubkey(), true)]; + run_program( + &bank_client, + &alice_keypair, + &program_id, + account_metas, + &1u8, + ) + .unwrap(); } diff --git a/sdk/src/loader_instruction.rs b/sdk/src/loader_instruction.rs index ecbe0d669..f1e4d2020 100644 --- a/sdk/src/loader_instruction.rs +++ b/sdk/src/loader_instruction.rs @@ -1,6 +1,8 @@ use crate::instruction::{AccountMeta, Instruction}; use crate::pubkey::Pubkey; use crate::sysvar::rent; +use bincode::serialize; +use serde::Serialize; #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub enum LoaderInstruction { @@ -34,6 +36,7 @@ pub enum LoaderInstruction { }, } +/// Create an instruction to write program data into an account pub fn write( account_pubkey: &Pubkey, program_id: &Pubkey, @@ -48,6 +51,7 @@ pub fn write( ) } +/// Create an instruction to finalize a program data account, once finalized it can no longer be modified pub fn finalize(account_pubkey: &Pubkey, program_id: &Pubkey) -> Instruction { let account_metas = vec![ AccountMeta::new(*account_pubkey, true), @@ -55,3 +59,15 @@ pub fn finalize(account_pubkey: &Pubkey, program_id: &Pubkey) -> Instruction { ]; Instruction::new(*program_id, &LoaderInstruction::Finalize, account_metas) } + +// Create an instruction to Invoke a program's "main" entrypoint with the given data +pub fn invoke_main( + program_id: &Pubkey, + data: &T, + account_metas: Vec, +) -> Instruction { + let ix_data = LoaderInstruction::InvokeMain { + data: serialize(data).unwrap().to_vec(), + }; + Instruction::new(*program_id, &ix_data, account_metas) +}