From 9be988db41c03cc644417584fb9bbee75554da14 Mon Sep 17 00:00:00 2001 From: Jack May Date: Mon, 16 Aug 2021 16:16:52 -0700 Subject: [PATCH] Fix memoverlap check (#19232) --- programs/bpf_loader/src/syscalls.rs | 30 +++++++++++++++++++++++++---- sdk/src/feature_set.rs | 5 +++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 401ca8c119..3757c604f1 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -22,7 +22,8 @@ use solana_sdk::{ epoch_schedule::EpochSchedule, feature_set::{ blake3_syscall_enabled, disable_fees_sysvar, enforce_aligned_host_addrs, - libsecp256k1_0_5_upgrade_enabled, memory_ops_syscalls, secp256k1_recover_syscall_enabled, + libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, memory_ops_syscalls, + secp256k1_recover_syscall_enabled, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -296,6 +297,7 @@ pub fn bind_syscall_context_objects<'a>( cost: invoke_context.get_compute_budget().cpi_bytes_per_unit, compute_meter: invoke_context.get_compute_meter(), loader_id, + mem_overlap_fix: invoke_context.is_feature_active(&mem_overlap_fix::id()), }), ); bind_feature_gated_syscall_context_object!( @@ -1215,11 +1217,17 @@ impl<'a> SyscallObject for SyscallKeccak256<'a> { } } +fn check_overlapping(src_addr: u64, dst_addr: u64, n: u64) -> bool { + (src_addr <= dst_addr && src_addr + n > dst_addr) + || (dst_addr <= src_addr && dst_addr + n > src_addr) +} + /// memcpy pub struct SyscallMemcpy<'a> { cost: u64, compute_meter: Rc>, loader_id: &'a Pubkey, + mem_overlap_fix: bool, } impl<'a> SyscallObject for SyscallMemcpy<'a> { fn call( @@ -1232,8 +1240,11 @@ impl<'a> SyscallObject for SyscallMemcpy<'a> { memory_mapping: &MemoryMapping, result: &mut Result>, ) { - // cannot be overlapping - if dst_addr + n > src_addr && src_addr > dst_addr { + if if self.mem_overlap_fix { + check_overlapping(src_addr, dst_addr, n) + } else { + dst_addr + n > src_addr && src_addr > dst_addr + } { *result = Err(SyscallError::CopyOverlapping.into()); return; } @@ -1253,7 +1264,7 @@ impl<'a> SyscallObject for SyscallMemcpy<'a> { *result = Ok(0); } } -/// memcpy +/// memmove pub struct SyscallMemmove<'a> { cost: u64, compute_meter: Rc>, @@ -3490,4 +3501,15 @@ mod tests { assert_eq!(got_rent, src_rent); } } + + #[test] + fn test_overlapping() { + assert!(!check_overlapping(10, 7, 3)); + assert!(check_overlapping(10, 8, 3)); + assert!(check_overlapping(10, 9, 3)); + assert!(check_overlapping(10, 10, 3)); + assert!(check_overlapping(10, 11, 3)); + assert!(check_overlapping(10, 12, 3)); + assert!(!check_overlapping(10, 13, 3)); + } } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 88e1b8e70d..86b93b5717 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -191,6 +191,10 @@ pub mod gate_large_block { solana_sdk::declare_id!("2ry7ygxiYURULZCrypHhveanvP5tzZ4toRwVp89oCNSj"); } +pub mod mem_overlap_fix { + solana_sdk::declare_id!("vXDCFK7gphrEmyf5VnKgLmqbdJ4UxD2eZH1qbdouYKF"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -233,6 +237,7 @@ lazy_static! { (disable_fees_sysvar::id(), "disable fees sysvar"), (stake_merge_with_unmatched_credits_observed::id(), "allow merging active stakes with unmatched credits_observed #18985"), (gate_large_block::id(), "validator checks block cost against max limit in realtime, reject if exceeds."), + (mem_overlap_fix::id(), "Memory overlap fix"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()