linux-kernel - Re: [PATCH] rust: io: use const generics for read/write offsets

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: <DCWBCL9U0IY4.NFNUMLRULAWM@kernel.org>
Date: Fri, 19 Sep 2025 01:26:28 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Joel Fernandes" <joelagnelf@...dia.com>
Cc: "Alice Ryhl" <aliceryhl@...gle.com>, "Maarten Lankhorst"
 <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
 "Thomas Zimmermann" <tzimmermann@...e.de>, "David Airlie"
 <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>, "Greg
 Kroah-Hartman" <gregkh@...uxfoundation.org>, "Rafael J. Wysocki"
 <rafael@...nel.org>, "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>, "Trevor
 Gross" <tmgross@...ch.edu>, "Bjorn Helgaas" <bhelgaas@...gle.com>,
 Krzysztof Wilczyński <kwilczynski@...nel.org>,
 <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>,
 <dri-devel@...ts.freedesktop.org>, <linux-pci@...r.kernel.org>
Subject: Re: [PATCH] rust: io: use const generics for read/write offsets

On Thu Sep 18, 2025 at 8:13 PM CEST, Joel Fernandes wrote:
> On Thu, Sep 18, 2025 at 03:02:11PM +0000, Alice Ryhl wrote:
>> Using build_assert! to assert that offsets are in bounds is really
>> fragile and likely to result in spurious and hard-to-debug build
>> failures. Therefore, build_assert! should be avoided for this case.
>> Thus, update the code to perform the check in const evaluation instead.
>
> I really don't think this patch is a good idea (and nobody I spoke to thinks
> so). Not only does it mess up the user's caller syntax completely, it is also

I appreacite you raising the concern, but I rather have other people speak up
themselves.

> super confusing to pass both a generic and a function argument separately.

Why? We assert that the offset has to be const, whereas the value does not
have this requirement, so this makes perfect sense to me.

(I agree that it can look unfamiliar at the first glance though.)

> Sorry if I knew this would be the syntax, I would have objected even when we
> spoke :)
>
> I think the best fix (from any I've seen so far), is to move the bindings
> calls of offending code into a closure and call the closure directly, as I
> posted in the other thread. I also passed the closure idea by Gary and he
> confirmed the compiler should behave correctly (I will check the code gen
> with/without later). Gary also provided a brilliant suggestion that we can
> call the closure directly instead of assigning it to a variable first. That
> fix is also smaller, and does not screw up the users. APIs should fix issues
> within them instead of relying on user to work around them.

This is not a workaround, this is an idiomatic solution (which I probably should
have been doing already when I introduced the I/O code).

We do exactly the same thing for DmaMask::new() [1] and we agreed on doing the
same thing for Alignment as well [2].

[1] https://rust.docs.kernel.org/kernel/dma/struct.DmaMask.html#method.new
[2] https://lore.kernel.org/rust-for-linux/20250908-num-v5-1-c0f2f681ea96@nvidia.com/