linux-kernel - Re: [PATCH v9] 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: <705CD461-60D9-492E-B82F-C88A848A4348@collabora.com>
Date: Wed, 16 Jul 2025 16:11:59 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>,
 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>,
 linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org,
 Alexandre Courbot <acourbot@...dia.com>
Subject: Re: [PATCH v9] rust: kernel: add support for bits/genmask macros

Hi Danilo,

> 
> Just for reference, I asked some questions regarding this code in [1].
> 
> Additional to "Why does genmask_u64(0..=100) compile?" I would expect the
> corresponding genmask_checked_u64() call to return None.
> 
> [1] https://lore.kernel.org/lkml/DBDP0BJW9VAZ.5KRU4V4288R8@kernel.org/
> 

Let’s transfer this discussion to this patch.

> I also quickly tried genmask and I have a few questions:
> 
>  (1) Why does genmask not use a const generic? I think this makes it more
>      obvious that it's only intended to be used from const context.

I guess none of us thought about it, since the current version also works.

> 
>  (2) Why is there no build_assert() when the range exceeds the number of bits
>      of the target type? I would expect genmask_u64(0..100) to fail.

Doesn’t it?

There is a build_assert in the underlying bit implementation. It was redundant
to have it both in bit_* and in genmask, because genmask calls bit().

In your example, bit_u64(100) hits that assert, and so it shouldn't compile.

Note that I can't add a test for this because "compile_fail" tests are not
supported yet, but there is a test for the checked version, i.e.:

+    /// // `80` is out of the supported bit range.
+    /// assert_eq!(genmask_checked_u64(21..=80), None);

> Additional to "Why does genmask_u64(0..=100) compile?" I would expect the
> corresponding genmask_checked_u64() call to return None.

The test above proves that it returns None.

> 
>  (3) OOC, why did you choose u32 as argument type?

No reason. i32 is the default integer type and signed integers don’t make
sense here, so I chose u32.

Also, we can trivially promote integers to wider types,  but the other way
around is not true. So my reasoning was that if you had u8, or u16s you could
trivially get u32s using into(), but if you had u32s and e.g. genmask_u16
required u16s, you'd have to resort to try_into() or `as`, which is annoying.

In any case, feel free to suggest anything else, I think.

— Daniel