diff --git a/perf/src/cuda_runtime.rs b/perf/src/cuda_runtime.rs index e850100ffb..4d61c09db9 100644 --- a/perf/src/cuda_runtime.rs +++ b/perf/src/cuda_runtime.rs @@ -26,7 +26,7 @@ pub fn pin(_mem: &mut Vec) { let err = (api.cuda_host_register)( _mem.as_mut_ptr() as *mut c_void, - _mem.capacity() * size_of::(), + _mem.capacity().saturating_mul(size_of::()), 0, ); if err != CUDA_SUCCESS { @@ -34,7 +34,7 @@ pub fn pin(_mem: &mut Vec) { "cudaHostRegister error: {} ptr: {:?} bytes: {}", err, _mem.as_ptr(), - _mem.capacity() * size_of::() + _mem.capacity().saturating_mul(size_of::()), ); } } @@ -279,7 +279,7 @@ impl PinnedVec { } pub fn push(&mut self, x: T) { - let (old_ptr, old_capacity) = self.prepare_realloc(self.x.len() + 1); + let (old_ptr, old_capacity) = self.prepare_realloc(self.x.len().saturating_add(1)); self.x.push(x); self.check_ptr(old_ptr, old_capacity, "push"); } @@ -295,13 +295,15 @@ impl PinnedVec { } pub fn append(&mut self, other: &mut Vec) { - let (old_ptr, old_capacity) = self.prepare_realloc(self.x.len() + other.len()); + let (old_ptr, old_capacity) = + self.prepare_realloc(self.x.len().saturating_add(other.len())); self.x.append(other); self.check_ptr(old_ptr, old_capacity, "resize"); } pub fn append_pinned(&mut self, other: &mut Self) { - let (old_ptr, old_capacity) = self.prepare_realloc(self.x.len() + other.len()); + let (old_ptr, old_capacity) = + self.prepare_realloc(self.x.len().saturating_add(other.len())); self.x.append(&mut other.x); self.check_ptr(old_ptr, old_capacity, "resize"); } diff --git a/perf/src/lib.rs b/perf/src/lib.rs index 9559e052e4..82feefaa70 100644 --- a/perf/src/lib.rs +++ b/perf/src/lib.rs @@ -1,4 +1,3 @@ -#![allow(clippy::integer_arithmetic)] pub mod cuda_runtime; pub mod packet; pub mod perf_libs; diff --git a/perf/src/recycler.rs b/perf/src/recycler.rs index 728fb29162..e1dfaa937e 100644 --- a/perf/src/recycler.rs +++ b/perf/src/recycler.rs @@ -1,6 +1,7 @@ use rand::{thread_rng, Rng}; use solana_measure::measure::Measure; use std::{ + convert::TryFrom, sync::{ atomic::{AtomicBool, AtomicUsize, Ordering}, Arc, Mutex, Weak, @@ -112,7 +113,14 @@ impl ObjectPool { } fn get_shrink_target(shrink_pct: u32, current_size: u32) -> u32 { - ((shrink_pct * current_size) + 99) / 100 + let shrink_pct = u64::from(shrink_pct); + let current_size = u64::from(current_size); + let shrink_target = shrink_pct + .saturating_mul(current_size) + .saturating_add(99) + .checked_div(100) + .unwrap_or(0); + u32::try_from(shrink_target).unwrap_or(u32::MAX) } fn shrink_if_necessary( @@ -138,7 +146,7 @@ impl ObjectPool { if self.len() > self.minimum_object_count as usize && self.len() > shrink_threshold_count as usize { - self.above_shrink_pct_count += 1; + self.above_shrink_pct_count = self.above_shrink_pct_count.saturating_add(1); } else { self.above_shrink_pct_count = 0; } @@ -147,7 +155,7 @@ impl ObjectPool { let mut recycler_shrink_elapsed = Measure::start("recycler_shrink"); // Do the shrink let target_size = std::cmp::max(self.minimum_object_count, shrink_threshold_count); - let ideal_num_to_remove = self.total_allocated_count - target_size; + let ideal_num_to_remove = self.total_allocated_count.saturating_sub(target_size); let mut shrink_removed_objects = Vec::with_capacity(ideal_num_to_remove as usize); for _ in 0..ideal_num_to_remove { if let Some(mut expired_object) = self.objects.pop() { @@ -160,7 +168,7 @@ impl ObjectPool { // before the object is allocated (see `should_allocate_new` logic below). // This race allows a difference of up to the number of threads allocating // with this recycler. - self.total_allocated_count -= 1; + self.total_allocated_count = self.total_allocated_count.saturating_sub(1); } else { break; } @@ -191,13 +199,13 @@ impl ObjectPool { AllocationDecision::Reuse(reused_object) } else if let Some(limit) = self.limit { if self.total_allocated_count < limit { - self.total_allocated_count += 1; + self.total_allocated_count = self.total_allocated_count.saturating_add(1); AllocationDecision::Allocate(self.total_allocated_count, self.len()) } else { AllocationDecision::AllocationLimitReached } } else { - self.total_allocated_count += 1; + self.total_allocated_count = self.total_allocated_count.saturating_add(1); AllocationDecision::Allocate(self.total_allocated_count, self.len()) } } diff --git a/perf/src/sigverify.rs b/perf/src/sigverify.rs index 1a2e1682f0..7473a0e1d6 100644 --- a/perf/src/sigverify.rs +++ b/perf/src/sigverify.rs @@ -8,16 +8,16 @@ use crate::cuda_runtime::PinnedVec; use crate::packet::{Packet, Packets}; use crate::perf_libs; use crate::recycler::Recycler; -use bincode::serialized_size; use rayon::ThreadPool; use solana_metrics::inc_new_counter_debug; use solana_rayon_threadlimit::get_thread_count; -use solana_sdk::message::MessageHeader; +use solana_sdk::message::MESSAGE_HEADER_LENGTH; use solana_sdk::pubkey::Pubkey; use solana_sdk::short_vec::decode_len; use solana_sdk::signature::Signature; #[cfg(test)] use solana_sdk::transaction::Transaction; +use std::convert::TryFrom; use std::mem::size_of; // Representing key tKeYE4wtowRb8yRroZShTipE18YVnqwXjsSAoNsFU6g @@ -74,6 +74,12 @@ impl std::convert::From> for PacketError { } } +impl std::convert::From for PacketError { + fn from(_e: std::num::TryFromIntError) -> Self { + Self::InvalidLen + } +} + pub fn init() { if let Some(api) = perf_libs::api() { unsafe { @@ -109,8 +115,8 @@ fn verify_packet(packet: &mut Packet) { let msg_end = packet.meta.size; for _ in 0..packet_offsets.sig_len { - let pubkey_end = pubkey_start as usize + size_of::(); - let sig_end = sig_start as usize + size_of::(); + let pubkey_end = pubkey_start.saturating_add(size_of::()); + let sig_end = sig_start.saturating_add(size_of::()); if pubkey_end >= packet.meta.size || sig_end >= packet.meta.size { packet.meta.discard = true; @@ -134,8 +140,8 @@ fn verify_packet(packet: &mut Packet) { packet.meta.is_tracer_tx = true; } - pubkey_start += size_of::(); - sig_start += size_of::(); + pubkey_start = pubkey_end; + sig_start = sig_end; } } @@ -146,13 +152,14 @@ pub fn batch_size(batches: &[Packets]) -> usize { // internal function to be unit-tested; should be used only by get_packet_offsets fn do_get_packet_offsets( packet: &Packet, - current_offset: u32, + current_offset: usize, ) -> Result { - let message_header_size = serialized_size(&MessageHeader::default()).unwrap() as usize; // should have at least 1 signature, sig lengths and the message header - if (1 + size_of::() + message_header_size) > packet.meta.size { - return Err(PacketError::InvalidLen); - } + let _ = 1usize + .checked_add(size_of::()) + .and_then(|v| v.checked_add(MESSAGE_HEADER_LENGTH)) + .filter(|v| *v <= packet.meta.size) + .ok_or(PacketError::InvalidLen)?; // read the length of Transaction.signatures (serialized with short_vec) let (sig_len_untrusted, sig_size) = @@ -160,53 +167,74 @@ fn do_get_packet_offsets( // Using msg_start_offset which is based on sig_len_untrusted introduces uncertainty. // Ultimately, the actual sigverify will determine the uncertainty. - let msg_start_offset = sig_size + sig_len_untrusted * size_of::(); + let msg_start_offset = sig_len_untrusted + .checked_mul(size_of::()) + .and_then(|v| v.checked_add(sig_size)) + .ok_or(PacketError::InvalidLen)?; + + let msg_start_offset_plus_one = msg_start_offset + .checked_add(1) + .ok_or(PacketError::InvalidLen)?; // Packet should have data at least for signatures, MessageHeader, 1 byte for Message.account_keys.len - if (msg_start_offset + message_header_size + 1) > packet.meta.size { - return Err(PacketError::InvalidSignatureLen); - } + let _ = msg_start_offset_plus_one + .checked_add(MESSAGE_HEADER_LENGTH) + .filter(|v| *v <= packet.meta.size) + .ok_or(PacketError::InvalidSignatureLen)?; // read MessageHeader.num_required_signatures (serialized with u8) - let sig_len_maybe_trusted = packet.data[msg_start_offset] as usize; + let sig_len_maybe_trusted = packet.data[msg_start_offset]; - let message_account_keys_len_offset = msg_start_offset + message_header_size; + let message_account_keys_len_offset = msg_start_offset + .checked_add(MESSAGE_HEADER_LENGTH) + .ok_or(PacketError::InvalidLen)?; // This reads and compares the MessageHeader num_required_signatures and // num_readonly_signed_accounts bytes. If num_required_signatures is not larger than // num_readonly_signed_accounts, the first account is not debitable, and cannot be charged // required transaction fees. - if packet.data[msg_start_offset] <= packet.data[msg_start_offset + 1] { + let readonly_signer_offset = msg_start_offset_plus_one; + if sig_len_maybe_trusted <= packet.data[readonly_signer_offset] { return Err(PacketError::PayerNotWritable); } + if usize::from(sig_len_maybe_trusted) != sig_len_untrusted { + return Err(PacketError::MismatchSignatureLen); + } + // read the length of Message.account_keys (serialized with short_vec) let (pubkey_len, pubkey_len_size) = decode_len(&packet.data[message_account_keys_len_offset..]) .map_err(|_| PacketError::InvalidShortVec)?; - if (message_account_keys_len_offset + pubkey_len * size_of::() + pubkey_len_size) - > packet.meta.size - { - return Err(PacketError::InvalidPubkeyLen); - } + let pubkey_start = message_account_keys_len_offset + .checked_add(pubkey_len_size) + .ok_or(PacketError::InvalidPubkeyLen)?; - let sig_start = current_offset as usize + sig_size; - let msg_start = current_offset as usize + msg_start_offset; - let pubkey_start = msg_start + message_header_size + pubkey_len_size; + let _ = pubkey_len + .checked_mul(size_of::()) + .and_then(|v| v.checked_add(pubkey_start)) + .filter(|v| *v <= packet.meta.size) + .ok_or(PacketError::InvalidPubkeyLen)?; - if sig_len_maybe_trusted != sig_len_untrusted { - return Err(PacketError::MismatchSignatureLen); - } + let sig_start = current_offset + .checked_add(sig_size) + .ok_or(PacketError::InvalidLen)?; + let msg_start = current_offset + .checked_add(msg_start_offset) + .ok_or(PacketError::InvalidLen)?; + let pubkey_start = current_offset + .checked_add(pubkey_start) + .ok_or(PacketError::InvalidLen)?; Ok(PacketOffsets::new( - sig_len_untrusted as u32, - sig_start as u32, - msg_start as u32, - pubkey_start as u32, + u32::try_from(sig_len_untrusted)?, + u32::try_from(sig_start)?, + u32::try_from(msg_start)?, + u32::try_from(pubkey_start)?, )) } -fn get_packet_offsets(packet: &Packet, current_offset: u32) -> PacketOffsets { +fn get_packet_offsets(packet: &Packet, current_offset: usize) -> PacketOffsets { let unsanitized_packet_offsets = do_get_packet_offsets(packet, current_offset); if let Ok(offsets) = unsanitized_packet_offsets { offsets @@ -226,13 +254,11 @@ pub fn generate_offsets(batches: &[Packets], recycler: &Recycler) -> T msg_start_offsets.set_pinnable(); let mut msg_sizes: PinnedVec<_> = recycler.allocate().unwrap(); msg_sizes.set_pinnable(); - let mut current_packet = 0; + let mut current_offset: usize = 0; let mut v_sig_lens = Vec::new(); batches.iter().for_each(|p| { let mut sig_lens = Vec::new(); p.packets.iter().for_each(|packet| { - let current_offset = current_packet as u32 * size_of::() as u32; - let packet_offsets = get_packet_offsets(packet, current_offset); sig_lens.push(packet_offsets.sig_len); @@ -241,19 +267,20 @@ pub fn generate_offsets(batches: &[Packets], recycler: &Recycler) -> T let mut pubkey_offset = packet_offsets.pubkey_start; let mut sig_offset = packet_offsets.sig_start; + let msg_size = current_offset.saturating_add(packet.meta.size) as u32; for _ in 0..packet_offsets.sig_len { signature_offsets.push(sig_offset); - sig_offset += size_of::() as u32; + sig_offset = sig_offset.saturating_add(size_of::() as u32); pubkey_offsets.push(pubkey_offset); - pubkey_offset += size_of::() as u32; + pubkey_offset = pubkey_offset.saturating_add(size_of::() as u32); msg_start_offsets.push(packet_offsets.msg_start); - msg_sizes - .push(current_offset + (packet.meta.size as u32) - packet_offsets.msg_start); + let msg_size = msg_size.saturating_sub(packet_offsets.msg_start); + msg_sizes.push(msg_size); } - current_packet += 1; + current_offset = current_offset.saturating_add(size_of::()); }); v_sig_lens.push(sig_lens); }); @@ -302,7 +329,7 @@ pub fn copy_return_values(sig_lens: &[Vec], out: &PinnedVec, rvs: &mut if 0 == out[num] { vout = 0; } - num += 1; + num = num.saturating_add(1); } *v = vout; } @@ -382,7 +409,7 @@ pub fn ed25519_verify( let mut elems = Vec::new(); let mut rvs = Vec::new(); - let mut num_packets = 0; + let mut num_packets: usize = 0; for p in batches.iter() { elems.push(perf_libs::Elems { elems: p.packets.as_ptr(), @@ -391,7 +418,7 @@ pub fn ed25519_verify( let mut v = Vec::new(); v.resize(p.packets.len(), 0); rvs.push(v); - num_packets += p.packets.len(); + num_packets = num_packets.saturating_add(p.packets.len()); } out.resize(signature_offsets.len(), 0); trace!("Starting verify num packets: {}", num_packets); @@ -435,6 +462,7 @@ pub fn make_packet_from_transaction(tx: Transaction) -> Packet { } #[cfg(test)] +#[allow(clippy::integer_arithmetic)] mod tests { use super::*; use crate::packet::{Packet, Packets}; @@ -653,7 +681,7 @@ mod tests { // Just like get_packet_offsets, but not returning redundant information. fn get_packet_offsets_from_tx(tx: Transaction, current_offset: u32) -> PacketOffsets { let packet = sigverify::make_packet_from_transaction(tx); - let packet_offsets = sigverify::get_packet_offsets(&packet, current_offset); + let packet_offsets = sigverify::get_packet_offsets(&packet, current_offset as usize); PacketOffsets::new( packet_offsets.sig_len, packet_offsets.sig_start - current_offset, diff --git a/sdk/program/src/message.rs b/sdk/program/src/message.rs index 578f73e478..d23a06da4e 100644 --- a/sdk/program/src/message.rs +++ b/sdk/program/src/message.rs @@ -141,6 +141,8 @@ fn get_program_ids(instructions: &[Instruction]) -> Vec { .collect() } +pub const MESSAGE_HEADER_LENGTH: usize = 3; + #[frozen_abi(digest = "BVC5RhetsNpheGipt5rUrkR6RDDUHtD5sCLK1UjymL4S")] #[derive(Serialize, Deserialize, Default, Debug, PartialEq, Eq, Clone, AbiExample)] #[serde(rename_all = "camelCase")] @@ -946,4 +948,12 @@ mod tests { Hash::from_str("CXRH7GHLieaQZRUjH1mpnNnUZQtU4V4RpJpAFgy77i3z").unwrap() ) } + + #[test] + fn test_message_header_len_constant() { + assert_eq!( + bincode::serialized_size(&MessageHeader::default()).unwrap() as usize, + MESSAGE_HEADER_LENGTH + ); + } }