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: <20250919205314.GA1884303@joelbox2>
Date: Fri, 19 Sep 2025 16:53:14 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: 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 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).

thanks,

 - Joel