diff --git a/no_std_test/src/main.rs b/no_std_test/src/main.rs index fbf87a53f..abd69d8bb 100644 --- a/no_std_test/src/main.rs +++ b/no_std_test/src/main.rs @@ -56,6 +56,7 @@ use secp256k1::ecdh::SharedSecret; use secp256k1::rand::{self, RngCore}; use secp256k1::serde::Serialize; use secp256k1::*; +use secp256k1::ffi::types::AlignedType; use serde_cbor::de; use serde_cbor::ser::SliceWrite; @@ -82,11 +83,11 @@ impl RngCore for FakeRng { #[start] fn start(_argc: isize, _argv: *const *const u8) -> isize { - let mut buf = [0u8; 600_000]; + let mut buf = [AlignedType::zeroed(); 37_000]; let size = Secp256k1::preallocate_size(); unsafe { libc::printf("needed size: %d\n\0".as_ptr() as _, size) }; - let mut secp = Secp256k1::preallocated_new(&mut buf).unwrap(); + let mut secp = unsafe {Secp256k1::preallocated_new(buf.as_mut() as *mut [AlignedType] as _)}; secp.randomize(&mut FakeRng); let secret_key = SecretKey::new(&mut FakeRng); let public_key = PublicKey::from_secret_key(&secp, &secret_key); @@ -161,5 +162,5 @@ fn panic(info: &PanicInfo) -> ! { let mut buf = Print::new(); write(&mut buf, *msg).unwrap(); buf.print(); - unsafe { intrinsics::abort() } + intrinsics::abort() } diff --git a/secp256k1-sys/Cargo.toml b/secp256k1-sys/Cargo.toml index 5f8d67ff4..b3b37638e 100644 --- a/secp256k1-sys/Cargo.toml +++ b/secp256k1-sys/Cargo.toml @@ -21,6 +21,9 @@ features = [ "recovery", "endomorphism", "lowmemory" ] [build-dependencies] cc = "1.0.28" +[dev-dependencies] +libc = "0.2" + [lib] name = "secp256k1_sys" path = "src/lib.rs" diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index c186ec1d8..a221f20ac 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -324,19 +324,21 @@ extern "C" { #[no_mangle] #[cfg(all(feature = "std", not(feature = "external-symbols")))] pub unsafe extern "C" fn rustsecp256k1_v0_2_0_context_create(flags: c_uint) -> *mut Context { - use std::mem; - assert!(mem::align_of::() >= mem::align_of::()); - assert_eq!(mem::size_of::(), mem::size_of::<&usize>()); - - let word_size = mem::size_of::(); - let n_words = (secp256k1_context_preallocated_size(flags) + word_size - 1) / word_size; - - let buf = vec![0usize; n_words + 1].into_boxed_slice(); - let ptr = Box::into_raw(buf) as *mut usize; - ::core::ptr::write(ptr, n_words); - let ptr: *mut usize = ptr.offset(1); - - secp256k1_context_preallocated_create(ptr as *mut c_void, flags) + use core::mem; + use std::alloc; + assert!(ALIGN_TO >= mem::align_of::()); + assert!(ALIGN_TO >= mem::align_of::<&usize>()); + assert!(ALIGN_TO >= mem::size_of::()); + + // We need to allocate `ALIGN_TO` more bytes in order to write the amount of bytes back. + let bytes = secp256k1_context_preallocated_size(flags) + ALIGN_TO; + let layout = alloc::Layout::from_size_align(bytes, ALIGN_TO).unwrap(); + let ptr = alloc::alloc(layout); + (ptr as *mut usize).write(bytes); + // We must offset a whole ALIGN_TO in order to preserve the same alignment + // this means we "lose" ALIGN_TO-size_of(usize) for padding. + let ptr = ptr.add(ALIGN_TO) as *mut c_void; + secp256k1_context_preallocated_create(ptr, flags) } #[cfg(all(feature = "std", not(feature = "external-symbols")))] @@ -353,13 +355,12 @@ pub unsafe fn secp256k1_context_create(flags: c_uint) -> *mut Context { #[no_mangle] #[cfg(all(feature = "std", not(feature = "external-symbols")))] pub unsafe extern "C" fn rustsecp256k1_v0_2_0_context_destroy(ctx: *mut Context) { + use std::alloc; secp256k1_context_preallocated_destroy(ctx); - let ctx: *mut usize = ctx as *mut usize; - - let n_words_ptr: *mut usize = ctx.offset(-1); - let n_words: usize = ::core::ptr::read(n_words_ptr); - let slice: &mut [usize] = slice::from_raw_parts_mut(n_words_ptr , n_words+1); - let _ = Box::from_raw(slice as *mut [usize]); + let ptr = (ctx as *mut u8).sub(ALIGN_TO); + let bytes = (ptr as *mut usize).read(); + let layout = alloc::Layout::from_size_align(bytes, ALIGN_TO).unwrap(); + alloc::dealloc(ptr, layout); } #[cfg(all(feature = "std", not(feature = "external-symbols")))] diff --git a/secp256k1-sys/src/types.rs b/secp256k1-sys/src/types.rs index 78216af0d..2eb00f565 100644 --- a/secp256k1-sys/src/types.rs +++ b/secp256k1-sys/src/types.rs @@ -1,5 +1,5 @@ #![allow(non_camel_case_types)] -use core::fmt; +use core::{fmt, mem}; pub type c_int = i32; pub type c_uchar = u8; @@ -26,11 +26,30 @@ impl fmt::Debug for c_void { } } +/// A type that is as aligned as the biggest alignment for fundamental types in C +/// since C11 that means as aligned as `max_align_t` is. +/// the exact size/alignment is unspecified. +// 16 matches is as big as the biggest alignment in any arch that rust supports https://github.com/rust-lang/rust/blob/2c31b45ae878b821975c4ebd94cc1e49f6073fd0/library/std/src/sys_common/alloc.rs +#[repr(align(16))] +#[derive(Default, Copy, Clone)] +pub struct AlignedType([u8; 16]); + +impl AlignedType { + pub fn zeroed() -> Self { + AlignedType([0u8; 16]) + } +} + +pub(crate) const ALIGN_TO: usize = mem::align_of::(); + + #[cfg(test)] mod tests { + extern crate libc; use std::os::raw; + use std::mem; use std::any::TypeId; - use types; + use {types, AlignedType}; #[test] fn verify_types() { @@ -38,6 +57,8 @@ mod tests { assert_eq!(TypeId::of::(), TypeId::of::()); assert_eq!(TypeId::of::(), TypeId::of::()); assert_eq!(TypeId::of::(), TypeId::of::()); + + assert!(mem::align_of::() >= mem::align_of::()); } } diff --git a/src/context.rs b/src/context.rs index 0e55d598c..ef7b1d6ed 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,9 +1,7 @@ use core::marker::PhantomData; -use core::mem::ManuallyDrop; -use ptr; -use ffi::{self, CPtr}; +use core::mem::{self, ManuallyDrop}; +use ffi::{self, types::AlignedType}; use ffi::types::{c_uint, c_void}; -use Error; use Secp256k1; #[cfg(feature = "std")] @@ -49,7 +47,7 @@ pub unsafe trait Context : private::Sealed { /// A constant description of the context. const DESCRIPTION: &'static str; /// A function to deallocate the memory when the context is dropped. - unsafe fn deallocate(ptr: *mut [u8]); + unsafe fn deallocate(ptr: *mut u8, size: usize); } /// Marker trait for indicating that an instance of `Secp256k1` can be used for signing. @@ -92,6 +90,8 @@ mod std_only { impl private::Sealed for VerifyOnly {} use super::*; + use std::alloc; + const ALIGN_TO: usize = mem::align_of::(); /// Represents the set of capabilities needed for signing. pub enum SignOnly {} @@ -112,8 +112,9 @@ mod std_only { const FLAGS: c_uint = ffi::SECP256K1_START_SIGN; const DESCRIPTION: &'static str = "signing only"; - unsafe fn deallocate(ptr: *mut [u8]) { - let _ = Box::from_raw(ptr); + unsafe fn deallocate(ptr: *mut u8, size: usize) { + let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap(); + alloc::dealloc(ptr, layout); } } @@ -121,8 +122,9 @@ mod std_only { const FLAGS: c_uint = ffi::SECP256K1_START_VERIFY; const DESCRIPTION: &'static str = "verification only"; - unsafe fn deallocate(ptr: *mut [u8]) { - let _ = Box::from_raw(ptr); + unsafe fn deallocate(ptr: *mut u8, size: usize) { + let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap(); + alloc::dealloc(ptr, layout); } } @@ -130,8 +132,9 @@ mod std_only { const FLAGS: c_uint = VerifyOnly::FLAGS | SignOnly::FLAGS; const DESCRIPTION: &'static str = "all capabilities"; - unsafe fn deallocate(ptr: *mut [u8]) { - let _ = Box::from_raw(ptr); + unsafe fn deallocate(ptr: *mut u8, size: usize) { + let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap(); + alloc::dealloc(ptr, layout); } } @@ -141,12 +144,13 @@ mod std_only { #[cfg(target_arch = "wasm32")] ffi::types::sanity_checks_for_wasm(); - let buf = vec![0u8; Self::preallocate_size_gen()].into_boxed_slice(); - let ptr = Box::into_raw(buf); + let size = unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) }; + let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap(); + let ptr = unsafe {alloc::alloc(layout)}; Secp256k1 { ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS) }, phantom: PhantomData, - buf: ptr, + size, } } } @@ -180,12 +184,13 @@ mod std_only { impl Clone for Secp256k1 { fn clone(&self) -> Secp256k1 { - let clone_size = unsafe {ffi::secp256k1_context_preallocated_clone_size(self.ctx)}; - let ptr_buf = Box::into_raw(vec![0u8; clone_size].into_boxed_slice()); + let size = unsafe {ffi::secp256k1_context_preallocated_clone_size(self.ctx as _)}; + let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap(); + let ptr = unsafe {alloc::alloc(layout)}; Secp256k1 { - ctx: unsafe { ffi::secp256k1_context_preallocated_clone(self.ctx, ptr_buf as *mut c_void) }, + ctx: unsafe { ffi::secp256k1_context_preallocated_clone(self.ctx, ptr as *mut c_void) }, phantom: PhantomData, - buf: ptr_buf, + size, } } } @@ -201,7 +206,7 @@ unsafe impl<'buf> Context for SignOnlyPreallocated<'buf> { const FLAGS: c_uint = ffi::SECP256K1_START_SIGN; const DESCRIPTION: &'static str = "signing only"; - unsafe fn deallocate(_ptr: *mut [u8]) { + unsafe fn deallocate(_ptr: *mut u8, _size: usize) { // Allocated by the user } } @@ -210,7 +215,7 @@ unsafe impl<'buf> Context for VerifyOnlyPreallocated<'buf> { const FLAGS: c_uint = ffi::SECP256K1_START_VERIFY; const DESCRIPTION: &'static str = "verification only"; - unsafe fn deallocate(_ptr: *mut [u8]) { + unsafe fn deallocate(_ptr: *mut u8, _size: usize) { // Allocated by the user } } @@ -219,35 +224,38 @@ unsafe impl<'buf> Context for AllPreallocated<'buf> { const FLAGS: c_uint = SignOnlyPreallocated::FLAGS | VerifyOnlyPreallocated::FLAGS; const DESCRIPTION: &'static str = "all capabilities"; - unsafe fn deallocate(_ptr: *mut [u8]) { + unsafe fn deallocate(_ptr: *mut u8, _size: usize) { // Allocated by the user } } impl<'buf, C: Context + 'buf> Secp256k1 { /// Lets you create a context with preallocated buffer in a generic manner(sign/verify/all) - pub fn preallocated_gen_new(buf: &'buf mut [u8]) -> Result, Error> { + /// + /// # Safety + /// * The pointer must be valid for [`preallocate_size_gen()`](#method.preallocated_gen_new) bytes + /// * The pointer must be as aligned as [`ffi::types::AlignedType`](../secp256k1-sys/src/types.html#AlignedType) + /// * The pointer must be writeable. + pub unsafe fn preallocated_gen_new(buf: *mut c_void) -> Secp256k1 { #[cfg(target_arch = "wasm32")] ffi::types::sanity_checks_for_wasm(); - if buf.len() < Self::preallocate_size_gen() { - return Err(Error::NotEnoughMemory); - } - Ok(Secp256k1 { - ctx: unsafe { - ffi::secp256k1_context_preallocated_create( - buf.as_mut_c_ptr() as *mut c_void, - C::FLAGS) - }, + Secp256k1 { + ctx: ffi::secp256k1_context_preallocated_create(buf, C::FLAGS), phantom: PhantomData, - buf: buf as *mut [u8], - }) + size: 0, // We don't care about the size because it's the caller responsibility to deallocate. + } } } impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context with all capabilities - pub fn preallocated_new(buf: &'buf mut [u8]) -> Result>, Error> { + /// + /// # Safety + /// * The pointer must be valid for [`preallocate_size()`](#method.preallocate_size) bytes + /// * The pointer must be as aligned as [`ffi::types::AlignedType`](../secp256k1-sys/src/types.html#AlignedType) + /// * The pointer must be writeable. + pub unsafe fn preallocated_new(buf: *mut c_void) -> Secp256k1> { Secp256k1::preallocated_gen_new(buf) } /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context @@ -270,14 +278,19 @@ impl<'buf> Secp256k1> { ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData, - buf: ptr::null_mut::<[u8;0]>() as *mut [u8] , + size: 0, // We don't care about the size because it's the caller responsibility to deallocate. }) } } impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context that can only be used for signing - pub fn preallocated_signing_only(buf: &'buf mut [u8]) -> Result>, Error> { + /// + /// # Safety + /// * The pointer must be valid for [`preallocate_size_gen()`](#method.preallocate_size_gen) bytes + /// * The pointer must be as aligned as [`ffi::types::AlignedType`](../secp256k1-sys/src/types.html#AlignedType) + /// * The pointer must be writeable. + pub unsafe fn preallocated_signing_only(buf: *mut c_void) -> Secp256k1> { Secp256k1::preallocated_gen_new(buf) } @@ -302,14 +315,19 @@ impl<'buf> Secp256k1> { ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData, - buf: ptr::null_mut::<[u8;0]>() as *mut [u8] , + size: 0, // We don't care about the size because it's the caller responsibility to deallocate. }) } } impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context that can only be used for verification - pub fn preallocated_verification_only(buf: &'buf mut [u8]) -> Result>, Error> { + /// + /// # Safety + /// * The pointer must be valid for [`preallocate_verification_size()`](#method.preallocate_verification_size) bytes + /// * The pointer must be as aligned as [`ffi::types::AlignedType`](../secp256k1-sys/src/types.html#AlignedType) + /// * The pointer must be writeable. + pub unsafe fn preallocated_verification_only(buf: *mut c_void) -> Secp256k1> { Secp256k1::preallocated_gen_new(buf) } @@ -334,7 +352,7 @@ impl<'buf> Secp256k1> { ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData, - buf: ptr::null_mut::<[u8;0]>() as *mut [u8] , + size: 0, // We don't care about the size because it's the caller responsibility to deallocate. }) } } \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index 4d887a0c1..97103e3b6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -548,8 +548,6 @@ pub enum Error { InvalidRecoveryId, /// Invalid tweak for add_*_assign or mul_*_assign InvalidTweak, - /// Didn't pass enough memory to context creation with preallocated memory - NotEnoughMemory, } impl Error { @@ -562,7 +560,6 @@ impl Error { Error::InvalidSecretKey => "secp: malformed or out-of-range secret key", Error::InvalidRecoveryId => "secp: bad recovery id", Error::InvalidTweak => "secp: bad tweak", - Error::NotEnoughMemory => "secp: not enough memory allocated", } } } @@ -584,7 +581,7 @@ impl std::error::Error for Error { pub struct Secp256k1 { ctx: *mut ffi::Context, phantom: PhantomData, - buf: *mut [u8], + size: usize, } // The underlying secp context does not contain any references to memory it does not own @@ -633,7 +630,7 @@ impl Drop for Secp256k1 { fn drop(&mut self) { unsafe { ffi::secp256k1_context_preallocated_destroy(self.ctx); - C::deallocate(self.buf); + C::deallocate(self.ctx as _, self.size); } } } @@ -788,6 +785,7 @@ fn from_hex(hex: &str, target: &mut [u8]) -> Result { mod tests { use rand::{RngCore, thread_rng}; use std::str::FromStr; + use std::alloc::{alloc, dealloc, Layout}; use std::marker::PhantomData; use key::{SecretKey, PublicKey}; @@ -795,7 +793,7 @@ mod tests { use super::constants; use super::{Secp256k1, Signature, Message}; use super::Error::{InvalidMessage, IncorrectSignature, InvalidSignature}; - use ffi; + use ffi::{self, types::AlignedType}; use context::*; macro_rules! hex { @@ -813,10 +811,10 @@ mod tests { let ctx_sign = unsafe { ffi::secp256k1_context_create(SignOnlyPreallocated::FLAGS) }; let ctx_vrfy = unsafe { ffi::secp256k1_context_create(VerifyOnlyPreallocated::FLAGS) }; - let buf: *mut [u8] = &mut [0u8;0] as _; - let full: Secp256k1 = Secp256k1{ctx: ctx_full, phantom: PhantomData, buf}; - let sign: Secp256k1 = Secp256k1{ctx: ctx_sign, phantom: PhantomData, buf}; - let vrfy: Secp256k1 = Secp256k1{ctx: ctx_vrfy, phantom: PhantomData, buf}; + let size = 0; + let full: Secp256k1 = Secp256k1{ctx: ctx_full, phantom: PhantomData, size}; + let sign: Secp256k1 = Secp256k1{ctx: ctx_sign, phantom: PhantomData, size}; + let vrfy: Secp256k1 = Secp256k1{ctx: ctx_vrfy, phantom: PhantomData, size}; let (sk, pk) = full.generate_keypair(&mut thread_rng()); let msg = Message::from_slice(&[2u8; 32]).unwrap(); @@ -872,26 +870,36 @@ mod tests { #[test] fn test_preallocation() { - let mut buf_ful = vec![0u8; Secp256k1::preallocate_size()]; - let mut buf_sign = vec![0u8; Secp256k1::preallocate_signing_size()]; - let mut buf_vfy = vec![0u8; Secp256k1::preallocate_verification_size()]; -// - let full = Secp256k1::preallocated_new(&mut buf_ful).unwrap(); - let sign = Secp256k1::preallocated_signing_only(&mut buf_sign).unwrap(); - let vrfy = Secp256k1::preallocated_verification_only(&mut buf_vfy).unwrap(); + const ALIGN_TO: usize = ::std::mem::align_of::(); + let ful_layout = Layout::from_size_align(Secp256k1::preallocate_size(), ALIGN_TO).unwrap(); + let sign_layout = Layout::from_size_align(Secp256k1::preallocate_signing_size(), ALIGN_TO).unwrap(); + let vrf_layout = Layout::from_size_align(Secp256k1::preallocate_verification_size(), ALIGN_TO).unwrap(); + let buf_ful = unsafe { alloc(ful_layout) }; + let buf_sign = unsafe { alloc(sign_layout) }; + let buf_vfy = unsafe { alloc(vrf_layout)} ; + { + let full = unsafe { Secp256k1::preallocated_new(buf_ful as _) }; + let sign = unsafe { Secp256k1::preallocated_signing_only(buf_sign as _) }; + let vrfy = unsafe { Secp256k1::preallocated_verification_only(buf_vfy as _) }; // drop(buf_vfy); // The buffer can't get dropped before the context. // println!("{:?}", buf_ful[5]); // Can't even read the data thanks to the borrow checker. - let (sk, pk) = full.generate_keypair(&mut thread_rng()); - let msg = Message::from_slice(&[2u8; 32]).unwrap(); - // Try signing - assert_eq!(sign.sign(&msg, &sk), full.sign(&msg, &sk)); - let sig = full.sign(&msg, &sk); + let (sk, pk) = full.generate_keypair(&mut thread_rng()); + let msg = Message::from_slice(&[2u8; 32]).unwrap(); + // Try signing + assert_eq!(sign.sign(&msg, &sk), full.sign(&msg, &sk)); + let sig = full.sign(&msg, &sk); - // Try verifying - assert!(vrfy.verify(&msg, &sig, &pk).is_ok()); - assert!(full.verify(&msg, &sig, &pk).is_ok()); + // Try verifying + assert!(vrfy.verify(&msg, &sig, &pk).is_ok()); + assert!(full.verify(&msg, &sig, &pk).is_ok()); + } + unsafe { + dealloc(buf_ful, ful_layout); + dealloc(buf_sign, sign_layout); + dealloc(buf_vfy, vrf_layout); + } } #[test]