Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Alternative] Making sure everything is aligned correctly #234

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions no_std_test/src/main.rs
Expand Up @@ -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;
Expand All @@ -82,7 +83,7 @@ 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) };

Expand Down Expand Up @@ -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()
}
3 changes: 3 additions & 0 deletions secp256k1-sys/Cargo.toml
Expand Up @@ -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"
Expand Down
39 changes: 20 additions & 19 deletions secp256k1-sys/src/lib.rs
Expand Up @@ -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::<usize>() >= mem::align_of::<u8>());
assert_eq!(mem::size_of::<usize>(), mem::size_of::<&usize>());

let word_size = mem::size_of::<usize>();
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::<usize>());
assert!(ALIGN_TO >= mem::align_of::<&usize>());
assert!(ALIGN_TO >= mem::size_of::<usize>());

// 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")))]
Expand All @@ -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")))]
Expand Down
25 changes: 23 additions & 2 deletions 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;
Expand All @@ -26,18 +26,39 @@ 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::<AlignedType>();


#[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() {
assert_eq!(TypeId::of::<types::c_int>(), TypeId::of::<raw::c_int>());
assert_eq!(TypeId::of::<types::c_uchar>(), TypeId::of::<raw::c_uchar>());
assert_eq!(TypeId::of::<types::c_uint>(), TypeId::of::<raw::c_uint>());
assert_eq!(TypeId::of::<types::c_char>(), TypeId::of::<raw::c_char>());

assert!(mem::align_of::<AlignedType>() >= mem::align_of::<self::libc::max_align_t>());
}
}

Expand Down
58 changes: 30 additions & 28 deletions src/context.rs
@@ -1,7 +1,7 @@
use core::marker::PhantomData;
use core::mem::ManuallyDrop;
use ptr;
use ffi::{self, CPtr};
use core::mem::{self, ManuallyDrop};
use core::ptr::NonNull;
use ffi::{self, CPtr, types::AlignedType};
use ffi::types::{c_uint, c_void};
use Error;
use Secp256k1;
Expand Down Expand Up @@ -49,7 +49,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: NonNull<[AlignedType]>);
}

/// Marker trait for indicating that an instance of `Secp256k1` can be used for signing.
Expand Down Expand Up @@ -112,26 +112,26 @@ 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: NonNull<[AlignedType]>) {
let _ = Box::from_raw(ptr.as_ptr());
}
}

unsafe impl Context for VerifyOnly {
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: NonNull<[AlignedType]>) {
let _ = Box::from_raw(ptr.as_ptr());
}
}

unsafe impl Context for All {
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: NonNull<[AlignedType]>) {
let _ = Box::from_raw(ptr.as_ptr());
}
}

Expand All @@ -141,10 +141,10 @@ 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 buf = vec![AlignedType::zeroed(); Self::preallocate_size_gen()].into_boxed_slice();
let ptr = NonNull::from(Box::leak(buf));
Secp256k1 {
ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS) },
ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr.as_ptr() as *mut c_void, C::FLAGS) },
phantom: PhantomData,
buf: ptr,
}
Expand Down Expand Up @@ -180,12 +180,14 @@ mod std_only {

impl<C: Context> Clone for Secp256k1<C> {
fn clone(&self) -> Secp256k1<C> {
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 word_size = mem::size_of::<AlignedType>();
let bytes = unsafe {ffi::secp256k1_context_preallocated_clone_size(self.ctx)};
let words = (bytes + word_size - 1) / word_size;
let ptr = NonNull::from(Box::leak(vec![AlignedType::zeroed(); words].into_boxed_slice()));
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_ptr() as *mut c_void) },
phantom: PhantomData,
buf: ptr_buf,
buf: ptr,
}
}
}
Expand All @@ -201,7 +203,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: NonNull<[AlignedType]>) {
// Allocated by the user
}
}
Expand All @@ -210,7 +212,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: NonNull<[AlignedType]>) {
// Allocated by the user
}
}
Expand All @@ -219,14 +221,14 @@ 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: NonNull<[AlignedType]>) {
// Allocated by the user
}
}

impl<'buf, C: Context + 'buf> Secp256k1<C> {
/// 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<Secp256k1<C>, Error> {
pub fn preallocated_gen_new(buf: &'buf mut [AlignedType]) -> Result<Secp256k1<C>, Error> {
#[cfg(target_arch = "wasm32")]
ffi::types::sanity_checks_for_wasm();

Expand All @@ -240,14 +242,14 @@ impl<'buf, C: Context + 'buf> Secp256k1<C> {
C::FLAGS)
},
phantom: PhantomData,
buf: buf as *mut [u8],
buf: buf.into(),
})
}
}

impl<'buf> Secp256k1<AllPreallocated<'buf>> {
/// Creates a new Secp256k1 context with all capabilities
pub fn preallocated_new(buf: &'buf mut [u8]) -> Result<Secp256k1<AllPreallocated<'buf>>, Error> {
pub fn preallocated_new(buf: &'buf mut [AlignedType]) -> Result<Secp256k1<AllPreallocated<'buf>>, Error> {
Secp256k1::preallocated_gen_new(buf)
}
/// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context
Expand All @@ -270,14 +272,14 @@ impl<'buf> Secp256k1<AllPreallocated<'buf>> {
ManuallyDrop::new(Secp256k1 {
ctx: raw_ctx,
phantom: PhantomData,
buf: ptr::null_mut::<[u8;0]>() as *mut [u8] ,
buf: NonNull::<[AlignedType; 0]>::dangling() as _,
})
}
}

impl<'buf> Secp256k1<SignOnlyPreallocated<'buf>> {
/// Creates a new Secp256k1 context that can only be used for signing
pub fn preallocated_signing_only(buf: &'buf mut [u8]) -> Result<Secp256k1<SignOnlyPreallocated<'buf>>, Error> {
pub fn preallocated_signing_only(buf: &'buf mut [AlignedType]) -> Result<Secp256k1<SignOnlyPreallocated<'buf>>, Error> {
Secp256k1::preallocated_gen_new(buf)
}

Expand All @@ -302,14 +304,14 @@ impl<'buf> Secp256k1<SignOnlyPreallocated<'buf>> {
ManuallyDrop::new(Secp256k1 {
ctx: raw_ctx,
phantom: PhantomData,
buf: ptr::null_mut::<[u8;0]>() as *mut [u8] ,
buf: NonNull::<[AlignedType; 0]>::dangling() as _,
})
}
}

impl<'buf> Secp256k1<VerifyOnlyPreallocated<'buf>> {
/// Creates a new Secp256k1 context that can only be used for verification
pub fn preallocated_verification_only(buf: &'buf mut [u8]) -> Result<Secp256k1<VerifyOnlyPreallocated<'buf>>, Error> {
pub fn preallocated_verification_only(buf: &'buf mut [AlignedType]) -> Result<Secp256k1<VerifyOnlyPreallocated<'buf>>, Error> {
Secp256k1::preallocated_gen_new(buf)
}

Expand All @@ -334,7 +336,7 @@ impl<'buf> Secp256k1<VerifyOnlyPreallocated<'buf>> {
ManuallyDrop::new(Secp256k1 {
ctx: raw_ctx,
phantom: PhantomData,
buf: ptr::null_mut::<[u8;0]>() as *mut [u8] ,
buf: NonNull::<[AlignedType; 0]>::dangling() as _,
})
}
}
23 changes: 14 additions & 9 deletions src/lib.rs
Expand Up @@ -161,7 +161,7 @@ pub use secp256k1_sys as ffi;
#[cfg(any(test, feature = "rand"))] use rand::Rng;
#[cfg(any(test, feature = "std"))] extern crate core;

use core::{fmt, ptr, str};
use core::{fmt, str, ptr::{self, NonNull}};

#[macro_use]
mod macros;
Expand All @@ -177,7 +177,8 @@ pub use key::PublicKey;
pub use context::*;
use core::marker::PhantomData;
use core::ops::Deref;
use ffi::CPtr;
use core::mem;
use ffi::{CPtr, types::AlignedType};

#[cfg(feature = "global-context")]
pub use context::global::SECP256K1;
Expand Down Expand Up @@ -584,7 +585,7 @@ impl std::error::Error for Error {
pub struct Secp256k1<C: Context> {
ctx: *mut ffi::Context,
phantom: PhantomData<C>,
buf: *mut [u8],
buf: NonNull<[AlignedType]>,
}

// The underlying secp context does not contain any references to memory it does not own
Expand Down Expand Up @@ -656,7 +657,10 @@ impl<C: Context> Secp256k1<C> {

/// Returns the required memory for a preallocated context buffer in a generic manner(sign/verify/all)
pub fn preallocate_size_gen() -> usize {
unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) }
let word_size = mem::size_of::<AlignedType>();
let bytes = unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) };

(bytes + word_size - 1) / word_size
}

/// (Re)randomizes the Secp256k1 context for cheap sidechannel resistance;
Expand Down Expand Up @@ -789,13 +793,14 @@ mod tests {
use rand::{RngCore, thread_rng};
use std::str::FromStr;
use std::marker::PhantomData;
use core::ptr::NonNull;

use key::{SecretKey, PublicKey};
use super::from_hex;
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 {
Expand All @@ -813,7 +818,7 @@ 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 buf = NonNull::<[AlignedType; 0]>::dangling() as _;
let full: Secp256k1<AllPreallocated> = Secp256k1{ctx: ctx_full, phantom: PhantomData, buf};
let sign: Secp256k1<SignOnlyPreallocated> = Secp256k1{ctx: ctx_sign, phantom: PhantomData, buf};
let vrfy: Secp256k1<VerifyOnlyPreallocated> = Secp256k1{ctx: ctx_vrfy, phantom: PhantomData, buf};
Expand Down Expand Up @@ -872,9 +877,9 @@ 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 mut buf_ful = vec![AlignedType::zeroed(); Secp256k1::preallocate_size()];
let mut buf_sign = vec![AlignedType::zeroed(); Secp256k1::preallocate_signing_size()];
let mut buf_vfy = vec![AlignedType::zeroed(); Secp256k1::preallocate_verification_size()];
//
let full = Secp256k1::preallocated_new(&mut buf_ful).unwrap();
let sign = Secp256k1::preallocated_signing_only(&mut buf_sign).unwrap();
Expand Down