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 - Infer account executable flag from owners #2034

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
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
1 change: 1 addition & 0 deletions ledger-tool/src/program.rs
Original file line number Diff line number Diff line change
@@ -544,6 +544,7 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
.get_current_instruction_context()
.unwrap(),
true, // copy_account_data
Some(invoke_context.program_cache_for_tx_batch),
)
.unwrap();

9 changes: 7 additions & 2 deletions program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ use {
bpf_loader_deprecated,
clock::Slot,
epoch_schedule::EpochSchedule,
feature_set::FeatureSet,
feature_set::{infer_account_is_executable_flag, FeatureSet},
hash::Hash,
instruction::{AccountMeta, InstructionError},
native_loader,
@@ -434,7 +434,12 @@ impl<'a> InvokeContext<'a> {
})?;
let borrowed_program_account = instruction_context
.try_borrow_instruction_account(self.transaction_context, program_account_index)?;
if !borrowed_program_account.is_executable() {
#[allow(deprecated)]
if !self
.get_feature_set()
.is_active(&infer_account_is_executable_flag::id())
&& !borrowed_program_account.is_executable()
{
ic_msg!(self, "Account {} is not executable", callee_program_id);
return Err(InstructionError::AccountNotExecutable);
}
1 change: 1 addition & 0 deletions program-test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -136,6 +136,7 @@ pub fn invoke_builtin_function(
.transaction_context
.get_current_instruction_context()?,
true, // copy_account_data // There is no VM so direct mapping can not be implemented here
Some(invoke_context.program_cache_for_tx_batch),
)?;

// Deserialize data back into instruction params
64 changes: 55 additions & 9 deletions programs/bpf_loader/benches/serialization.rs
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@ extern crate test;

use {
solana_bpf_loader_program::serialization::serialize_parameters,
solana_program_runtime::loaded_programs::{ProgramCacheForTxBatch, ProgramRuntimeEnvironments},
solana_sdk::{
account::{Account, AccountSharedData},
bpf_loader, bpf_loader_deprecated,
@@ -125,8 +126,16 @@ fn bench_serialize_unaligned(bencher: &mut Bencher) {
let instruction_context = transaction_context
.get_current_instruction_context()
.unwrap();
let program_cache_for_tx_batch =
ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0);
bencher.iter(|| {
let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap();
let _ = serialize_parameters(
&transaction_context,
instruction_context,
false,
Some(&program_cache_for_tx_batch),
)
.unwrap();
});
}

@@ -136,8 +145,16 @@ fn bench_serialize_unaligned_copy_account_data(bencher: &mut Bencher) {
let instruction_context = transaction_context
.get_current_instruction_context()
.unwrap();
let program_cache_for_tx_batch =
ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0);
bencher.iter(|| {
let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap();
let _ = serialize_parameters(
&transaction_context,
instruction_context,
true,
Some(&program_cache_for_tx_batch),
)
.unwrap();
});
}

@@ -147,9 +164,16 @@ fn bench_serialize_aligned(bencher: &mut Bencher) {
let instruction_context = transaction_context
.get_current_instruction_context()
.unwrap();

let program_cache_for_tx_batch =
ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0);
bencher.iter(|| {
let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap();
let _ = serialize_parameters(
&transaction_context,
instruction_context,
false,
Some(&program_cache_for_tx_batch),
)
.unwrap();
});
}

@@ -159,9 +183,16 @@ fn bench_serialize_aligned_copy_account_data(bencher: &mut Bencher) {
let instruction_context = transaction_context
.get_current_instruction_context()
.unwrap();

let program_cache_for_tx_batch =
ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0);
bencher.iter(|| {
let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap();
let _ = serialize_parameters(
&transaction_context,
instruction_context,
true,
Some(&program_cache_for_tx_batch),
)
.unwrap();
});
}

@@ -171,8 +202,16 @@ fn bench_serialize_unaligned_max_accounts(bencher: &mut Bencher) {
let instruction_context = transaction_context
.get_current_instruction_context()
.unwrap();
let program_cache_for_tx_batch =
ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0);
bencher.iter(|| {
let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap();
let _ = serialize_parameters(
&transaction_context,
instruction_context,
false,
Some(&program_cache_for_tx_batch),
)
.unwrap();
});
}

@@ -182,8 +221,15 @@ fn bench_serialize_aligned_max_accounts(bencher: &mut Bencher) {
let instruction_context = transaction_context
.get_current_instruction_context()
.unwrap();

let program_cache_for_tx_batch =
ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0);
bencher.iter(|| {
let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap();
let _ = serialize_parameters(
&transaction_context,
instruction_context,
false,
Some(&program_cache_for_tx_batch),
)
.unwrap();
});
}
66 changes: 54 additions & 12 deletions programs/bpf_loader/src/lib.rs
Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@ use {
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
feature_set::{
bpf_account_data_direct_mapping, enable_bpf_loader_set_authority_checked_ix,
infer_account_is_executable_flag,
},
instruction::{AccountMeta, InstructionError},
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
@@ -430,7 +431,12 @@ pub fn process_instruction_inner(
}

// Program Invocation
if !program_account.is_executable() {
#[allow(deprecated)]
if !invoke_context
.get_feature_set()
.is_active(&infer_account_is_executable_flag::id())
&& !program_account.is_executable()
{
ic_logger_msg!(log_collector, "Program is not executable");
return Err(Box::new(InstructionError::IncorrectProgramId));
}
@@ -718,7 +724,12 @@ fn process_loader_upgradeable_instruction(

let program =
instruction_context.try_borrow_instruction_account(transaction_context, 1)?;
if !program.is_executable() {
#[allow(deprecated)]
if !invoke_context
.get_feature_set()
.is_active(&infer_account_is_executable_flag::id())
&& !program.is_executable()
{
ic_logger_msg!(log_collector, "Program account not executable");
return Err(InstructionError::AccountNotExecutable);
}
@@ -1367,6 +1378,10 @@ fn execute<'a, 'b: 'a>(
invoke_context.transaction_context,
instruction_context,
!direct_mapping,
invoke_context
.get_feature_set()
.is_active(&infer_account_is_executable_flag::id())
.then_some(invoke_context.program_cache_for_tx_batch),
)?;
serialize_time.stop();

@@ -1457,13 +1472,20 @@ fn execute<'a, 'b: 'a>(
instruction_account_index as IndexOfAccount,
)?;

error = EbpfError::SyscallError(Box::new(if account.is_executable() {
InstructionError::ExecutableDataModified
} else if account.is_writable() {
InstructionError::ExternalAccountDataModified
} else {
InstructionError::ReadonlyDataModified
}));
error = EbpfError::SyscallError(Box::new(
#[allow(deprecated)]
if !invoke_context
.get_feature_set()
.is_active(&infer_account_is_executable_flag::id())
&& account.is_executable()
{
InstructionError::ExecutableDataModified
} else if account.is_writable() {
InstructionError::ExternalAccountDataModified
} else {
InstructionError::ReadonlyDataModified
},
));
}
}
}
@@ -1701,13 +1723,21 @@ mod tests {

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

@@ -2382,10 +2412,22 @@ mod tests {
.unwrap()
.1
.set_executable(false);
process_instruction(
let instruction_data = bincode::serialize(&UpgradeableLoaderInstruction::Upgrade).unwrap();
mock_process_instruction(
&bpf_loader_upgradeable::id(),
Vec::new(),
&instruction_data,
transaction_accounts,
instruction_accounts,
Err(InstructionError::AccountNotExecutable),
Entrypoint::vm,
|invoke_context| {
let mut feature_set = invoke_context.get_feature_set().clone();
feature_set.deactivate(&infer_account_is_executable_flag::id());
invoke_context.mock_set_feature_set(Arc::new(feature_set));
test_utils::load_all_invoked_programs(invoke_context);
},
|_invoke_context| {},
);

// Case: Program account now owned by loader
Loading