Skip to content

Commit

Permalink
arithmetic internals: Clarify memory safety of calls to bn_mul_mont.
Browse files Browse the repository at this point in the history
Replace `debug_asesrt!`-based checking with proper error checking. The
error cases will never be reached because the callers already ensured
that the slices are the correct lengths, but this is more clearly
correct.

The previous step defining the `InOut` type didn't work out so well,
so replace `InOut` with `AliasingSlices` that does the same thing. The
cost is more monomorphization, but that will become moot soon, and it
already isn't too bad since there are only three cases to consider.
It does help reduce the number of length checks that end up getting
generated.
  • Loading branch information
briansmith committed Jan 20, 2025
1 parent 4955dd7 commit b794f56
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 78 deletions.
19 changes: 13 additions & 6 deletions src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,34 @@
// OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

use crate::limb::LIMB_BITS;
pub(crate) use self::{constant::limbs_from_hex, limb_slice_error::LimbSliceError};
use crate::{error::LenMismatchError, limb::LIMB_BITS};

#[macro_use]
mod ffi;

mod constant;

#[cfg(feature = "alloc")]
pub mod bigint;

mod inout;
pub(crate) mod inout;
pub mod montgomery;

mod n0;

// The minimum number of limbs allowed for any `&[Limb]` operation.
//
// This must be at least 4 for bn_mul_mont to work, at least on x86.
//
// TODO: Use `256 / LIMB_BITS` so that the limit is independent of limb size.
#[allow(dead_code)] // XXX: Presently only used by `bigint`.
pub const MIN_LIMBS: usize = 4;

// The maximum number of limbs allowed for any `&[Limb]` operation.
pub const MAX_LIMBS: usize = 8192 / LIMB_BITS;

pub use self::{constant::limbs_from_hex, inout::InOut};
cold_exhaustive_error! {
enum limb_slice_error::LimbSliceError {
len_mismatch => LenMismatch(LenMismatchError),
too_short => TooShort(usize),
too_long => TooLong(usize),
}
}
50 changes: 27 additions & 23 deletions src/arithmetic/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(crate) use self::{
modulusvalue::OwnedModulusValue,
private_exponent::PrivateExponent,
};
use super::{montgomery::*, InOut, MAX_LIMBS};
use super::{montgomery::*, LimbSliceError, MAX_LIMBS};
use crate::{
bits::BitLength,
c, error,
Expand Down Expand Up @@ -99,13 +99,8 @@ fn from_montgomery_amm<M>(limbs: BoxedLimbs<M>, m: &Modulus<M>) -> Elem<M, Unenc
let mut one = [0; MAX_LIMBS];
one[0] = 1;
let one = &one[..m.limbs().len()];
limbs_mul_mont(
InOut::InPlace(&mut limbs),
one,
m.limbs(),
m.n0(),
m.cpu_features(),
);
limbs_mul_mont((&mut limbs[..], one), m.limbs(), m.n0(), m.cpu_features())
.unwrap_or_else(unwrap_impossible_limb_slice_error);
Elem {
limbs,
encoding: PhantomData,
Expand Down Expand Up @@ -151,12 +146,12 @@ where
(AF, BF): ProductEncoding,
{
limbs_mul_mont(
InOut::InPlace(&mut b.limbs),
&a.limbs,
(&mut b.limbs[..], &a.limbs[..]),
m.limbs(),
m.n0(),
m.cpu_features(),
);
)
.unwrap_or_else(unwrap_impossible_limb_slice_error);
Elem {
limbs: b.limbs,
encoding: PhantomData,
Expand Down Expand Up @@ -217,7 +212,8 @@ fn elem_squared<M, E>(
where
(E, E): ProductEncoding,
{
limbs_square_mont(&mut a.limbs, m.limbs(), m.n0(), m.cpu_features());
limbs_square_mont(&mut a.limbs, m.limbs(), m.n0(), m.cpu_features())
.unwrap_or_else(unwrap_impossible_limb_slice_error);
Elem {
limbs: a.limbs,
encoding: PhantomData,
Expand Down Expand Up @@ -479,13 +475,8 @@ pub fn elem_exp_consttime<M>(
let src1 = entry(previous, src1, num_limbs);
let src2 = entry(previous, src2, num_limbs);
let dst = entry_mut(rest, 0, num_limbs);
limbs_mul_mont(
InOut::Disjoint(dst, src1),
src2,
m.limbs(),
m.n0(),
m.cpu_features(),
);
limbs_mul_mont((dst, src1, src2), m.limbs(), m.n0(), m.cpu_features())
.map_err(error::erase::<LimbSliceError>)?;
}

let tmp = m.zero();
Expand Down Expand Up @@ -649,15 +640,16 @@ pub fn elem_exp_consttime<M>(
mut i: LeakyWindow,
num_limbs: usize,
cpu_features: cpu::Features,
) {
) -> Result<(), LimbSliceError> {
loop {
scatter(table, acc, i, num_limbs);
i *= 2;
if i >= TABLE_ENTRIES as LeakyWindow {
break;
}
limbs_square_mont(acc, m_cached, n0, cpu_features);
limbs_square_mont(acc, m_cached, n0, cpu_features)?;
}
Ok(())
}

// All entries in `table` will be Montgomery encoded.
Expand All @@ -670,7 +662,8 @@ pub fn elem_exp_consttime<M>(
acc.copy_from_slice(base_cached);

// Fill in entries 1, 2, 4, 8, 16.
scatter_powers_of_2(table, acc, m_cached, n0, 1, num_limbs, cpu_features);
scatter_powers_of_2(table, acc, m_cached, n0, 1, num_limbs, cpu_features)
.map_err(error::erase::<LimbSliceError>)?;
// Fill in entries 3, 6, 12, 24; 5, 10, 20, 30; 7, 14, 28; 9, 18; 11, 22; 13, 26; 15, 30;
// 17; 19; 21; 23; 25; 27; 29; 31.
for i in (3..(TABLE_ENTRIES as LeakyWindow)).step_by(2) {
Expand All @@ -683,7 +676,8 @@ pub fn elem_exp_consttime<M>(
Window::from(i - 1), // Not secret
num_limbs,
);
scatter_powers_of_2(table, acc, m_cached, n0, i, num_limbs, cpu_features);
scatter_powers_of_2(table, acc, m_cached, n0, i, num_limbs, cpu_features)
.map_err(error::erase::<LimbSliceError>)?;
}

let acc = limb::fold_5_bit_windows(
Expand Down Expand Up @@ -729,6 +723,16 @@ pub fn elem_verify_equal_consttime<M, E>(
}
}

#[cold]
#[inline(never)]
fn unwrap_impossible_limb_slice_error(err: LimbSliceError) {
match err {
LimbSliceError::LenMismatch(_) => unreachable!(),
LimbSliceError::TooShort(_) => unreachable!(),
LimbSliceError::TooLong(_) => unreachable!(),
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
92 changes: 92 additions & 0 deletions src/arithmetic/ffi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2024-2025 Brian Smith.
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
// copyright notice and this permission notice appear in all copies.
//
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY
// SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
// OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

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

const _MAX_LIMBS_ADDRESSES_MEMORY_SAFETY_ISSUES: () = {
// BoringSSL's limit: 8 kiloBYTES.
const BN_MONTGOMERY_MAX_WORDS: usize = (8 * 1092) / size_of::<Limb>();
assert!(MAX_LIMBS <= BN_MONTGOMERY_MAX_WORDS);

// Some 64-bit assembly implementations were written to take `len` as a
// `c_int`, so they zero out the undefined top half of `len` to convert it
// to a `usize`. But, others don't.
assert!(MAX_LIMBS <= usize_from_u32(u32::MAX));
};

macro_rules! bn_mul_mont_ffi {
( $in_out:expr, $n:expr, $n0:expr, $cpu:expr,
unsafe { ($MIN_LEN:expr, $Cpu:ty) => $f:ident }) => {{
use crate::{c, limb::Limb};
prefixed_extern! {
// `r` and/or 'a' and/or 'b' may alias.
// XXX: BoringSSL declares these functions to return `int`.
fn $f(
r: *mut Limb,
a: *const Limb,
b: *const Limb,
n: *const Limb,
n0: &N0,
len: c::size_t,
);
}
unsafe {
crate::arithmetic::ffi::bn_mul_mont_ffi::<$Cpu, { $MIN_LEN }>(
$in_out, $n, $n0, $cpu, $f,
)
}
}};
}

#[inline]
pub(super) unsafe fn bn_mul_mont_ffi<Cpu, const MIN_LEN: usize>(
mut in_out: impl AliasingSlices<Limb>,
n: &[Limb],
n0: &N0,
cpu: Cpu,
f: unsafe extern "C" fn(
r: *mut Limb,
a: *const Limb,
b: *const Limb,
n: *const Limb,
n0: &N0,
len: c::size_t,
),
) -> Result<(), LimbSliceError> {
/// The x86 implementation of `bn_mul_mont`, at least, requires at least 4
/// limbs. For a long time we have required 4 limbs for all targets, though
/// this may be unnecessary.
const _MIN_LIMBS_AT_LEAST_4: () = assert!(MIN_LIMBS >= 4);
// We haven't tested shorter lengths.
assert!(MIN_LEN >= MIN_LIMBS);
if n.len() < MIN_LEN {
return Err(LimbSliceError::too_short(n.len()));
}

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

in_out
.with_pointers(n.len(), |r, a, b| {
let len = n.len();
let n = n.as_ptr();
let _: Cpu = cpu;
unsafe { f(r, a, b, n, n0, len) };
})
.map_err(LimbSliceError::len_mismatch)
}
63 changes: 59 additions & 4 deletions src/arithmetic/inout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,63 @@
// OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

pub enum InOut<'io, T: ?Sized> {
InPlace(&'io mut T),
#[cfg_attr(target_arch = "x86_64", allow(dead_code))]
Disjoint(&'io mut T, &'io T),
pub(crate) use crate::error::LenMismatchError;

pub(crate) trait AliasingSlices<T> {
fn with_pointers<R>(
&mut self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
) -> Result<R, LenMismatchError>;
}

impl<T> AliasingSlices<T> for &mut [T] {
fn with_pointers<R>(
&mut self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
) -> Result<R, LenMismatchError> {
let r = self;
if r.len() != expected_len {
return Err(LenMismatchError::new(r.len()));
}
Ok(f(r.as_mut_ptr(), r.as_ptr(), r.as_ptr()))
}
}

impl<T> AliasingSlices<T> for (&mut [T], &[T]) {
fn with_pointers<R>(
&mut self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
) -> Result<R, LenMismatchError> {
let (r, a) = self;
if r.len() != expected_len {
return Err(LenMismatchError::new(r.len()));
}
if a.len() != expected_len {
return Err(LenMismatchError::new(a.len()));
}
Ok(f(r.as_mut_ptr(), r.as_ptr(), a.as_ptr()))
}
}

impl<T> AliasingSlices<T> for (&mut [T], &[T], &[T]) {
fn with_pointers<R>(
&mut self,
expected_len: usize,
f: impl FnOnce(*mut T, *const T, *const T) -> R,
) -> Result<R, LenMismatchError> {
let (r, a, b) = self;
if r.len() != expected_len {
return Err(LenMismatchError::new(r.len()));
}
if a.len() != expected_len {
return Err(LenMismatchError::new(a.len()));
}
if b.len() != expected_len {
return Err(LenMismatchError::new(b.len()));
}
Ok(f(r.as_mut_ptr(), a.as_ptr(), b.as_ptr()))
}
}
Loading

0 comments on commit b794f56

Please sign in to comment.