Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] This problem wasn't introduced by this patch, but Bounded::shr lacks a bound check for SHIFT < T::BITS, allowing wrapping shifts that break safety invariants and lead to Undefined Behavior. -- --- Patch [1]: [PATCH v2 01/12] rust: num: use const_assert! in Bounded --- commit afcfa782abd748c8cc818867c320e813e00f8278 Author: Eliot Courtney <[email protected]> rust: num: use const_assert! in Bounded This patch converts the const-block asserts in bounded.rs to use the const_assert! macro, matching the styling in the rest of the file. > diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs > index dafe77782d797..9ad7df1a243d9 100644 > --- a/rust/kernel/num/bounded.rs > +++ b/rust/kernel/num/bounded.rs > @@ -485,7 +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!(RES + SHIFT >= N) } > + const_assert!(RES + SHIFT >= N); [Severity: Critical] This isn't a bug introduced by this patch, but does Bounded::shr lack a bounds check for SHIFT being less than the number of bits in the type? If a caller invokes Bounded::shr with a SHIFT value greater than or equal to the bit width of the type, the shift amount can be masked in release builds without overflow checks. This would leave the original value unchanged. Because the condition RES + SHIFT >= N would still be satisfied, this unchanged value is unsafely wrapped in the returned Bounded type. If the value exceeds RES bits, it breaks the safety invariant of the struct. Subsequent safe calls like Bounded::cast could then trigger undefined behavior when encountering unwrap_unchecked on a failed try_from conversion. Could we add an additional bounds check here to prevent wrapping shifts? > > // SAFETY: We shift the value right by `SHIFT`, reducing the number > of bits needed to > // represent the shifted value by as much, and just asserted that > `RES >= N - SHIFT`. [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
