Re: [PATCH 02/12] rust: num: reject Bounded::shr overshifts at build time

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The `Bounded` struct derives `Default`, which safely bypasses the required compile-time invariant checks in `Bounded::__new`.
--

--- Patch [2]: [PATCH 02/12] rust: num: reject Bounded::shr overshifts at build time ---
commit a9c888d83116c04cce01b982b58983e2d81e53c5
Author: Eliot Courtney <[email protected]>

rust: num: reject Bounded::shr overshifts at build time

This commit updates shr in the Bounded struct to reject shifts that are
greater than or equal to the type's bit width at build time. This prevents
runtime panics or masked shift amounts.

> diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
> index dafe77782d797..f263107f001ed 100644
> --- a/rust/kernel/num/bounded.rs
> +++ b/rust/kernel/num/bounded.rs

[Severity: Medium]
This is a pre-existing issue, but does deriving Default on Bounded safely
bypass the required compile-time invariant checks?

Looking at the struct definition in rust/kernel/num/bounded.rs:

#[derive(Clone, Copy, Debug, Default, Hash)]
pub struct Bounded<T: Integer, const N: u32>(T);

The Bounded struct defines strict type invariants (such as N > 0 and
N <= T::BITS) which are enforced at compile time in its __new
constructor. However, deriving Default automatically implements a constructor
that initializes the tuple struct field via Self(T::default()), completely
bypassing __new.

Can safe downstream code construct an invalid instance using
Bounded::<u32, 0>::default()?

If this invalid instance is dereferenced, deref evaluates fits_within(0, 0),
calculating 32 - 0 = 32. Does left-shifting by 32 bits then trigger a runtime
panic in debug builds due to an overshift, or mask the shift amount in release
builds?

[ ... ]

> @@ -485,6 +485,7 @@ pub fn cast<U>(self) -> Bounded<U, N>
>      /// assert_eq!(v_shifted.get(), 0xff);
>      /// ```
>      pub fn shr<const SHIFT: u32, const RES: u32>(self) -> Bounded<T, RES> {
> +        const { assert!(SHIFT < T::BITS) }
>          const { assert!(RES + SHIFT >= N) }
>  
>          // SAFETY: We shift the value right by `SHIFT`, reducing the number of bits needed to

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2