linux-kernel - Re: [PATCH RFC v2 3/3] gpu: nova-core: use BoundedInt

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: <b13c6327-bd3a-448e-8825-1cf81bb16ef5@nvidia.com>
Date: Mon, 13 Oct 2025 09:58:56 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: Yury Norov <yury.norov@...il.com>, Alexandre Courbot <acourbot@...dia.com>
Cc: Danilo Krummrich <dakr@...nel.org>, Jesung Yang <y.j3ms.n@...il.com>,
 Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feong@...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>,
 nouveau@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org
Subject: Re: [PATCH RFC v2 3/3] gpu: nova-core: use BoundedInt

Hi Yury,

On 10/10/2025 1:20 PM, Yury Norov wrote:
[...]
>>>>          regs::NV_PFALCON_FALCON_DMATRFBASE1::default()
>>>> -            .set_base((dma_start >> 40) as u16)
>>>> +            .try_set_base(dma_start >> 40)?
>>>>              .write(bar, &E::ID);
>>>
>>> Does it mean that something like the following syntax is possible?
>>>
>>>         regs::NV_PFALCON_FALCON_DMATRFBASE1::default()
>>>             .try_set_base1(base1 >> 40)?        // fail here
>>>             .try_set_base2(base2 >> 40)?        // skip
>>>             .write(bar, &E::ID) else { pr_err!(); return -EINVAL };
>>>
>>> This is my main concern: Rust is advertised a as runtime-safe language
>>> (at lease safer than C), but current design isn't safe against one of
>>> the most common errors: type overflow.
>>
>> Not sure I understand what you mean, but if you are talking about fields
>> overflow, this cannot happen with the current design. The non-fallible
>> setter can only be invoked if the compiler can prove that the argument
>> does fit withing the field. Otherwise, one has to use the fallible
>> setter (as this chunk does, because `dma_start >> 40` can still spill
>> over the capacity of `base`), which performs a runtime check and returns
>> `EOVERFLOW` if the value didn't fit.
>  
> Yeah, this design addresses my major question to the bitfields series
> from Joel: setters must be fallible. I played with this approach, and
> it does exactly what I have in mind.
> 
> I still have a question regarding compile-time flavor of the setter.
> In C we've got a builtin_constant_p, and use it like:
>         
>    static inline int set_base(unsigned int base)
>    {
>         BUILD_BUG_ON_ZERO(const_true(base > MAX_BASE));
> 
>         // Eliminated for compile-time 'base'
>         if (base > MAX_BASE)
>                 return -EOVERFLOW;
> 
>         __set_base(base);
> 
>         return 0;
>    }
> 
> Can we do the same trick in rust? Would be nice to have a single
> setter for both compile and runtime cases.

I don't think we could combine the setter and try setter variants on the rust
side, because the former returns Self and the latter returns Result. Also, both
the variants already have compile time asserts which may cover what you're
referring to.

The try setter variants in fact are not strictly needed, because the user can
provide a bounded integer (after performing any fallible conversions on the
caller side). Alex and me discussed adding that for a better user/caller
experience [1].

[1] https://lore.kernel.org/all/C35B5306-98C6-447B-A239-9D6A6C548A4F@nvidia.com/

Or did you mean something else?

thanks,

 - Joel