Skip to content

Commit

Permalink
Feature: Explicitly limit `TransactionContext::instruction_trace_capa…
Browse files Browse the repository at this point in the history
…city` (#27938)

* Renames instruction_stack_capacity => instruction_stack_capacity.

* Replaces number_of_instructions_at_transaction_level by instruction_trace_capacity.

* Adds MaxInstructionTraceLengthExceeded.

* Adjusts TransactionContext::new() parameter.

* Adds feature gate limit_max_instruction_trace_length.

* Adds test_max_instruction_trace_length().
  • Loading branch information
Lichtso authored Sep 26, 2022
1 parent de7a5f2 commit 71aee4f
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 29 deletions.
1 change: 1 addition & 0 deletions docs/src/developing/programming-model/runtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ log_u64_units: 100,
create_program address units: 1500,
invoke_units: 1000,
max_invoke_depth: 4,
max_instruction_trace_length: 64,
max_call_depth: 64,
stack_frame_size: 4096,
log_pubkey_units: 100,
Expand Down
3 changes: 3 additions & 0 deletions program-runtime/src/compute_budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub struct ComputeBudget {
pub invoke_units: u64,
/// Maximum cross-program invocation depth allowed
pub max_invoke_depth: usize,
/// Maximum cross-program invocation and instructions per transaction
pub max_instruction_trace_length: usize,
/// Base number of compute units consumed to call SHA256
pub sha256_base_cost: u64,
/// Incremental number of units consumed by SHA256 (based on bytes)
Expand Down Expand Up @@ -100,6 +102,7 @@ impl ComputeBudget {
create_program_address_units: 1500,
invoke_units: 1000,
max_invoke_depth: 4,
max_instruction_trace_length: 64,
sha256_base_cost: 85,
sha256_byte_cost: 1,
sha256_max_slices: 20_000,
Expand Down
31 changes: 24 additions & 7 deletions program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1022,11 +1022,12 @@ pub fn with_mock_invoke_context<R, F: FnMut(&mut InvokeContext) -> R>(
}];
let preparation =
prepare_mock_invoke_context(transaction_accounts, instruction_accounts, &program_indices);
let compute_budget = ComputeBudget::default();
let mut transaction_context = TransactionContext::new(
preparation.transaction_accounts,
Some(Rent::default()),
ComputeBudget::default().max_invoke_depth.saturating_add(1),
1,
compute_budget.max_invoke_depth.saturating_add(1),
compute_budget.max_instruction_trace_length,
);
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
Expand Down Expand Up @@ -1057,11 +1058,12 @@ pub fn mock_process_instruction(
preparation
.transaction_accounts
.push((*loader_id, processor_account));
let compute_budget = ComputeBudget::default();
let mut transaction_context = TransactionContext::new(
preparation.transaction_accounts,
Some(Rent::default()),
ComputeBudget::default().max_invoke_depth.saturating_add(1),
1,
compute_budget.max_invoke_depth.saturating_add(1),
compute_budget.max_instruction_trace_length,
);
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
Expand Down Expand Up @@ -1284,7 +1286,7 @@ mod tests {
accounts,
Some(Rent::default()),
ComputeBudget::default().max_invoke_depth,
1,
MAX_DEPTH,
);
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);

Expand All @@ -1309,6 +1311,21 @@ mod tests {
assert!(depth_reached < MAX_DEPTH);
}

#[test]
fn test_max_instruction_trace_length() {
const MAX_INSTRUCTIONS: usize = 8;
let mut transaction_context =
TransactionContext::new(Vec::new(), Some(Rent::default()), 1, MAX_INSTRUCTIONS);
for _ in 0..MAX_INSTRUCTIONS {
transaction_context.push().unwrap();
transaction_context.pop().unwrap();
}
assert_eq!(
transaction_context.push(),
Err(InstructionError::MaxInstructionTraceLengthExceeded)
);
}

#[test]
fn test_process_instruction() {
let callee_program_id = solana_sdk::pubkey::new_rand();
Expand Down Expand Up @@ -1345,7 +1362,7 @@ mod tests {
})
.collect::<Vec<_>>();
let mut transaction_context =
TransactionContext::new(accounts, Some(Rent::default()), 2, 9);
TransactionContext::new(accounts, Some(Rent::default()), 2, 18);
let mut invoke_context =
InvokeContext::new_mock(&mut transaction_context, builtin_programs);

Expand Down Expand Up @@ -1436,7 +1453,7 @@ mod tests {
let accounts = vec![(solana_sdk::pubkey::new_rand(), AccountSharedData::default())];

let mut transaction_context =
TransactionContext::new(accounts, Some(Rent::default()), 1, 3);
TransactionContext::new(accounts, Some(Rent::default()), 1, 1);
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
invoke_context.compute_budget =
ComputeBudget::new(compute_budget::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64);
Expand Down
18 changes: 13 additions & 5 deletions programs/bpf_loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ use {
cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts,
disable_deploy_of_alloc_free_syscall, disable_deprecated_loader,
enable_bpf_loader_extend_program_ix, error_on_syscall_bpf_function_hash_collisions,
reject_callx_r10,
limit_max_instruction_trace_length, reject_callx_r10,
},
instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction,
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
program_error::MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED,
program_error::{
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED, MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED,
},
program_utils::limited_deserialize,
pubkey::Pubkey,
saturating_add_assign,
Expand Down Expand Up @@ -1362,14 +1364,20 @@ impl Executor for BpfExecutor {
}
match result {
Ok(status) if status != SUCCESS => {
let error: InstructionError = if status
let error: InstructionError = if (status
== MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED
&& !invoke_context
.feature_set
.is_active(&cap_accounts_data_allocations_per_transaction::id())
.is_active(&cap_accounts_data_allocations_per_transaction::id()))
|| (status == MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED
&& !invoke_context
.feature_set
.is_active(&limit_max_instruction_trace_length::id()))
{
// Until the cap_accounts_data_allocations_per_transaction feature is
// enabled, map the MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED error to InvalidError
// enabled, map the `MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED` error to `InvalidError`.
// Until the limit_max_instruction_trace_length feature is
// enabled, map the `MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED` error to `InvalidError`.
InstructionError::InvalidError
} else {
status.into()
Expand Down
6 changes: 4 additions & 2 deletions programs/bpf_loader/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3870,8 +3870,10 @@ mod tests {
)
})
.collect::<Vec<_>>();
let mut transaction_context = TransactionContext::new(transaction_accounts, None, 4, 1);
for (index_in_trace, stack_height) in [1, 2, 3, 2, 2, 3, 4, 3].into_iter().enumerate() {
let instruction_trace = [1, 2, 3, 2, 2, 3, 4, 3];
let mut transaction_context =
TransactionContext::new(transaction_accounts, None, 4, instruction_trace.len());
for (index_in_trace, stack_height) in instruction_trace.into_iter().enumerate() {
while stack_height <= transaction_context.get_instruction_context_stack_height() {
transaction_context.pop().unwrap();
}
Expand Down
20 changes: 14 additions & 6 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl RentDebits {
}

pub type BankStatusCache = StatusCache<Result<()>>;
#[frozen_abi(digest = "7FSSacrCi7vf2QZFm3Ui9JqTii4U6h1XWYD3LKSuVwV8")]
#[frozen_abi(digest = "A7T7XohiSoo8FGoCPTsaXAYYugXTkoYnBjQAdBgYHH85")]
pub type BankSlotDelta = SlotDelta<Result<()>>;

// Eager rent collection repeats in cyclic manner.
Expand Down Expand Up @@ -4363,7 +4363,14 @@ impl Bank {
None
},
compute_budget.max_invoke_depth.saturating_add(1),
tx.message().instructions().len(),
if self
.feature_set
.is_active(&feature_set::limit_max_instruction_trace_length::id())
{
compute_budget.max_instruction_trace_length
} else {
std::usize::MAX
},
);
if self
.feature_set
Expand Down Expand Up @@ -18774,7 +18781,6 @@ pub(crate) mod tests {
sol_to_lamports(1.),
bank.last_blockhash(),
);
let number_of_instructions_at_transaction_level = tx.message().instructions.len();
let num_accounts = tx.message().account_keys.len();
let sanitized_tx = SanitizedTransaction::try_from_legacy_transaction(tx).unwrap();
let mut error_counters = TransactionErrorMetrics::default();
Expand All @@ -18797,7 +18803,7 @@ pub(crate) mod tests {
loaded_txs[0].0.as_ref().unwrap().accounts.clone(),
Some(Rent::default()),
compute_budget.max_invoke_depth.saturating_add(1),
number_of_instructions_at_transaction_level,
compute_budget.max_instruction_trace_length,
);

assert_eq!(
Expand Down Expand Up @@ -18945,8 +18951,10 @@ pub(crate) mod tests {

#[test]
fn test_inner_instructions_list_from_instruction_trace() {
let mut transaction_context = TransactionContext::new(vec![], None, 3, 3);
for (index_in_trace, stack_height) in [1, 2, 1, 1, 2, 3, 2].into_iter().enumerate() {
let instruction_trace = [1, 2, 1, 1, 2, 3, 2];
let mut transaction_context =
TransactionContext::new(vec![], None, 3, instruction_trace.len());
for (index_in_trace, stack_height) in instruction_trace.into_iter().enumerate() {
while stack_height <= transaction_context.get_instruction_context_stack_height() {
transaction_context.pop().unwrap();
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/nonce_keyed_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ mod test {
},
];
let mut transaction_context =
TransactionContext::new(accounts, Some(Rent::default()), 1, 2);
TransactionContext::new(accounts, Some(Rent::default()), 1, 1);
let mut $invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
};
}
Expand Down
4 changes: 4 additions & 0 deletions sdk/program/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ pub enum InstructionError {
/// Max accounts exceeded
#[error("Max accounts exceeded")]
MaxAccountsExceeded,

/// Max instruction trace length exceeded
#[error("Max instruction trace length exceeded")]
MaxInstructionTraceLengthExceeded,
// Note: For any new error added here an equivalent ProgramError and its
// conversions must also be added
}
Expand Down
14 changes: 14 additions & 0 deletions sdk/program/src/program_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ pub enum ProgramError {
MaxAccountsDataAllocationsExceeded,
#[error("Account data reallocation was invalid")]
InvalidRealloc,
#[error("Instruction trace length exceeded the maximum allowed per transaction")]
MaxInstructionTraceLengthExceeded,
}

pub trait PrintProgramError {
Expand Down Expand Up @@ -97,6 +99,9 @@ impl PrintProgramError for ProgramError {
msg!("Error: MaxAccountsDataAllocationsExceeded")
}
Self::InvalidRealloc => msg!("Error: InvalidRealloc"),
Self::MaxInstructionTraceLengthExceeded => {
msg!("Error: MaxInstructionTraceLengthExceeded")
}
}
}
}
Expand Down Expand Up @@ -129,6 +134,7 @@ pub const UNSUPPORTED_SYSVAR: u64 = to_builtin!(17);
pub const ILLEGAL_OWNER: u64 = to_builtin!(18);
pub const MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED: u64 = to_builtin!(19);
pub const INVALID_ACCOUNT_DATA_REALLOC: u64 = to_builtin!(20);
pub const MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED: u64 = to_builtin!(21);
// Warning: Any new program errors added here must also be:
// - Added to the below conversions
// - Added as an equivalent to InstructionError
Expand Down Expand Up @@ -159,6 +165,9 @@ impl From<ProgramError> for u64 {
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED
}
ProgramError::InvalidRealloc => INVALID_ACCOUNT_DATA_REALLOC,
ProgramError::MaxInstructionTraceLengthExceeded => {
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED
}
ProgramError::Custom(error) => {
if error == 0 {
CUSTOM_ZERO
Expand Down Expand Up @@ -193,6 +202,7 @@ impl From<u64> for ProgramError {
ILLEGAL_OWNER => Self::IllegalOwner,
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED => Self::MaxAccountsDataAllocationsExceeded,
INVALID_ACCOUNT_DATA_REALLOC => Self::InvalidRealloc,
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED => Self::MaxInstructionTraceLengthExceeded,
_ => Self::Custom(error as u32),
}
}
Expand Down Expand Up @@ -225,6 +235,9 @@ impl TryFrom<InstructionError> for ProgramError {
Ok(Self::MaxAccountsDataAllocationsExceeded)
}
Self::Error::InvalidRealloc => Ok(Self::InvalidRealloc),
Self::Error::MaxInstructionTraceLengthExceeded => {
Ok(Self::MaxInstructionTraceLengthExceeded)
}
_ => Err(error),
}
}
Expand Down Expand Up @@ -257,6 +270,7 @@ where
ILLEGAL_OWNER => Self::IllegalOwner,
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED => Self::MaxAccountsDataAllocationsExceeded,
INVALID_ACCOUNT_DATA_REALLOC => Self::InvalidRealloc,
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED => Self::MaxInstructionTraceLengthExceeded,
_ => {
// A valid custom error has no bits set in the upper 32
if error >> BUILTIN_BIT_SHIFT == 0 {
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,10 @@ pub mod increase_tx_account_lock_limit {
solana_sdk::declare_id!("9LZdXeKGeBV6hRLdxS1rHbHoEUsKqesCC2ZAPTPKJAbK");
}

pub mod limit_max_instruction_trace_length {
solana_sdk::declare_id!("GQALDaC48fEhZGWRj9iL5Q889emJKcj3aCvHF7VCbbF4");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -652,6 +656,7 @@ lazy_static! {
(epoch_accounts_hash::id(), "enable epoch accounts hash calculation #27539"),
(remove_deprecated_request_unit_ix::id(), "remove support for RequestUnitsDeprecated instruction #27500"),
(increase_tx_account_lock_limit::id(), "increase tx account lock limit to 128 #27241"),
(limit_max_instruction_trace_length::id(), "limit max instruction trace length"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down
26 changes: 18 additions & 8 deletions sdk/src/transaction_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ pub struct TransactionContext {
accounts: Pin<Box<[RefCell<AccountSharedData>]>>,
#[cfg(not(target_os = "solana"))]
account_touched_flags: RefCell<Pin<Box<[bool]>>>,
instruction_context_capacity: usize,
instruction_stack_capacity: usize,
instruction_trace_capacity: usize,
instruction_stack: Vec<usize>,
instruction_trace: Vec<InstructionContext>,
return_data: TransactionReturnData,
Expand All @@ -130,8 +131,8 @@ impl TransactionContext {
pub fn new(
transaction_accounts: Vec<TransactionAccount>,
rent: Option<Rent>,
instruction_context_capacity: usize,
_number_of_instructions_at_transaction_level: usize,
instruction_stack_capacity: usize,
instruction_trace_capacity: usize,
) -> Self {
let (account_keys, accounts): (Vec<Pubkey>, Vec<RefCell<AccountSharedData>>) =
transaction_accounts
Expand All @@ -143,8 +144,9 @@ impl TransactionContext {
account_keys: Pin::new(account_keys.into_boxed_slice()),
accounts: Pin::new(accounts.into_boxed_slice()),
account_touched_flags: RefCell::new(Pin::new(account_touched_flags.into_boxed_slice())),
instruction_context_capacity,
instruction_stack: Vec::with_capacity(instruction_context_capacity),
instruction_stack_capacity,
instruction_trace_capacity,
instruction_stack: Vec::with_capacity(instruction_stack_capacity),
instruction_trace: vec![InstructionContext::default()],
return_data: TransactionReturnData::default(),
accounts_resize_delta: RefCell::new(0),
Expand Down Expand Up @@ -213,6 +215,11 @@ impl TransactionContext {
.map(|index| index as IndexOfAccount)
}

/// Gets the max length of the InstructionContext trace
pub fn get_instruction_trace_capacity(&self) -> usize {
self.instruction_trace_capacity
}

/// Returns the instruction trace length.
///
/// Not counting the last empty InstructionContext which is always pre-reserved for the next instruction.
Expand Down Expand Up @@ -246,8 +253,8 @@ impl TransactionContext {
}

/// Gets the max height of the InstructionContext stack
pub fn get_instruction_context_capacity(&self) -> usize {
self.instruction_context_capacity
pub fn get_instruction_stack_capacity(&self) -> usize {
self.instruction_stack_capacity
}

/// Gets instruction stack height, top-level instructions are height
Expand Down Expand Up @@ -307,8 +314,11 @@ impl TransactionContext {
callee_instruction_accounts_lamport_sum;
}
let index_in_trace = self.get_instruction_trace_length();
if index_in_trace >= self.instruction_trace_capacity {
return Err(InstructionError::MaxInstructionTraceLengthExceeded);
}
self.instruction_trace.push(InstructionContext::default());
if nesting_level >= self.instruction_context_capacity {
if nesting_level >= self.instruction_stack_capacity {
return Err(InstructionError::CallDepth);
}
self.instruction_stack.push(index_in_trace);
Expand Down
1 change: 1 addition & 0 deletions storage-proto/proto/transaction_by_addr.proto
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ enum InstructionErrorType {
ILLEGAL_OWNER = 49;
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED = 50;
MAX_ACCOUNTS_EXCEEDED = 51;
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED = 52;
}

message UnixTimestamp {
Expand Down
Loading

0 comments on commit 71aee4f

Please sign in to comment.