linux-kernel - Re: [PATCH v6] rust: kernel: add support for bits/genmask macros

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: <aFAqLYBMHkC_X-dr@Mac.home>
Date: Mon, 16 Jun 2025 07:29:01 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Alexandre Courbot <acourbot@...dia.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v6] rust: kernel: add support for bits/genmask macros

On Mon, Jun 16, 2025 at 11:14:58AM -0300, Daniel Almeida wrote:
> Hi,
> 
[...]
> >> +        }
> >> +
> >> +        /// Computes `1 << n` by performing a compile-time assertion that `n` is
> >> +        /// in bounds.
> >> +        ///
> >> +        /// This version is the default and should be used if `n` is known at
> >> +        /// compile time.
> >> +        #[inline]
> >> +        pub const fn $const_name(n: u32) -> $ty {
> >> +            build_assert!(n < <$ty>::BITS);
> >> +            1 as $ty << n
> >> +        }
> >> +    };
> >> +}
> >> +
> >> +impl_bit_fn!(checked_bit_u64, unbounded_bit_u64, bit_u64, u64);
> >> +impl_bit_fn!(checked_bit_u32, unbounded_bit_u32, bit_u32, u32);
> >> +impl_bit_fn!(checked_bit_u16, unbounded_bit_u16, bit_u16, u16);
> >> +impl_bit_fn!(checked_bit_u8, unbounded_bit_u8, bit_u8, u8);
> >> +
> >> +macro_rules! impl_genmask_fn {
> >> +    (
> >> +        $ty:ty, $checked_bit:ident, $bit:ident, $genmask:ident, $genmask_checked:ident, $genmask_unbounded:ident,
> >> +        $(#[$genmask_ex:meta])*
> >> +    ) => {
> >> +        /// Creates a compile-time contiguous bitmask for the given range by
> >> +        /// validating the range at runtime.
> >> +        ///
> >> +        /// Returns [`None`] if the range is invalid, i.e.: if the start is
> >> +        /// greater than or equal to the end.
> >> +        #[inline]
> >> +        pub fn $genmask_checked(range: Range<u32>) -> Option<$ty> {
> >> +            if range.start >= range.end || range.end > <$ty>::BITS {
> >> +                return None;
> >> +            }
> > 
> > From this check I assumed that you interpret `range` as non-inclusive,
> > since `range.end == 32` is valid on u32...
> > 
> >> +            let high = $checked_bit(range.end)?;
> > 
> > ... however IIUC `checked_bit` will return `None` here in such a case.
> > Should the argument be `range.end - 1`?
> > 
> > Your examples do seem to interpret the range as inclusive though, so
> > probably the check should be `|| range.end >= <$ty>::BITS`. But that
> > triggers the question, is it ok to use `Range` that way, when its
> > documentation specifically states that it is bounded exclusively above?
> > We could use `RangeInclusive` to match the semantics, which would
> > require us to write the ranges as `0..=7`. At least it is clear that the
> > upper bound is inclusive.
> 
> Sorry, the idea was to indeed interpret a..b as inclusive. I specifically

Please don't do this.

> thought we'd suprise a lot of people if we deviated from the way genmask works
> in C. In other words, I assumed a lot of people would write a..b, when what
> they meant is a..=b.
> 

We should tell/educate people to do the right thing, if a..b is not
inclusive in Rust, then we should treat them as non-inclusive in Rust
kernel code. Otherwise you create confusion for no reason. My assumption
is that most people will ask "what's the right way to do this" first
instead of replicating the old way.

Regards,
Boqun

> > 
[...]