Skip to content
Merged
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions runtime/src/bank/builtins/core_bpf_migration/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ pub enum CoreBpfMigrationError {
/// Incorrect account owner
#[error("Incorrect account owner for {0:?}")]
IncorrectOwner(Pubkey),
/// Program account not executable
#[error("Program account not executable for program {0:?}")]
ProgramAccountNotExecutable(Pubkey),
/// Program has a data account
#[error("Data account exists for program {0:?}")]
ProgramHasDataAccount(Pubkey),
Expand Down
8 changes: 7 additions & 1 deletion runtime/src/bank/builtins/core_bpf_migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ impl Bank {
};
let lamports =
self.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program());
let account = AccountSharedData::new_data(lamports, &state, &bpf_loader_upgradeable::id())?;
let mut account =
AccountSharedData::new_data(lamports, &state, &bpf_loader_upgradeable::id())?;
account.set_executable(true);
Ok(account)
}

Expand Down Expand Up @@ -557,6 +559,9 @@ pub(crate) mod tests {
// Program account is owned by the upgradeable loader.
assert_eq!(program_account.owner(), &bpf_loader_upgradeable::id());

// Program account is executable.
assert!(program_account.executable());

// Program account has the correct state, with a pointer to its program
// data address.
let program_account_state: UpgradeableLoaderState = program_account.state().unwrap();
Expand Down Expand Up @@ -887,6 +892,7 @@ pub(crate) mod tests {
let owner = &bpf_loader_upgradeable::id();

let mut account = AccountSharedData::new(lamports, space, owner);
account.set_executable(true);
account.data_as_mut_slice().copy_from_slice(&data);
bank.store_account_and_update_capitalization(program_address, &account);
account
Expand Down
39 changes: 36 additions & 3 deletions runtime/src/bank/builtins/core_bpf_migration/target_core_bpf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ pub(crate) struct TargetCoreBpf {
impl TargetCoreBpf {
/// Collects the details of a Core BPF program and verifies it is properly
/// configured.
/// The program account should exist with a pointer to its data account.
/// The program account should exist with a pointer to its data account
/// and it should be marked as executable.
/// The program data account should exist with the correct state
/// (a ProgramData header and the program ELF).
pub(crate) fn new_checked(
Expand All @@ -39,6 +40,13 @@ impl TargetCoreBpf {
return Err(CoreBpfMigrationError::IncorrectOwner(*program_address));
}

// The program account should be executable.
if !program_account.executable() {
return Err(CoreBpfMigrationError::ProgramAccountNotExecutable(
*program_address,
));
}

// The program account should have a pointer to its data account.
match program_account.deserialize_data::<UpgradeableLoaderState>()? {
UpgradeableLoaderState::Program {
Expand Down Expand Up @@ -94,11 +102,11 @@ mod tests {
solana_sdk::{account::WritableAccount, bpf_loader_upgradeable},
};

fn store_account(bank: &Bank, address: &Pubkey, data: &[u8], owner: &Pubkey) {
fn store_account(bank: &Bank, address: &Pubkey, data: &[u8], owner: &Pubkey, executable: bool) {
let space = data.len();
let lamports = bank.get_minimum_balance_for_rent_exemption(space);
let mut account = AccountSharedData::new(lamports, space, owner);
account.set_executable(true);
account.set_executable(executable);
account.data_as_mut_slice().copy_from_slice(data);
bank.store_account_and_update_capitalization(address, &account);
}
Expand All @@ -125,18 +133,36 @@ mod tests {
})
.unwrap(),
&Pubkey::new_unique(), // Not the upgradeable loader
true,
);
assert_matches!(
TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(),
CoreBpfMigrationError::IncorrectOwner(..)
);

// Fail if the program account is not executable.
store_account(
&bank,
&program_address,
&bincode::serialize(&UpgradeableLoaderState::Program {
programdata_address: program_data_address,
})
.unwrap(),
&bpf_loader_upgradeable::id(),
false, // Not executable
);
assert_matches!(
TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(),
CoreBpfMigrationError::ProgramAccountNotExecutable(..)
);

// Fail if the program account does not have the correct state.
store_account(
&bank,
&program_address,
&[4u8; 200], // Not the correct state
&bpf_loader_upgradeable::id(),
true,
);
assert_matches!(
TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(),
Expand All @@ -154,6 +180,7 @@ mod tests {
})
.unwrap(),
&bpf_loader_upgradeable::id(),
true,
);
assert_matches!(
TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(),
Expand All @@ -171,6 +198,7 @@ mod tests {
})
.unwrap(),
&bpf_loader_upgradeable::id(),
true,
);

// Store the proper program account.
Expand All @@ -182,6 +210,7 @@ mod tests {
})
.unwrap(),
&bpf_loader_upgradeable::id(),
true,
);

// Fail if the program data account does not exist.
Expand All @@ -200,6 +229,7 @@ mod tests {
})
.unwrap(),
&Pubkey::new_unique(), // Not the upgradeable loader
false,
Comment thread
CriesofCarrots marked this conversation as resolved.
);
assert_matches!(
TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(),
Expand All @@ -212,6 +242,7 @@ mod tests {
&program_data_address,
&[4u8; 200], // Not the correct state
&bpf_loader_upgradeable::id(),
false,
);
assert_matches!(
TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(),
Expand All @@ -228,6 +259,7 @@ mod tests {
})
.unwrap(),
&bpf_loader_upgradeable::id(),
false,
);
assert_matches!(
TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(),
Expand Down Expand Up @@ -257,6 +289,7 @@ mod tests {
&program_data_address,
&data,
&bpf_loader_upgradeable::id(),
false,
);

let target_core_bpf = TargetCoreBpf::new_checked(&bank, &program_address).unwrap();
Expand Down