arithmetic: Clarify memory safety of elem_exp_consttime.

`elem_exp_consttime` was written weirdly in that it had a bunch of
wrappers functions around assembly functions, `bn_gather5`, etc.,
where the wrapper functions were not marked `unsafe` but also they
didn't themselves enforce safety properties. It kind of made sense
but technically they should have been `unsafe`. And, there were just a
lot of confusing details in how they were written.

Start over with the wrapper functions, and put the new versions of
them in the `x86_64_mont` submodule.

We intentionally avoid adding any masking to `power` arguments, which
would make them completely safe.

To review this properly, one needs to compare how these functions are
documented and used in `crypto/fipsmodule/bn/internal.h` and
`crypto/fipsmodule/bn/exponentiation.c` (note this file was renamed in
recent versions of BoringSSL; use the version that corresponds to the
most recently-merged BoringSSL in *ring*.)

```
git difftool HEAD^1:src/arithmetic/bigint.rs \
                    src/arithmetic/x86_64_mont.rs
```

Author: briansmith

src/arithmetic/bigint.rs

@@ -415,7 +415,7 @@ pub fn elem_exp_consttime<M>(
     base: Elem<M, R>,
     exponent: &PrivateExponent,
     m: &Modulus<M>,
-) -> Result<Elem<M, Unencoded>, error::Unspecified> {
+) -> Result<Elem<M, Unencoded>, LimbSliceError> {
     use crate::{bssl, limb::Window};
 
     const WINDOW_BITS: usize = 5;
@@ -476,8 +476,7 @@ 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((dst, src1, src2), m.limbs(), m.n0(), m.cpu_features())
-            .map_err(error::erase::<LimbSliceError>)?;
+        limbs_mul_mont((dst, src1, src2), m.limbs(), m.n0(), m.cpu_features())?;
     }
 
     let tmp = m.zero();
@@ -502,12 +501,10 @@ pub fn elem_exp_consttime<M>(
     base: Elem<M, R>,
     exponent: &PrivateExponent,
     m: &Modulus<M>,
-) -> Result<Elem<M, Unencoded>, error::Unspecified> {
+) -> Result<Elem<M, Unencoded>, LimbSliceError> {
+    use super::x86_64_mont::{gather5, mul_mont_gather5_amm, power5_amm, scatter5};
     use crate::{cpu, limb::LIMB_BYTES};
 
-    // Pretty much all the math here requires CPU feature detection to have
-    // been done. `cpu_features` isn't threaded through all the internal
-    // functions, so just make it clear that it has been done at this point.
     let cpu_features = m.cpu_features();
 
     // The x86_64 assembly was written under the assumption that the input data
@@ -534,85 +531,6 @@ pub fn elem_exp_consttime<M>(
         table.split_at_mut(TABLE_ENTRIES * num_limbs)
     };
 
-    fn scatter(table: &mut [Limb], acc: &[Limb], i: LeakyWindow, num_limbs: usize) {
-        prefixed_extern! {
-            fn bn_scatter5(a: *const Limb, a_len: c::size_t, table: *mut Limb, i: LeakyWindow);
-        }
-        unsafe { bn_scatter5(acc.as_ptr(), num_limbs, table.as_mut_ptr(), i) }
-    }
-
-    fn gather(table: &[Limb], acc: &mut [Limb], i: Window, num_limbs: usize) {
-        prefixed_extern! {
-            fn bn_gather5(r: *mut Limb, a_len: c::size_t, table: *const Limb, i: Window);
-        }
-        unsafe { bn_gather5(acc.as_mut_ptr(), num_limbs, table.as_ptr(), i) }
-    }
-
-    fn limbs_mul_mont_gather5_amm(
-        table: &[Limb],
-        acc: &mut [Limb],
-        base: &[Limb],
-        m: &[Limb],
-        n0: &N0,
-        i: Window,
-        num_limbs: usize,
-    ) {
-        prefixed_extern! {
-            fn bn_mul_mont_gather5(
-                rp: *mut Limb,
-                ap: *const Limb,
-                table: *const Limb,
-                np: *const Limb,
-                n0: &N0,
-                num: c::size_t,
-                power: Window,
-            );
-        }
-        unsafe {
-            bn_mul_mont_gather5(
-                acc.as_mut_ptr(),
-                base.as_ptr(),
-                table.as_ptr(),
-                m.as_ptr(),
-                n0,
-                num_limbs,
-                i,
-            );
-        }
-    }
-
-    fn power_amm(
-        table: &[Limb],
-        acc: &mut [Limb],
-        m_cached: &[Limb],
-        n0: &N0,
-        i: Window,
-        num_limbs: usize,
-    ) {
-        prefixed_extern! {
-            fn bn_power5(
-                r: *mut Limb,
-                a: *const Limb,
-                table: *const Limb,
-                n: *const Limb,
-                n0: &N0,
-                num: c::size_t,
-                i: Window,
-            );
-        }
-        unsafe {
-            bn_power5(
-                acc.as_mut_ptr(),
-                acc.as_ptr(),
-                table.as_ptr(),
-                m_cached.as_ptr(),
-                n0,
-                num_limbs,
-                i,
-            );
-        }
-    }
-
     // These are named `(tmp, am, np)` in BoringSSL.
     let (acc, base_cached, m_cached): (&mut [Limb], &[Limb], &[Limb]) = {
         let (acc, rest) = state.split_at_mut(num_limbs);
@@ -639,11 +557,10 @@ pub fn elem_exp_consttime<M>(
         m_cached: &[Limb],
         n0: &N0,
         mut i: LeakyWindow,
-        num_limbs: usize,
         cpu_features: cpu::Features,
     ) -> Result<(), LimbSliceError> {
         loop {
-            scatter(table, acc, i, num_limbs);
+            scatter5(acc, table, i)?;
             i *= 2;
             if i >= TABLE_ENTRIES as LeakyWindow {
                 break;
@@ -657,38 +574,34 @@ pub fn elem_exp_consttime<M>(
 
     // acc = table[0] = base**0 (i.e. 1).
     m.oneR(acc);
-    scatter(table, acc, 0, num_limbs);
+    scatter5(acc, table, 0)?;
 
     // acc = base**1 (i.e. base).
     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)
-        .map_err(error::erase::<LimbSliceError>)?;
+    scatter_powers_of_2(table, acc, m_cached, n0, 1, cpu_features)?;
     // 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) {
-        limbs_mul_mont_gather5_amm(
-            table,
-            acc,
-            base_cached,
-            m_cached,
-            n0,
-            Window::from(i - 1), // Not secret
-            num_limbs,
-        );
-        scatter_powers_of_2(table, acc, m_cached, n0, i, num_limbs, cpu_features)
-            .map_err(error::erase::<LimbSliceError>)?;
+        let power = Window::from(i - 1);
+        assert!(power < 32); // Not secret,
+        unsafe {
+            mul_mont_gather5_amm(acc, base_cached, table, m_cached, n0, power, cpu_features)
+        }?;
+        scatter_powers_of_2(table, acc, m_cached, n0, i, cpu_features)?;
     }
 
     let acc = limb::fold_5_bit_windows(
         exponent.limbs(),
         |initial_window| {
-            gather(table, acc, initial_window, num_limbs);
+            unsafe { gather5(acc, table, initial_window) }
+                .unwrap_or_else(unwrap_impossible_limb_slice_error);
             acc
         },
         |acc, window| {
-            power_amm(table, acc, m_cached, n0, window, num_limbs);
+            unsafe { power5_amm(acc, table, m_cached, n0, window, cpu_features) }
+                .unwrap_or_else(unwrap_impossible_limb_slice_error);
             acc
         },
     );
@@ -766,7 +679,9 @@ mod tests {
                         .expect("valid exponent")
                 };
                 let base = into_encoded(base, &m);
-                let actual_result = elem_exp_consttime(base, &e, &m).unwrap();
+                let actual_result = elem_exp_consttime(base, &e, &m)
+                    .map_err(error::erase::<LimbSliceError>)
+                    .unwrap();
                 assert_elem_eq(&actual_result, &expected_result);
 
                 Ok(())

src/arithmetic/x86_64_mont.rs

@@ -1,4 +1,4 @@
-// Copyright 2024-2025 Brian Smith.
+// Copyright 2015-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
@@ -14,7 +14,7 @@
 
 #![cfg(target_arch = "x86_64")]
 
-use super::{inout::AliasingSlices2, n0::N0, MAX_LIMBS};
+use super::{inout::AliasingSlices2, n0::N0, LimbSliceError, MAX_LIMBS};
 use crate::{
     c,
     cpu::{
@@ -23,9 +23,9 @@ use crate::{
         GetFeature as _,
     },
     error::LenMismatchError,
-    limb::Limb,
+    limb::{LeakyWindow, Limb, Window},
 };
-use core::ops::ControlFlow;
+use core::{num::NonZeroUsize, ops::ControlFlow};
 
 #[inline]
 pub(super) fn bn_sqr8x_mont(
@@ -75,3 +75,171 @@ pub(super) fn bn_sqr8x_mont(
     });
     ControlFlow::Break(r)
 }
+
+#[inline(always)]
+pub(super) fn scatter5(
+    a: &[Limb],
+    table: &mut [Limb],
+    power: LeakyWindow,
+) -> Result<(), LimbSliceError> {
+    prefixed_extern! {
+        // Upstream uses `num: c::size_t` too, and `power: c::size_t`; see
+        // `_MAX_LIMBS_ADDRESSES_MEMORY_SAFETY_ISSUES`.
+        fn bn_scatter5(
+            inp: *const Limb,
+            num: c::NonZero_size_t,
+            table: *mut Limb,
+            power: LeakyWindow,
+        );
+    }
+    let num_limbs = check_common(a, table)?;
+    assert!(power < 32);
+    unsafe { bn_scatter5(a.as_ptr(), num_limbs, table.as_mut_ptr(), power) };
+    Ok(())
+}
+
+// SAFETY: `power` must be less than 32.
+#[inline(always)]
+pub(super) unsafe fn gather5(
+    r: &mut [Limb],
+    table: &[Limb],
+    power: Window,
+) -> Result<(), LimbSliceError> {
+    prefixed_extern! {
+        // Upstream uses `num: c::size_t` too, and `power: c::size_t`; see
+        // `_MAX_LIMBS_ADDRESSES_MEMORY_SAFETY_ISSUES`.
+        pub(super) fn bn_gather5(
+            out: *mut Limb,
+            num: c::NonZero_size_t,
+            table: *const Limb,
+            power: Window);
+    }
+    let num_limbs = check_common(r, table)?;
+    // SAFETY: We cannot assert that `power` is in range because it is secret.
+    // TODO: Create a `Window5` type that is guaranteed to be in range.
+    unsafe { bn_gather5(r.as_mut_ptr(), num_limbs, table.as_ptr(), power) };
+    Ok(())
+}
+
+// SAFETY: `power` must be less than 32.
+#[inline(always)]
+pub(super) unsafe fn mul_mont_gather5_amm(
+    r: &mut [Limb],
+    a: &[Limb],
+    table: &[Limb],
+    n: &[Limb],
+    n0: &N0,
+    power: Window,
+    _maybe_adx_bmi1_bmi2: cpu::Features, // TODO: Option<(Adx, Bmi1, Bmi2)>,
+) -> Result<(), LimbSliceError> {
+    prefixed_extern! {
+        // Upstream has `num: c::int` and `power: c::int`; see
+        // `_MAX_LIMBS_ADDRESSES_MEMORY_SAFETY_ISSUES`.
+        pub(super) fn bn_mul_mont_gather5(
+            rp: *mut Limb,
+            ap: *const Limb,
+            table: *const Limb,
+            np: *const Limb,
+            n0: &N0,
+            num: c::NonZero_size_t,
+            power: Window,
+        );
+    }
+    let num_limbs = check_common_with_n(r, table, n)?;
+    if a.len() != num_limbs.get() {
+        return Err(LimbSliceError::len_mismatch(LenMismatchError::new(a.len())));
+    }
+    // SAFETY: We cannot assert that `power` is in range because it is secret.
+    // TODO: Create a `Window5` type that is guaranteed to be in range.
+    unsafe {
+        bn_mul_mont_gather5(
+            r.as_mut_ptr(),
+            a.as_ptr(),
+            table.as_ptr(),
+            n.as_ptr(),
+            n0,
+            num_limbs,
+            power,
+        )
+    };
+    Ok(())
+}
+
+// SAFETY: `power` must be less than 32.
+#[inline(always)]
+pub(super) unsafe fn power5_amm(
+    in_out: &mut [Limb],
+    table: &[Limb],
+    n: &[Limb],
+    n0: &N0,
+    power: Window,
+    _maybe_adx_bmi1_bmi2: cpu::Features, // TODO: Option<(Adx, Bmi1, Bmi2)>,
+) -> Result<(), LimbSliceError> {
+    prefixed_extern! {
+        // Upstream has `num: c::int` and `power: c::int`; see
+        // `_MAX_LIMBS_ADDRESSES_MEMORY_SAFETY_ISSUES`.
+        fn bn_power5(
+            rp: *mut Limb,
+            ap: *const Limb,
+            table: *const Limb,
+            np: *const Limb,
+            n0: &N0,
+            num: c::NonZero_size_t,
+            power: Window,
+        );
+    }
+    let num_limbs = check_common_with_n(in_out, table, n)?;
+    // SAFETY: We cannot assert that `power` is in range because it is secret.
+    // TODO: Create a `Window5` type that is guaranteed to be in range.
+    unsafe {
+        bn_power5(
+            in_out.as_mut_ptr(),
+            in_out.as_ptr(),
+            table.as_ptr(),
+            n.as_ptr(),
+            n0,
+            num_limbs,
+            power,
+        );
+    }
+    Ok(())
+}
+
+// Helps the compiler will be able to hoist all of these checks out of the
+// loops in the caller. Try to help the compiler by doing the checks
+// consistently in each function and also by inlining this function and all the
+// callers.
+#[inline(always)]
+fn check_common(a: &[Limb], table: &[Limb]) -> Result<NonZeroUsize, LimbSliceError> {
+    assert_eq!((table.as_ptr() as usize) % 16, 0); // According to BoringSSL.
+    let num_limbs = NonZeroUsize::new(a.len()).ok_or_else(|| LimbSliceError::too_short(a.len()))?;
+    if num_limbs.get() % 8 != 0 {
+        // TODO: Use a different error.
+        return Err(LimbSliceError::len_mismatch(LenMismatchError::new(a.len())));
+    }
+    if num_limbs.get() > MAX_LIMBS {
+        return Err(LimbSliceError::too_long(a.len()));
+    }
+    if num_limbs.get() * 32 != table.len() {
+        return Err(LimbSliceError::len_mismatch(LenMismatchError::new(
+            table.len(),
+        )));
+    };
+    Ok(num_limbs)
+}
+
+#[inline(always)]
+fn check_common_with_n(
+    a: &[Limb],
+    table: &[Limb],
+    n: &[Limb],
+) -> Result<NonZeroUsize, LimbSliceError> {
+    // Choose `a` instead of `n` so that every function starts with
+    // `check_common` passing the exact same arguments, so that the compiler
+    // can easily de-dupe the checks.
+    let num_limbs = check_common(a, table)?;
+    if n.len() != num_limbs.get() {
+        return Err(LimbSliceError::len_mismatch(LenMismatchError::new(n.len())));
+    }
+    Ok(num_limbs)
+}