linux-kernel - Re: [PATCH 1/6] rust: num: add `shr` and `shl` methods to `Bounded`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXCLNRVGGqsYeiBM@yury>
Date: Wed, 21 Jan 2026 03:15:49 -0500
From: Yury Norov <ynorov@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>,
	Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>,
	Yury Norov <yury.norov@...il.com>,
	John Hubbard <jhubbard@...dia.com>,
	Alistair Popple <apopple@...dia.com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	Timur Tabi <ttabi@...dia.com>, Edwin Peer <epeer@...dia.com>,
	Eliot Courtney <ecourtney@...dia.com>,
	Daniel Almeida <daniel.almeida@...labora.com>,
	Dirk Behme <dirk.behme@...bosch.com>,
	Steven Price <steven.price@....com>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] rust: num: add `shr` and `shl` methods to `Bounded`

On Tue, Jan 20, 2026 at 03:17:54PM +0900, Alexandre Courbot wrote:
> Shifting a `Bounded` left or right changes the number of bits required
> to represent the value. Add methods that perform the shift and return a
> `Bounded` with the appropriately adjusted bit width.
> 
> These methods are particularly useful for bitfield extraction.
> 
> Suggested-by: Alice Ryhl <aliceryhl@...gle.com>
> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
> ---
>  rust/kernel/num/bounded.rs | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
> index 5ef8361cf5d5..6e3f4a7a5262 100644
> --- a/rust/kernel/num/bounded.rs
> +++ b/rust/kernel/num/bounded.rs
> @@ -475,6 +475,46 @@ pub fn cast<U>(self) -> Bounded<U, N>
>          // `N` bits, and with the same signedness.
>          unsafe { Bounded::__new(value) }
>      }
> +
> +    /// Right-shifts `self` by `SHIFT` and returns the result as a `Bounded<_, { N - SHIFT }>`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::num::Bounded;
> +    ///
> +    /// let v = Bounded::<u32, 16>::new::<0xff00>();
> +    /// let v_shifted: Bounded::<u32, 8> = v.shr::<8, _>();

This syntax is really confusing. Does it work for runtime shifts?
Is there any chance to make it just v.shr(8)?

> +    ///
> +    /// assert_eq!(v_shifted.get(), 0xff);
> +    /// ```
> +    pub fn shr<const SHIFT: u32, const RES: u32>(self) -> Bounded<T, RES> {
> +        const { assert!(RES == N - SHIFT) }
> +
> +        // 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`.
> +        unsafe { Bounded::__new(self.0 >> SHIFT) }

Assuming you relax it to RES >= N - SHIFT, as suggested by Alice,
you'd also check for SHIFT < N.

> +    }
> +
> +    /// Left-shifts `self` by `SHIFT` and returns the result as a `Bounded<_, { N + SHIFT }>`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::num::Bounded;
> +    ///
> +    /// let v = Bounded::<u32, 8>::new::<0xff>();
> +    /// let v_shifted: Bounded::<u32, 16> = v.shl::<8, _>();
> +    ///
> +    /// assert_eq!(v_shifted.get(), 0xff00);
> +    /// ```
> +    pub fn shl<const SHIFT: u32, const RES: u32>(self) -> Bounded<T, RES> {
> +        const { assert!(RES == N + SHIFT) }
> +
> +        // SAFETY: we shift the value left by `SHIFT`, augmenting the number of bits needed to
> +        // represent the shifted value by as much, and just asserted that `RES == N + SHIFT`.
> +        unsafe { Bounded::__new(self.0 << SHIFT) }
> +    }

So, it protects most significant bits when shifting left, but doesn't
protect least significant bits when shifting right.

It also makes impossible to left-shift Bounded::<u32, 32> at all, or
shift Bounded::<u32, 31> for 2 or more bits. This doesn't look nice.

At this layer, there's seemingly nothing wrong to loose bits during
regular shift, just like non-bounded integers do. (Lets consider them
naturally bounded.) At higher layers, people may add any extra checks
as desired.

Even more, you mention you're going to use .shl and .shr for bitfield
extraction, which means you want to loose some bits intentionally.

Let's design shifts like this. Plain .shl() and .shr() will operate on
bounded integers just like '<<' and '>>' operate on non-bounded ones,
i.e. they may loose bits and the result has the same bit capacity. (But
shifting over the capacity should be be forbidden as undef).

If I want to adjust the capacity, I just do it explicitly:

    let x = Bounded::<u32, 12>::new::<0x123>();
    let a = x.shl(4); // 12-bit 0x230
    let b = x.extend::<12+4>().shl(4) // 16-bit 0x1230
    let c = x.shr(4); // 12-bit 0x012
    let d = if x & 0xf { None } else { x.shr(4).try_shrink::<12-4>() }

For b and d you can invent handy helpers, of course, and for a and c
you can add 'safe' versions that will check shifted-out parts for
emptiness at runtime, in case you need it.