Skip to content

Commit

Permalink
Skip deserialization of readonly accounts (#15813)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jackcmay
jackcmay authored Mar 12, 2021
1 parent 4bbeb9c commit cc38ae7
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 81 deletions.
33 changes: 18 additions & 15 deletions programs/bpf/c/src/invoke/invoke.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ 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_WRITE_DEESCALATION = 14;

static const int MINT_INDEX = 0;
static const int ARGUMENT_INDEX = 1;
Expand Down Expand Up @@ -272,6 +271,24 @@ 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: {
Expand Down Expand Up @@ -474,7 +491,6 @@ extern uint64_t entrypoint(const uint8_t *input) {
sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts));
break;
}

case TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER: {
sol_log("Test privilege deescalation escalation signer");
sol_assert(true == accounts[INVOKED_ARGUMENT_INDEX].is_signer);
Expand Down Expand Up @@ -505,19 +521,6 @@ extern uint64_t entrypoint(const uint8_t *input) {
sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)));
break;
}

case TEST_WRITE_DEESCALATION: {
sol_log("Test writable deescalation");

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));
break;
}
default:
sol_panic();
}
Expand Down
2 changes: 1 addition & 1 deletion programs/bpf/c/src/invoked/invoked.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
}

case VERIFY_PRIVILEGE_ESCALATION: {
sol_log("Should never get here!");
sol_log("Verify privilege escalation");
break;
}

Expand Down
31 changes: 21 additions & 10 deletions programs/bpf/rust/invoke/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ 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_WRITE_DEESCALATION: u8 = 14;

// const MINT_INDEX: usize = 0;
const ARGUMENT_INDEX: usize = 1;
Expand Down Expand Up @@ -354,6 +353,27 @@ fn process_instruction(
assert_eq!(data[i as usize], 42);
}
}

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]
);
}
}
TEST_PRIVILEGE_ESCALATION_SIGNER => {
msg!("Test privilege escalation signer");
Expand Down Expand Up @@ -557,15 +577,6 @@ fn process_instruction(
);
invoke(&invoked_instruction, accounts)?;
}
TEST_WRITE_DEESCALATION => {
msg!("Test writable deescalation");
let instruction = create_instruction(
*accounts[INVOKED_PROGRAM_INDEX].key,
&[(accounts[INVOKED_ARGUMENT_INDEX].key, false, false)],
vec![WRITE_ACCOUNT, 10],
);
let _ = invoke(&instruction, accounts);
}
_ => panic!(),
}

Expand Down
5 changes: 3 additions & 2 deletions programs/bpf/rust/invoked/src/processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ fn process_instruction(
}
NESTED_INVOKE => {
msg!("nested invoke");

const ARGUMENT_INDEX: usize = 0;
const INVOKED_ARGUMENT_INDEX: usize = 1;
const INVOKED_PROGRAM_INDEX: usize = 3;
Expand Down Expand Up @@ -231,8 +230,10 @@ fn process_instruction(
}
WRITE_ACCOUNT => {
msg!("write account");
const ARGUMENT_INDEX: usize = 0;

for i in 0..instruction_data[1] {
accounts[0].data.borrow_mut()[i as usize] = instruction_data[1];
accounts[ARGUMENT_INDEX].data.borrow_mut()[i as usize] = instruction_data[1];
}
}
_ => panic!(),
Expand Down
17 changes: 9 additions & 8 deletions programs/bpf/tests/programs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,13 @@ fn run_program(
vm.execute_program_jit(&mut instruction_meter)
};
assert_eq!(SUCCESS, result.unwrap());
deserialize_parameters(&bpf_loader::id(), parameter_accounts, &parameter_bytes).unwrap();
deserialize_parameters(
&bpf_loader::id(),
parameter_accounts,
&parameter_bytes,
true,
)
.unwrap();
if i == 1 {
assert_eq!(instruction_count, vm.get_total_instruction_count());
}
Expand Down Expand Up @@ -736,7 +742,6 @@ 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_WRITE_DEESCALATION: u8 = 14;

#[allow(dead_code)]
#[derive(Debug)]
Expand Down Expand Up @@ -854,6 +859,7 @@ 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(),
Expand All @@ -872,6 +878,7 @@ fn test_program_bpf_invoke_sanity() {
invoked_program_id.clone(),
invoked_program_id.clone(),
invoked_program_id.clone(),
invoked_program_id.clone(),
],
};
assert_eq!(invoked_programs.len(), expected_invoked_programs.len());
Expand Down Expand Up @@ -976,12 +983,6 @@ fn test_program_bpf_invoke_sanity() {
&[invoked_program_id.clone()],
);

do_invoke_failure_test_local(
TEST_WRITE_DEESCALATION,
TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified),
&[invoked_program_id.clone()],
);

// Check resulting state

assert_eq!(43, bank.get_balance(&derived_key1));
Expand Down
8 changes: 7 additions & 1 deletion programs/bpf_loader/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use solana_sdk::{
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::Clock,
entrypoint::SUCCESS,
feature_set::skip_ro_deserialization,
ic_logger_msg, ic_msg,
instruction::InstructionError,
keyed_account::{from_keyed_account, next_keyed_account, KeyedAccount},
Expand Down Expand Up @@ -818,7 +819,12 @@ impl Executor for BpfExecutor {
execute_time.stop();
}
let mut deserialize_time = Measure::start("deserialize");
deserialize_parameters(loader_id, parameter_accounts, &parameter_bytes)?;
deserialize_parameters(
loader_id,
parameter_accounts,
&parameter_bytes,
invoke_context.is_feature_active(&skip_ro_deserialization::id()),
)?;
deserialize_time.stop();
invoke_context.update_timing(
serialize_time.as_us(),
Expand Down
Loading

0 comments on commit cc38ae7

Please sign in to comment.