From 2f7f243022c731400563f4ca8dfcf978d26eb76b Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 28 May 2021 20:34:10 +0000 Subject: [PATCH] Always bail if program modifies a ro account (backport #17569) (#17584) * Always bail if program modifies a ro account (#17569) (cherry picked from commit a3240aebdebeb576d421d9f0724503ba6811ef89) * resolve conflicts * nudge Co-authored-by: Jack May --- programs/bpf/Cargo.lock | 7 ++ programs/bpf/Cargo.toml | 1 + programs/bpf/build.rs | 1 + programs/bpf/c/src/invoke/invoke.c | 38 ++++----- programs/bpf/rust/invoke/src/lib.rs | 41 +++++----- .../bpf/rust/ro_account_modify/Cargo.toml | 19 +++++ .../bpf/rust/ro_account_modify/src/lib.rs | 70 +++++++++++++++++ programs/bpf/tests/programs.rs | 77 ++++++++++++++++++- programs/bpf_loader/src/lib.rs | 9 +-- programs/bpf_loader/src/serialization.rs | 55 ++++--------- programs/bpf_loader/src/syscalls.rs | 19 +---- runtime/src/message_processor.rs | 52 ++++--------- sdk/src/feature_set.rs | 10 --- 13 files changed, 248 insertions(+), 151 deletions(-) create mode 100644 programs/bpf/rust/ro_account_modify/Cargo.toml create mode 100644 programs/bpf/rust/ro_account_modify/src/lib.rs diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index cb57a48a5f..932756a963 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -3007,6 +3007,13 @@ dependencies = [ "solana-program 1.7.0", ] +[[package]] +name = "solana-bpf-rust-ro-account_modify" +version = "1.7.0" +dependencies = [ + "solana-program 1.7.0", +] + [[package]] name = "solana-bpf-rust-ro-modify" version = "1.7.0" diff --git a/programs/bpf/Cargo.toml b/programs/bpf/Cargo.toml index 8bb3d8097f..ceed8470bd 100644 --- a/programs/bpf/Cargo.toml +++ b/programs/bpf/Cargo.toml @@ -70,6 +70,7 @@ members = [ "rust/param_passing_dep", "rust/rand", "rust/ro_modify", + "rust/ro_account_modify", "rust/sanity", "rust/sha", "rust/spoof1", diff --git a/programs/bpf/build.rs b/programs/bpf/build.rs index a044671a04..6c7ad480ef 100644 --- a/programs/bpf/build.rs +++ b/programs/bpf/build.rs @@ -83,6 +83,7 @@ fn main() { "param_passing", "rand", "ro_modify", + "ro_account_modify", "sanity", "sha", "spoof1", diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index 0ae306220e..88e6f4a9bb 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -17,6 +17,7 @@ static const uint8_t TEST_INSTRUCTION_META_TOO_LARGE = 10; static const uint8_t TEST_RETURN_ERROR = 11; static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER = 12; static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE = 13; +static const uint8_t TEST_WRITABLE_DEESCALATION_WRITABLE = 14; static const int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -271,24 +272,6 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_assert(accounts[ARGUMENT_INDEX].data[i] == 0); } } - sol_log("Test writable deescalation"); - { - uint8_t buffer[10]; - for (int i = 0; i < 10; i++) { - buffer[i] = accounts[INVOKED_ARGUMENT_INDEX].data[i]; - } - SolAccountMeta arguments[] = { - {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}}; - uint8_t data[] = {WRITE_ACCOUNT, 10}; - const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, - arguments, SOL_ARRAY_SIZE(arguments), - data, SOL_ARRAY_SIZE(data)}; - sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)); - - for (int i = 0; i < 10; i++) { - sol_assert(buffer[i] == accounts[INVOKED_ARGUMENT_INDEX].data[i]); - } - } break; } case TEST_PRIVILEGE_ESCALATION_SIGNER: { @@ -521,6 +504,25 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts))); break; } + case TEST_WRITABLE_DEESCALATION_WRITABLE: { + sol_log("Test writable deescalation"); + uint8_t buffer[10]; + for (int i = 0; i < 10; i++) { + buffer[i] = accounts[INVOKED_ARGUMENT_INDEX].data[i]; + } + SolAccountMeta arguments[] = { + {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}}; + uint8_t data[] = {WRITE_ACCOUNT, 10}; + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)); + + for (int i = 0; i < 10; i++) { + sol_assert(buffer[i] == accounts[INVOKED_ARGUMENT_INDEX].data[i]); + } + break; + } default: sol_panic(); } diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index e0889766ee..cc15b015e9 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -29,6 +29,7 @@ const TEST_INSTRUCTION_META_TOO_LARGE: u8 = 10; const TEST_RETURN_ERROR: u8 = 11; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER: u8 = 12; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13; +const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14; // const MINT_INDEX: usize = 0; const ARGUMENT_INDEX: usize = 1; @@ -354,27 +355,6 @@ fn process_instruction( } } - msg!("Test writable deescalation"); - { - const NUM_BYTES: usize = 10; - let mut buffer = [0; NUM_BYTES]; - buffer.copy_from_slice( - &accounts[INVOKED_ARGUMENT_INDEX].data.borrow_mut()[..NUM_BYTES], - ); - - let instruction = create_instruction( - *accounts[INVOKED_PROGRAM_INDEX].key, - &[(accounts[INVOKED_ARGUMENT_INDEX].key, false, false)], - vec![WRITE_ACCOUNT, NUM_BYTES as u8], - ); - let _ = invoke(&instruction, accounts); - - assert_eq!( - buffer, - accounts[INVOKED_ARGUMENT_INDEX].data.borrow_mut()[..NUM_BYTES] - ); - } - msg!("Create account and init data"); { let from_lamports = accounts[FROM_INDEX].lamports(); @@ -603,6 +583,25 @@ fn process_instruction( ); invoke(&invoked_instruction, accounts)?; } + TEST_WRITABLE_DEESCALATION_WRITABLE => { + msg!("Test writable deescalation writable"); + const NUM_BYTES: usize = 10; + let mut buffer = [0; NUM_BYTES]; + buffer + .copy_from_slice(&accounts[INVOKED_ARGUMENT_INDEX].data.borrow_mut()[..NUM_BYTES]); + + let instruction = create_instruction( + *accounts[INVOKED_PROGRAM_INDEX].key, + &[(accounts[INVOKED_ARGUMENT_INDEX].key, false, false)], + vec![WRITE_ACCOUNT, NUM_BYTES as u8], + ); + let _ = invoke(&instruction, accounts); + + assert_eq!( + buffer, + accounts[INVOKED_ARGUMENT_INDEX].data.borrow_mut()[..NUM_BYTES] + ); + } _ => panic!(), } diff --git a/programs/bpf/rust/ro_account_modify/Cargo.toml b/programs/bpf/rust/ro_account_modify/Cargo.toml new file mode 100644 index 0000000000..6f3996d90e --- /dev/null +++ b/programs/bpf/rust/ro_account_modify/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "solana-bpf-rust-ro-account_modify" +version = "1.7.0" +description = "Solana BPF test program written in Rust" +authors = ["Solana Maintainers "] +repository = "https://github.com/solana-labs/solana" +license = "Apache-2.0" +homepage = "https://solana.com/" +documentation = "https://docs.rs/solana-bpf-rust-ro-modify" +edition = "2018" + +[dependencies] +solana-program = { path = "../../../../sdk/program", version = "=1.7.0" } + +[lib] +crate-type = ["cdylib"] + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] diff --git a/programs/bpf/rust/ro_account_modify/src/lib.rs b/programs/bpf/rust/ro_account_modify/src/lib.rs new file mode 100644 index 0000000000..8a69f09a89 --- /dev/null +++ b/programs/bpf/rust/ro_account_modify/src/lib.rs @@ -0,0 +1,70 @@ +use solana_program::{ + account_info::AccountInfo, + entrypoint, + entrypoint::ProgramResult, + instruction::{AccountMeta, Instruction}, + msg, + program::invoke, + pubkey::Pubkey, +}; + +const ARGUMENT_INDEX: usize = 0; + +const INSTRUCTION_MODIFY: u8 = 0; +const INSTRUCTION_INVOKE_MODIFY: u8 = 1; +const INSTRUCTION_MODIFY_INVOKE: u8 = 2; +const INSTRUCTION_VERIFY_MODIFIED: u8 = 3; + +entrypoint!(process_instruction); +fn process_instruction( + program_id: &Pubkey, + accounts: &[AccountInfo], + instruction_data: &[u8], +) -> ProgramResult { + assert!(!accounts[ARGUMENT_INDEX].is_writable); + + match instruction_data[0] { + INSTRUCTION_MODIFY => { + msg!("modify ro account"); + assert_eq!(0, accounts[ARGUMENT_INDEX].try_borrow_data()?[0]); + accounts[ARGUMENT_INDEX].try_borrow_mut_data()?[0] = 1; + } + INSTRUCTION_INVOKE_MODIFY => { + msg!("invoke and modify ro account"); + + assert_eq!(0, accounts[ARGUMENT_INDEX].try_borrow_data()?[0]); + + let instruction = Instruction { + program_id: *program_id, + accounts: vec![AccountMeta::new_readonly( + *accounts[ARGUMENT_INDEX].key, + false, + )], + data: vec![INSTRUCTION_MODIFY], + }; + invoke(&instruction, accounts)?; + } + INSTRUCTION_MODIFY_INVOKE => { + msg!("modify and invoke ro account"); + + assert_eq!(0, accounts[ARGUMENT_INDEX].try_borrow_data()?[0]); + accounts[ARGUMENT_INDEX].try_borrow_mut_data()?[0] = 1; + + let instruction = Instruction { + program_id: *program_id, + accounts: vec![AccountMeta::new_readonly( + *accounts[ARGUMENT_INDEX].key, + false, + )], + data: vec![INSTRUCTION_VERIFY_MODIFIED], + }; + invoke(&instruction, accounts)?; + } + INSTRUCTION_VERIFY_MODIFIED => { + msg!("verify modified"); + assert_eq!(1, accounts[ARGUMENT_INDEX].try_borrow_data()?[0]) + } + _ => panic!("Unknown instruction"), + } + Ok(()) +} diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 8b0710c0f8..80b88b0b2f 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -17,7 +17,7 @@ use solana_bpf_loader_program::{ use solana_cli_output::display::println_transaction; use solana_rbpf::{ static_analysis::Analysis, - vm::{Config, Executable, Tracer} + vm::{Config, Executable, Tracer}, }; use solana_runtime::{ bank::{Bank, ExecuteTimings, NonceRollbackInfo, TransactionBalancesSet, TransactionResults}, @@ -278,7 +278,6 @@ fn run_program( &bpf_loader::id(), parameter_accounts, parameter_bytes.as_slice(), - true, ) .unwrap(); } @@ -757,6 +756,7 @@ fn test_program_bpf_invoke_sanity() { const TEST_RETURN_ERROR: u8 = 11; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER: u8 = 12; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13; + const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14; #[allow(dead_code)] #[derive(Debug)] @@ -874,7 +874,6 @@ fn test_program_bpf_invoke_sanity() { invoked_program_id.clone(), invoked_program_id.clone(), invoked_program_id.clone(), - invoked_program_id.clone(), ], Languages::Rust => vec![ solana_sdk::system_program::id(), @@ -894,7 +893,6 @@ fn test_program_bpf_invoke_sanity() { invoked_program_id.clone(), invoked_program_id.clone(), invoked_program_id.clone(), - invoked_program_id.clone(), solana_sdk::system_program::id(), ], }; @@ -1000,6 +998,12 @@ fn test_program_bpf_invoke_sanity() { &[invoked_program_id.clone()], ); + do_invoke_failure_test_local( + TEST_WRITABLE_DEESCALATION_WRITABLE, + TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified), + &[invoked_program_id.clone()], + ); + // Check resulting state assert_eq!(43, bank.get_balance(&derived_key1)); @@ -2454,3 +2458,68 @@ fn test_program_bpf_finalize() { TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete) ); } + +#[cfg(feature = "bpf_rust")] +#[test] +fn test_program_bpf_ro_account_modify() { + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(50); + let mut bank = Bank::new(&genesis_config); + let (name, id, entrypoint) = solana_bpf_loader_program!(); + bank.add_builtin(&name, id, entrypoint); + let bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); + + let program_id = load_bpf_program( + &bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_bpf_rust_ro_account_modify", + ); + + let argument_keypair = Keypair::new(); + let account = AccountSharedData::new(42, 100, &program_id); + bank.store_account(&argument_keypair.pubkey(), &account); + + let from_keypair = Keypair::new(); + let account = AccountSharedData::new(84, 0, &solana_sdk::system_program::id()); + bank.store_account(&from_keypair.pubkey(), &account); + + let mint_pubkey = mint_keypair.pubkey(); + let account_metas = vec![ + AccountMeta::new_readonly(argument_keypair.pubkey(), false), + AccountMeta::new_readonly(program_id, false), + ]; + + let instruction = Instruction::new_with_bytes(program_id, &[0], account_metas.clone()); + let message = Message::new(&[instruction], Some(&mint_pubkey)); + let result = bank_client.send_and_confirm_message(&[&mint_keypair], message); + println!("result: {:?}", result); + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified) + ); + + let instruction = Instruction::new_with_bytes(program_id, &[1], account_metas.clone()); + let message = Message::new(&[instruction], Some(&mint_pubkey)); + let result = bank_client.send_and_confirm_message(&[&mint_keypair], message); + println!("result: {:?}", result); + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified) + ); + + let instruction = Instruction::new_with_bytes(program_id, &[2], account_metas.clone()); + let message = Message::new(&[instruction], Some(&mint_pubkey)); + let result = bank_client.send_and_confirm_message(&[&mint_keypair], message); + println!("result: {:?}", result); + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified) + ); +} diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 2cad10a0b5..9de3f9f312 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -32,7 +32,7 @@ use solana_sdk::{ bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::Clock, entrypoint::SUCCESS, - feature_set::{skip_ro_deserialization, upgradeable_close_instruction}, + feature_set::upgradeable_close_instruction, ic_logger_msg, ic_msg, instruction::InstructionError, keyed_account::{from_keyed_account, keyed_account_at_index}, @@ -827,12 +827,7 @@ impl Executor for BpfExecutor { } let mut deserialize_time = Measure::start("deserialize"); let keyed_accounts = invoke_context.get_keyed_accounts()?; - deserialize_parameters( - loader_id, - keyed_accounts, - parameter_bytes.as_slice(), - invoke_context.is_feature_active(&skip_ro_deserialization::id()), - )?; + deserialize_parameters(loader_id, keyed_accounts, parameter_bytes.as_slice())?; deserialize_time.stop(); invoke_context.update_timing( serialize_time.as_us(), diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs index 31683274e5..f9529c017d 100644 --- a/programs/bpf_loader/src/serialization.rs +++ b/programs/bpf_loader/src/serialization.rs @@ -40,12 +40,11 @@ pub fn deserialize_parameters( loader_id: &Pubkey, keyed_accounts: &[KeyedAccount], buffer: &[u8], - skip_ro_deserialization: bool, ) -> Result<(), InstructionError> { if *loader_id == bpf_loader_deprecated::id() { - deserialize_parameters_unaligned(keyed_accounts, buffer, skip_ro_deserialization) + deserialize_parameters_unaligned(keyed_accounts, buffer) } else { - deserialize_parameters_aligned(keyed_accounts, buffer, skip_ro_deserialization) + deserialize_parameters_aligned(keyed_accounts, buffer) } } @@ -127,33 +126,28 @@ pub fn serialize_parameters_unaligned( pub fn deserialize_parameters_unaligned( keyed_accounts: &[KeyedAccount], buffer: &[u8], - skip_ro_deserialization: bool, ) -> Result<(), InstructionError> { let mut start = size_of::(); // number of accounts for (i, keyed_account) in keyed_accounts.iter().enumerate() { let (is_dup, _) = is_dup(&keyed_accounts[..i], keyed_account); start += 1; // is_dup if !is_dup { - if keyed_account.is_writable() || !skip_ro_deserialization { - start += size_of::(); // is_signer - start += size_of::(); // is_writable - start += size_of::(); // key - keyed_account - .try_account_ref_mut()? - .set_lamports(LittleEndian::read_u64(&buffer[start..])); - start += size_of::() // lamports + start += size_of::(); // is_signer + start += size_of::(); // is_writable + start += size_of::(); // key + keyed_account + .try_account_ref_mut()? + .set_lamports(LittleEndian::read_u64(&buffer[start..])); + start += size_of::() // lamports + size_of::(); // data length - let end = start + keyed_account.data_len()?; - keyed_account - .try_account_ref_mut()? - .set_data_from_slice(&buffer[start..end]); - start += keyed_account.data_len()? // data + let end = start + keyed_account.data_len()?; + keyed_account + .try_account_ref_mut()? + .set_data_from_slice(&buffer[start..end]); + start += keyed_account.data_len()? // data + size_of::() // owner + size_of::() // executable + size_of::(); // rent_epoch - } else { - start += get_serialized_account_size_unaligned(keyed_account)?; - } } } Ok(()) @@ -253,7 +247,6 @@ pub fn serialize_parameters_aligned( pub fn deserialize_parameters_aligned( keyed_accounts: &[KeyedAccount], buffer: &[u8], - skip_ro_deserialization: bool, ) -> Result<(), InstructionError> { let mut start = size_of::(); // number of accounts for (i, keyed_account) in keyed_accounts.iter().enumerate() { @@ -261,7 +254,7 @@ pub fn deserialize_parameters_aligned( start += size_of::(); // position if is_dup { start += 7; // padding to 64-bit aligned - } else if keyed_account.is_writable() || !skip_ro_deserialization { + } else { let mut account = keyed_account.try_account_ref_mut()?; start += size_of::() // is_signer + size_of::() // is_writable @@ -286,8 +279,6 @@ pub fn deserialize_parameters_aligned( start += pre_len + MAX_PERMITTED_DATA_INCREASE; // data start += (start as *const u8).align_offset(align_of::()); start += size_of::(); // rent_epoch - } else { - start += get_serialized_account_size_aligned(keyed_account)?; } } Ok(()) @@ -454,24 +445,13 @@ mod tests { } }) .collect(); - deserialize_parameters( - &bpf_loader::id(), - &de_keyed_accounts, - serialized.as_slice(), - true, - ) - .unwrap(); + deserialize_parameters(&bpf_loader::id(), &de_keyed_accounts, serialized.as_slice()) + .unwrap(); for ((account, de_keyed_account), key) in accounts.iter().zip(de_keyed_accounts).zip(keys.clone()) { assert_eq!(key, *de_keyed_account.unsigned_key()); let account = account.borrow(); - assert_eq!(account.lamports(), de_keyed_account.lamports().unwrap()); - assert_eq!( - account.data(), - de_keyed_account.try_account_ref().unwrap().data() - ); - assert_eq!(*account.owner(), de_keyed_account.owner().unwrap()); assert_eq!(account.executable(), de_keyed_account.executable().unwrap()); assert_eq!(account.rent_epoch(), de_keyed_account.rent_epoch().unwrap()); } @@ -517,7 +497,6 @@ mod tests { &bpf_loader_deprecated::id(), &de_keyed_accounts, serialized.as_slice(), - true, ) .unwrap(); for ((account, de_keyed_account), key) in diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 15406fe1ed..b6a0da2f46 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -19,9 +19,9 @@ use solana_sdk::{ entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, epoch_schedule::EpochSchedule, feature_set::{ - cpi_data_cost, cpi_share_ro_and_exec_accounts, demote_sysvar_write_locks, - enforce_aligned_host_addrs, keccak256_syscall_enabled, - set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall, update_data_on_realloc, + cpi_data_cost, demote_sysvar_write_locks, enforce_aligned_host_addrs, + keccak256_syscall_enabled, set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall, + update_data_on_realloc, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -1169,7 +1169,6 @@ trait SyscallInvokeSigned<'a> { fn translate_accounts( &self, account_keys: &[Pubkey], - caller_write_privileges: &[bool], program_account_index: usize, account_infos_addr: u64, account_infos_len: u64, @@ -1246,7 +1245,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { fn translate_accounts( &self, account_keys: &[Pubkey], - caller_write_privileges: &[bool], program_account_index: usize, account_infos_addr: u64, account_infos_len: u64, @@ -1368,7 +1366,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { get_translated_accounts( account_keys, - caller_write_privileges, program_account_index, &account_info_keys, account_infos, @@ -1585,7 +1582,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { fn translate_accounts( &self, account_keys: &[Pubkey], - caller_write_privileges: &[bool], program_account_index: usize, account_infos_addr: u64, account_infos_len: u64, @@ -1689,7 +1685,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { get_translated_accounts( account_keys, - caller_write_privileges, program_account_index, &account_info_keys, account_infos, @@ -1779,7 +1774,6 @@ impl<'a> SyscallObject for SyscallInvokeSignedC<'a> { fn get_translated_accounts<'a, T, F>( account_keys: &[Pubkey], - caller_write_privileges: &[bool], program_account_index: usize, account_info_keys: &[&Pubkey], account_infos: &[T], @@ -1801,11 +1795,7 @@ where SyscallError::InstructionError(InstructionError::MissingAccount) })?; - if i == program_account_index - || account.borrow().executable() - || (invoke_context.is_feature_active(&cpi_share_ro_and_exec_accounts::id()) - && !caller_write_privileges[i]) - { + if i == program_account_index || account.borrow().executable() { // Use the known account accounts.push(account); refs.push(None); @@ -2000,7 +1990,6 @@ fn call<'a>( check_authorized_program(&callee_program_id, &instruction.data, &invoke_context)?; let (accounts, account_refs) = syscall.translate_accounts( &message.account_keys, - &caller_write_privileges, callee_program_id_index, account_infos_addr, account_infos_len, diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 7fa60e5fb3..15cc2c0ee2 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -8,10 +8,7 @@ use solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, account_utils::StateMut, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, - feature_set::{ - cpi_share_ro_and_exec_accounts, demote_sysvar_write_locks, instructions_sysvar_enabled, - FeatureSet, - }, + feature_set::{demote_sysvar_write_locks, instructions_sysvar_enabled, FeatureSet}, ic_msg, instruction::{CompiledInstruction, Instruction, InstructionError}, keyed_account::{create_keyed_accounts_unified, keyed_account_at_index, KeyedAccount}, @@ -254,7 +251,6 @@ pub struct ThisInvokeContext<'a> { rent: Rent, message: &'a Message, pre_accounts: Vec, - executable_accounts: &'a [(Pubkey, Rc>)], account_deps: &'a [(Pubkey, Rc>)], programs: &'a [(Pubkey, ProcessInstructionWithContext)], logger: Rc>, @@ -301,7 +297,6 @@ impl<'a> ThisInvokeContext<'a> { rent, message, pre_accounts, - executable_accounts, account_deps, programs, logger: Rc::new(RefCell::new(ThisLogger { log_collector })), @@ -454,41 +449,22 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { self.feature_set.is_active(feature_id) } fn get_account(&self, pubkey: &Pubkey) -> Option>> { - if self.is_feature_active(&cpi_share_ro_and_exec_accounts::id()) { - if let Some((_, account)) = self - .executable_accounts - .iter() - .find(|(key, _)| key == pubkey) - { - Some(account.clone()) - } else if let Some((_, account)) = - self.account_deps.iter().find(|(key, _)| key == pubkey) - { + if let Some(account) = self.pre_accounts.iter().find_map(|pre| { + if pre.key == *pubkey { + Some(pre.account.clone()) + } else { + None + } + }) { + return Some(account); + } + self.account_deps.iter().find_map(|(key, account)| { + if key == pubkey { Some(account.clone()) } else { - self.pre_accounts - .iter() - .find(|pre| pre.key == *pubkey) - .map(|pre| pre.account.clone()) + None } - } else { - if let Some(account) = self.pre_accounts.iter().find_map(|pre| { - if pre.key == *pubkey { - Some(pre.account.clone()) - } else { - None - } - }) { - return Some(account); - } - self.account_deps.iter().find_map(|(key, account)| { - if key == pubkey { - Some(account.clone()) - } else { - None - } - }) - } + }) } fn update_timing( &mut self, diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index aaccf5b074..ee56c8be9f 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -91,14 +91,6 @@ pub mod check_program_owner { solana_sdk::declare_id!("5XnbR5Es9YXEARRuP6mdvoxiW3hx5atNNeBmwVd8P3QD"); } -pub mod cpi_share_ro_and_exec_accounts { - solana_sdk::declare_id!("6VgVBi3uRVqp56TtEwNou8idgdmhCD1aYqX8FaJ1fnJb"); -} - -pub mod skip_ro_deserialization { - solana_sdk::declare_id!("6Sw5JV84f7QkDe8gvRxpcPWFnPpfpgEnNziiy8sELaCp"); -} - pub mod require_stake_for_gossip { solana_sdk::declare_id!("6oNzd5Z3M2L1xo4Q5hoox7CR2DuW7m1ETLWH5jHJthwa"); } @@ -166,8 +158,6 @@ lazy_static! { (warp_timestamp_again::id(), "warp timestamp again, adjust bounding to 25% fast 80% slow #15204"), (check_init_vote_data::id(), "check initialized Vote data"), (check_program_owner::id(), "limit programs to operating on accounts owned by itself"), - (cpi_share_ro_and_exec_accounts::id(), "share RO and Executable accounts during cross-program invocations"), - (skip_ro_deserialization::id(), "skip deserialization of read-only accounts"), (require_stake_for_gossip::id(), "require stakes for propagating crds values through gossip #15561"), (cpi_data_cost::id(), "charge the compute budget for data passed via CPI"), (upgradeable_close_instruction::id(), "close upgradeable buffer accounts"),