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] [day] [month] [year] [list]

Message-Id: <DCZ44OA67SLZ.HKORUXU2L9K2@nvidia.com>
Date: Mon, 22 Sep 2025 15:25:00 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Joel Fernandes" <joelagnelf@...dia.com>, "Benno Lossin"
 <lossin@...nel.org>
Cc: "Danilo Krummrich" <dakr@...nel.org>, "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>, "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" <linux-kernel@...r.kernel.org>,
 "rust-for-linux@...r.kernel.org" <rust-for-linux@...r.kernel.org>,
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Subject: Re: [PATCH] rust: io: use const generics for read/write offsets

On Sat Sep 20, 2025 at 5:53 AM JST, Joel Fernandes wrote:
> On Fri, Sep 19, 2025 at 11:26:19AM +0200, Benno Lossin wrote:
>> On Fri Sep 19, 2025 at 9:59 AM CEST, Joel Fernandes wrote:
>> > Hello, Danilo,
>> >
>> >> On Sep 19, 2025, at 1:26 AM, Danilo Krummrich <dakr@...nel.org> wrote:
>> >> 
>> >> 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.
>> >
>> > I did not mean to speak for others, sorry it came across like that
>> > (and that is certainly not what I normally do). But I discussed the
>> > patch in person since we are at a conference and discussing it in
>> > person, and I did not get a lot of consensus on this. That is what I
>> > was trying to say. If it was a brilliant or great idea, I would have
>> > hoped for at least one person to tell me that this is exactly how we
>> > should do it.
>> 
>> I'm also not really thrilled to see lots more turbofish syntax. However,
>> if we can avoid the nasty build_assert errors then in my opinion it's
>> better. (yes we do have Gary's cool klint tool to handle them correctly,
>
> Yes, thanks. Also I tried to apply this patch and it doesn't always work
> because of array indexing usecase in nova, where we compute the offset based
> on a runtime register index  (**/nova-core/**/macros.rs). Here idx is not a
> constant:
>
>             /// Read the array register at index `idx` from its address in `io`.
>             #[inline(always)]
>             pub(crate) fn read<const SIZE: usize, T>(
>                 io: &T,
>                 idx: usize,
>             ) -> Self where
>                 T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
>
> In **/ga102.rs, we have the following usage where ucode_idx cannot be const:
>
> regs::NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION::read(bar, ucode_idx).data()
>
> So I am afraid this wont work. Also even if it did work, it means now we have
> to also put idx as a const generic (turbofish syntax).

We could always use the `try_read*` variant for these, but that would
introduce runtime checking for errors that can't happen. We have been
pretty successful in avoiding using `try_read*` in Nova so far, and I
think that's something we should try to preserve as it brings confidence
that our register accesses are correct.