Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature - Remove accounts executable flag checks #2182

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Adjusts tests.
Lichtso committed Nov 4, 2024
commit 90a41bd862805ce7d70939d60e264be1920176a8
60 changes: 44 additions & 16 deletions programs/bpf_loader/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1685,7 +1685,7 @@ mod tests {
fn test_bpf_loader_invoke_main() {
let loader_id = bpf_loader::id();
let program_id = Pubkey::new_unique();
let mut program_account =
let program_account =
load_program_account_from_elf(&loader_id, "test_elfs/out/noop_aligned.so");
let parameter_id = Pubkey::new_unique();
let parameter_account = AccountSharedData::new(1, 0, &loader_id);
@@ -1735,7 +1735,7 @@ mod tests {
&[],
vec![
(program_id, program_account.clone()),
(parameter_id, parameter_account),
(parameter_id, parameter_account.clone()),
],
vec![parameter_meta.clone(), parameter_meta],
Ok(()),
@@ -1746,7 +1746,7 @@ mod tests {
&loader_id,
vec![0],
&[],
vec![(program_id, program_account.clone())],
vec![(program_id, program_account)],
Vec::new(),
Err(InstructionError::ProgramFailedToComplete),
Entrypoint::vm,
@@ -1758,14 +1758,29 @@ mod tests {
);

// Case: Account not a program
program_account.set_executable(false);
mock_process_instruction(
&loader_id,
vec![0],
&[],
vec![(program_id, parameter_account.clone())],
Vec::new(),
Err(InstructionError::IncorrectProgramId),
Entrypoint::vm,
|invoke_context| {
let mut feature_set = invoke_context.get_feature_set().clone();
feature_set.deactivate(&remove_accounts_executable_flag_checks::id());
invoke_context.mock_set_feature_set(Arc::new(feature_set));
test_utils::load_all_invoked_programs(invoke_context);
},
|_invoke_context| {},
);
process_instruction(
&loader_id,
&[0],
&[],
vec![(program_id, program_account)],
vec![(program_id, parameter_account)],
Vec::new(),
Err(InstructionError::IncorrectProgramId),
Err(InstructionError::UnsupportedProgramId),
);
}

@@ -2428,22 +2443,35 @@ mod tests {
);

// Case: Program account not executable
let (mut transaction_accounts, instruction_accounts) = get_accounts(
let (transaction_accounts, mut instruction_accounts) = get_accounts(
&buffer_address,
&upgrade_authority_address,
&upgrade_authority_address,
&elf_orig,
&elf_new,
);
transaction_accounts
.get_mut(1)
.unwrap()
.1
.set_executable(false);
process_instruction(
transaction_accounts,
instruction_accounts,
*instruction_accounts.get_mut(1).unwrap() = instruction_accounts.get(2).unwrap().clone();
let instruction_data = bincode::serialize(&UpgradeableLoaderInstruction::Upgrade).unwrap();
mock_process_instruction(
&bpf_loader_upgradeable::id(),
Vec::new(),
&instruction_data,
transaction_accounts.clone(),
instruction_accounts.clone(),
Err(InstructionError::AccountNotExecutable),
Entrypoint::vm,
|invoke_context| {
let mut feature_set = invoke_context.get_feature_set().clone();
feature_set.deactivate(&remove_accounts_executable_flag_checks::id());
invoke_context.mock_set_feature_set(Arc::new(feature_set));
test_utils::load_all_invoked_programs(invoke_context);
},
|_invoke_context| {},
);
process_instruction(
transaction_accounts.clone(),
instruction_accounts.clone(),
Err(InstructionError::InvalidAccountData),
);

// Case: Program account now owned by loader
@@ -3642,7 +3670,7 @@ mod tests {
(program_address, program_account.clone()),
],
Vec::new(),
Err(InstructionError::InvalidAccountData),
Err(InstructionError::UnsupportedProgramId),
);

// Case: Reopen should fail
16 changes: 8 additions & 8 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
@@ -918,15 +918,15 @@ fn test_program_sbf_invoke_sanity() {

do_invoke_failure_test_local(
TEST_PPROGRAM_NOT_OWNED_BY_LOADER,
TransactionError::InstructionError(0, InstructionError::AccountNotExecutable),
&[],
TransactionError::InstructionError(0, InstructionError::UnsupportedProgramId),
&[argument_keypair.pubkey()],
None,
);

do_invoke_failure_test_local(
TEST_PPROGRAM_NOT_EXECUTABLE,
TransactionError::InstructionError(0, InstructionError::AccountNotExecutable),
&[],
TransactionError::InstructionError(0, InstructionError::UnsupportedProgramId),
&[unexecutable_program_keypair.pubkey()],
None,
);

@@ -1895,7 +1895,7 @@ fn test_program_sbf_invoke_in_same_tx_as_deployment() {
let (result, _, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(2, InstructionError::InvalidAccountData),
TransactionError::InstructionError(2, InstructionError::UnsupportedProgramId),
);
}
}
@@ -2006,7 +2006,7 @@ fn test_program_sbf_invoke_in_same_tx_as_redeployment() {
let (result, _, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(1, InstructionError::InvalidAccountData),
TransactionError::InstructionError(1, InstructionError::UnsupportedProgramId),
);
}
}
@@ -2101,7 +2101,7 @@ fn test_program_sbf_invoke_in_same_tx_as_undeployment() {
let (result, _, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(1, InstructionError::InvalidAccountData),
TransactionError::InstructionError(1, InstructionError::UnsupportedProgramId),
);
}
}
@@ -4650,7 +4650,7 @@ fn test_deny_executable_write() {
let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
assert_eq!(
result.unwrap_err().unwrap(),
TransactionError::InstructionError(0, InstructionError::ExecutableDataModified)
TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified)
);
}
}
4 changes: 2 additions & 2 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
@@ -7231,7 +7231,7 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
bank.process_transaction(&transaction),
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidAccountData
InstructionError::UnsupportedProgramId
)),
);
{
@@ -7255,7 +7255,7 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
bank.process_transaction(&transaction),
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidAccountData,
InstructionError::UnsupportedProgramId,
)),
);
{
10 changes: 9 additions & 1 deletion svm/src/account_loader.rs
Original file line number Diff line number Diff line change
@@ -886,7 +886,15 @@ mod tests {
instructions,
);

let load_results = load_accounts_aux_test(tx, &accounts, &mut error_metrics);
let mut feature_set = FeatureSet::all_enabled();
feature_set.deactivate(&feature_set::remove_accounts_executable_flag_checks::id());
let load_results = load_accounts_with_features_and_rent(
tx,
&accounts,
&RentCollector::default(),
&mut error_metrics,
&mut feature_set,
);

assert_eq!(error_metrics.invalid_program_for_execution, 1);
assert_eq!(load_results.len(), 1);