Skip to content

Commit

Permalink
arithmetic: Improve AliasingSlices3 w.r.t. dangling pointers.
Browse files Browse the repository at this point in the history
Rename `AliasingSlices3` method to clarify how the dangling pointer
aspect. Add a new helper to it that helps users avoid dangling
pointers.
  • Loading branch information
briansmith committed Jan 26, 2025
1 parent 731eff0 commit e109dc3
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 12 deletions.
17 changes: 10 additions & 7 deletions src/arithmetic/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use super::{inout::AliasingSlices3, n0::N0, LimbSliceError, MAX_LIMBS, MIN_LIMBS};
use crate::{c, limb::Limb, polyfill::usize_from_u32};
use core::mem::size_of;
use core::{mem::size_of, num::NonZeroUsize};

const _MAX_LIMBS_ADDRESSES_MEMORY_SAFETY_ISSUES: () = {
// BoringSSL's limit: 8 kiloBYTES.
Expand All @@ -40,7 +40,7 @@ macro_rules! bn_mul_mont_ffi {
b: *const Limb,
n: *const Limb,
n0: &N0,
len: c::size_t,
len: c::NonZero_size_t,
);
}
unsafe {
Expand All @@ -63,7 +63,7 @@ pub(super) unsafe fn bn_mul_mont_ffi<Cpu, const LEN_MIN: usize, const LEN_MOD: u
b: *const Limb,
n: *const Limb,
n0: &N0,
len: c::size_t,
len: c::NonZero_size_t,
),
) -> Result<(), LimbSliceError> {
assert_eq!(n.len() % LEN_MOD, 0); // The caller should guard against this.
Expand All @@ -77,15 +77,18 @@ pub(super) unsafe fn bn_mul_mont_ffi<Cpu, const LEN_MIN: usize, const LEN_MOD: u
if n.len() < LEN_MIN {
return Err(LimbSliceError::too_short(n.len()));
}
let len = NonZeroUsize::new(n.len()).unwrap_or_else(|| {
// Unreachable because we checked against `LEN_MIN`, and we checked
// `LEN_MIN` is nonzero.
unreachable!()

Check warning on line 83 in src/arithmetic/ffi.rs

View check run for this annotation

Codecov / codecov/patch

src/arithmetic/ffi.rs#L81-L83

Added lines #L81 - L83 were not covered by tests
});

// Avoid stack overflow from the alloca inside.
if n.len() > MAX_LIMBS {
if len.get() > MAX_LIMBS {
return Err(LimbSliceError::too_long(n.len()));
}

in_out
.with_pointers(n.len(), |r, a, b| {
let len = n.len();
.with_non_dangling_non_null_pointers_rab(len, |r, a, b| {
let n = n.as_ptr();
let _: Cpu = cpu;
unsafe { f(r, a, b, n, n0, len) };
Expand Down
34 changes: 29 additions & 5 deletions src/arithmetic/inout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

pub(crate) use crate::error::LenMismatchError;
use core::num::NonZeroUsize;

pub(crate) trait AliasingSlices3<T> {
/// The pointers passed to `f` will all be non-null and properly aligned,
/// but may be dangling.
/// and will not be dangling.
///
/// The first pointer, `r` points to potentially-uninitialized writable
/// space for `expected_len` elements of type `T`. Accordingly, `f` must
Expand All @@ -28,15 +29,38 @@ pub(crate) trait AliasingSlices3<T> {
/// `r`, `a`, and/or `b` may alias each other, but only in the following
/// ways: `ptr::eq(r, a)`, `ptr::eq(r, b)`, and/or `ptr::eq(a, b)`; they
/// will not be "overlapping."
fn with_pointers<R>(
///
/// Implementations of this trait shouldn't override this default
/// implementation.
#[inline(always)]
fn with_non_dangling_non_null_pointers_rab<R>(
self,
expected_len: NonZeroUsize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
) -> Result<R, LenMismatchError>
where
Self: Sized,
{
self.with_potentially_dangling_non_null_pointers_rab(expected_len.into(), f)
}

/// If `expected_len == 0` then the pointers passed to `f` may be
/// dangling pointers, which should not be passed to C functions. In all
/// other respects, this works like
/// `Self::with_non_dangling_non_null_pointers_rab`.
///
/// Implementations of this trait should implement this method and not
/// `with_non_dangling_non_null_pointers_rab`. Users of this trait should
/// use `with_non_dangling_non_null_pointers_rab` and not this.
fn with_potentially_dangling_non_null_pointers_rab<R>(
self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
) -> Result<R, LenMismatchError>;
}

impl<T> AliasingSlices3<T> for &mut [T] {
fn with_pointers<R>(
fn with_potentially_dangling_non_null_pointers_rab<R>(
self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
Expand All @@ -50,7 +74,7 @@ impl<T> AliasingSlices3<T> for &mut [T] {
}

impl<T> AliasingSlices3<T> for (&mut [T], &[T]) {
fn with_pointers<R>(
fn with_potentially_dangling_non_null_pointers_rab<R>(
self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
Expand All @@ -67,7 +91,7 @@ impl<T> AliasingSlices3<T> for (&mut [T], &[T]) {
}

impl<T> AliasingSlices3<T> for (&mut [T], &[T], &[T]) {
fn with_pointers<R>(
fn with_potentially_dangling_non_null_pointers_rab<R>(
self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
Expand Down

0 comments on commit e109dc3

Please sign in to comment.