Skip to content

Commit

Permalink
v1.18: Cleanup - disable_bpf_loader_instructions (backport of #35164)…
Browse files Browse the repository at this point in the history 
… (#35209)

Cleanup - `disable_bpf_loader_instructions` (#35164)

* Cleans up disable_bpf_loader_instructions.

* fix test_program_sbf_disguised_as_sbf_loader

* remove bpf loader execute bench

* Revert "remove bpf loader execute bench"

This reverts commit f3042ee.

* move test utility functions out of test file

* update bench to loader v3

* clippy

* fix dev-context build

* fix dev-context import

* dev-context-util

* move dev-context-util attr to module level for loader_utils

---------

Co-authored-by: HaoranYi <[email protected]>
(cherry picked from commit d472725)

Co-authored-by: Alexander Meißner <[email protected]>
  • Loading branch information
@mergify @Lichtso
mergify[bot] and Lichtso authored Feb 16, 2024
1 parent 3f42d8a commit bc676ff
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 503 deletions.
213 changes: 8 additions & 205 deletions programs/bpf_loader/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ use {
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
feature_set::{
bpf_account_data_direct_mapping, deprecate_executable_meta_update_in_bpf_loader,
disable_bpf_loader_instructions, enable_bpf_loader_extend_program_ix,
enable_bpf_loader_set_authority_checked_ix, FeatureSet,
enable_bpf_loader_extend_program_ix, enable_bpf_loader_set_authority_checked_ix,
FeatureSet,
},
instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction,
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
native_loader,
program_utils::limited_deserialize,
Expand Down Expand Up @@ -387,7 +386,11 @@ pub fn process_instruction_inner(
process_loader_upgradeable_instruction(invoke_context)
} else if bpf_loader::check_id(program_id) {
invoke_context.consume_checked(DEFAULT_LOADER_COMPUTE_UNITS)?;
process_loader_instruction(invoke_context)
ic_logger_msg!(
log_collector,
"BPF loader management instructions are no longer supported",
);
Err(InstructionError::UnsupportedProgramId)
} else if bpf_loader_deprecated::check_id(program_id) {
invoke_context.consume_checked(DEPRECATED_LOADER_COMPUTE_UNITS)?;
ic_logger_msg!(log_collector, "Deprecated loader is no longer supported");
Expand Down Expand Up @@ -1358,72 +1361,6 @@ fn common_close_account(
Ok(())
}

fn process_loader_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> {
let transaction_context = &invoke_context.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?;
let instruction_data = instruction_context.get_instruction_data();
let program_id = instruction_context.get_last_program_key(transaction_context)?;
let mut program = instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
if program.get_owner() != program_id {
ic_msg!(
invoke_context,
"Executable account not owned by the BPF loader"
);
return Err(InstructionError::IncorrectProgramId);
}

// Return `UnsupportedProgramId` error for bpf_loader when
// `disable_bpf_loader_instruction` feature is activated.
if invoke_context
.feature_set
.is_active(&disable_bpf_loader_instructions::id())
{
ic_msg!(
invoke_context,
"BPF loader management instructions are no longer supported"
);
return Err(InstructionError::UnsupportedProgramId);
}

let is_program_signer = program.is_signer();
match limited_deserialize(instruction_data)? {
LoaderInstruction::Write { offset, bytes } => {
if !is_program_signer {
ic_msg!(invoke_context, "Program account did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
drop(program);
write_program_data(offset as usize, &bytes, invoke_context)?;
}
LoaderInstruction::Finalize => {
if !is_program_signer {
ic_msg!(invoke_context, "key[0] did not sign the transaction");
return Err(InstructionError::MissingRequiredSignature);
}
deploy_program!(
invoke_context,
*program.get_key(),
program.get_owner(),
program.get_data().len(),
invoke_context.programs_loaded_for_tx_batch.slot(),
{},
program.get_data(),
);

// `deprecate_executable_meta_update_in_bpf_loader` feature doesn't
// apply to bpf_loader v2. Instead, the deployment by bpf_loader
// will be deprecated by its own feature
// `disable_bpf_loader_instructions`. Before we activate
// deprecate_executable_meta_update_in_bpf_loader, we should
// activate `disable_bpf_loader_instructions` first.
program.set_executable(true)?;
ic_msg!(invoke_context, "Finalized account {:?}", program.get_key());
}
}

Ok(())
}

fn execute<'a, 'b: 'a>(
executable: &'a Executable<InvokeContext<'static>>,
invoke_context: &'a mut InvokeContext<'b>,
Expand Down Expand Up @@ -1703,7 +1640,6 @@ mod tests {
Entrypoint::vm,
|invoke_context| {
let mut features = FeatureSet::all_enabled();
features.deactivate(&disable_bpf_loader_instructions::id());
features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
invoke_context.feature_set = Arc::new(features);
test_utils::load_all_invoked_programs(invoke_context);
Expand All @@ -1724,137 +1660,6 @@ mod tests {
program_account
}

#[test]
fn test_bpf_loader_write() {
let loader_id = bpf_loader::id();
let program_id = Pubkey::new_unique();
let mut program_account = AccountSharedData::new(1, 0, &loader_id);
let instruction_data = bincode::serialize(&LoaderInstruction::Write {
offset: 3,
bytes: vec![1, 2, 3],
})
.unwrap();

// Case: No program account
process_instruction(
&loader_id,
&[],
&instruction_data,
Vec::new(),
Vec::new(),
Err(InstructionError::NotEnoughAccountKeys),
);

// Case: Not signed
process_instruction(
&loader_id,
&[],
&instruction_data,
vec![(program_id, program_account.clone())],
vec![AccountMeta {
pubkey: program_id,
is_signer: false,
is_writable: true,
}],
Err(InstructionError::MissingRequiredSignature),
);

// Case: Write bytes to an offset
program_account.set_data(vec![0; 6]);
let accounts = process_instruction(
&loader_id,
&[],
&instruction_data,
vec![(program_id, program_account.clone())],
vec![AccountMeta {
pubkey: program_id,
is_signer: true,
is_writable: true,
}],
Ok(()),
);
assert_eq!(&vec![0, 0, 0, 1, 2, 3], accounts.first().unwrap().data());

// Case: Overflow
program_account.set_data(vec![0; 5]);
process_instruction(
&loader_id,
&[],
&instruction_data,
vec![(program_id, program_account)],
vec![AccountMeta {
pubkey: program_id,
is_signer: true,
is_writable: true,
}],
Err(InstructionError::AccountDataTooSmall),
);
}

#[test]
fn test_bpf_loader_finalize() {
let loader_id = bpf_loader::id();
let program_id = Pubkey::new_unique();
let mut program_account =
load_program_account_from_elf(&loader_id, "test_elfs/out/noop_aligned.so");
program_account.set_executable(false);
let instruction_data = bincode::serialize(&LoaderInstruction::Finalize).unwrap();

// Case: No program account
process_instruction(
&loader_id,
&[],
&instruction_data,
Vec::new(),
Vec::new(),
Err(InstructionError::NotEnoughAccountKeys),
);

// Case: Not signed
process_instruction(
&loader_id,
&[],
&instruction_data,
vec![(program_id, program_account.clone())],
vec![AccountMeta {
pubkey: program_id,
is_signer: false,
is_writable: true,
}],
Err(InstructionError::MissingRequiredSignature),
);

// Case: Finalize
let accounts = process_instruction(
&loader_id,
&[],
&instruction_data,
vec![(program_id, program_account.clone())],
vec![AccountMeta {
pubkey: program_id,
is_signer: true,
is_writable: true,
}],
Ok(()),
);
assert!(accounts.first().unwrap().executable());

// Case: Finalize bad ELF
*program_account.data_as_mut_slice().get_mut(0).unwrap() = 0;
process_instruction(
&loader_id,
&[],
&instruction_data,
vec![(program_id, program_account)],
vec![AccountMeta {
pubkey: program_id,
is_signer: true,
is_writable: true,
}],
Err(InstructionError::InvalidAccountData),
);
}

#[test]
fn test_bpf_loader_invoke_main() {
let loader_id = bpf_loader::id();
Expand All @@ -1876,7 +1681,7 @@ mod tests {
&[],
Vec::new(),
Vec::new(),
Err(InstructionError::NotEnoughAccountKeys),
Err(InstructionError::UnsupportedProgramId),
);

// Case: Only a program account
Expand Down Expand Up @@ -1926,7 +1731,6 @@ mod tests {
Entrypoint::vm,
|invoke_context| {
let mut features = FeatureSet::all_enabled();
features.deactivate(&disable_bpf_loader_instructions::id());
features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
invoke_context.feature_set = Arc::new(features);
invoke_context.mock_set_remaining(0);
Expand Down Expand Up @@ -2476,7 +2280,6 @@ mod tests {
Entrypoint::vm,
|invoke_context| {
let mut features = FeatureSet::all_enabled();
features.deactivate(&disable_bpf_loader_instructions::id());
features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
invoke_context.feature_set = Arc::new(features);
},
Expand Down
1 change: 1 addition & 0 deletions programs/sbf/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ solana_rbpf = { workspace = true }

[dev-dependencies]
solana-ledger = { workspace = true }
solana-runtime = { workspace = true, features = ["dev-context-only-utils"] }
solana-sdk = { workspace = true, features = ["dev-context-only-utils"] }

[[bench]]
Expand Down
26 changes: 13 additions & 13 deletions programs/sbf/benches/bpf_loader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@

use {
solana_rbpf::memory_region::MemoryState,
solana_sdk::feature_set::bpf_account_data_direct_mapping, std::slice,
solana_sdk::{feature_set::bpf_account_data_direct_mapping, signer::keypair::Keypair},
std::slice,
};

extern crate test;
Expand All @@ -27,7 +28,7 @@ use {
bank::Bank,
bank_client::BankClient,
genesis_utils::{create_genesis_config, GenesisConfigInfo},
loader_utils::{load_program, load_program_from_file},
loader_utils::{load_program_from_file, load_upgradeable_program_and_advance_slot},
},
solana_sdk::{
account::AccountSharedData,
Expand Down Expand Up @@ -190,12 +191,6 @@ fn bench_program_execute_noop(bencher: &mut Bencher) {
..
} = create_genesis_config(50);

// deactivate `disable_bpf_loader_instructions` feature so that the program
// can be loaded, finalized and benched.
genesis_config
.accounts
.remove(&feature_set::disable_bpf_loader_instructions::id());

genesis_config
.accounts
.remove(&feature_set::deprecate_executable_meta_update_in_bpf_loader::id());
Expand All @@ -204,12 +199,17 @@ fn bench_program_execute_noop(bencher: &mut Bencher) {
let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests();
let mut bank_client = BankClient::new_shared(bank.clone());

let invoke_program_id = load_program(&bank_client, &bpf_loader::id(), &mint_keypair, "noop");
let bank = bank_client
.advance_slot(1, bank_forks.as_ref(), &Pubkey::default())
.expect("Failed to advance the slot");

let authority_keypair = Keypair::new();
let mint_pubkey = mint_keypair.pubkey();

let (_, invoke_program_id) = load_upgradeable_program_and_advance_slot(
&mut bank_client,
bank_forks.as_ref(),
&mint_keypair,
&authority_keypair,
"noop",
);

let account_metas = vec![AccountMeta::new(mint_pubkey, true)];

let instruction =
Expand Down
Loading

0 comments on commit bc676ff

Please sign in to comment.