linux-kernel - Re: [PATCH] gpu: nova-core: define named constants for magic numbers

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: <cda61d51-ad85-4464-a637-426b960a83c6@gmail.com>
Date: Sun, 13 Jul 2025 23:10:37 -0700
From: Rhys Lloyd <krakow20@...il.com>
To: Alexandre Courbot <acourbot@...dia.com>, dakr@...nel.org
Cc: airlied@...il.com, simona@...ll.ch, nouveau@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] gpu: nova-core: define named constants for magic numbers

On 7/13/25 8:11 PM, Alexandre Courbot wrote:
> On Sun Jul 13, 2025 at 11:51 AM JST, Rhys Lloyd wrote:
>> Introduce an associated constant `MIN_LEN` for each struct that checks
>> the length of the input data in its constructor against a magic number.
>>
>> Signed-off-by: Rhys Lloyd <krakow20@...il.com>
> As I mentioned in [1], I think this would be better addressed by working
> in terms of `sizeof` upon the relevant structures, after making them
> `#[repr(C)]`. It might require splitting them a bit since some contain
> other data (or we can maybe turn them into DSTs).
>
> [1] https://lore.kernel.org/rust-for-linux/DB97X8JAJFI4.3G1I8ZPC1MWLS@nvidia.com/

As far as I can tell, only one of the five structs with `MIN_LEN` have 
the same layout in-memory as they do in the `data` byte slice, that 
being `BitHeader`.  Perhaps `#[repr(packed)]` could be used for 
`PmuLookupTableEntry`, sacrificing alignment, but that is undesirable as 
it comes with its own footguns such as unaligned loads.  The other 
structs include optional values and vectors which do not have the same 
encoding when reading from the `data` byte slice as they do in memory.  
I have worked with DSTs before, but I don't recommend them for 
non-library code since they are not first-class citizens in Rust.  
Notably the fat pointer is not resized when taking a reference to the 
unsized struct field, and constructing such objects is cumbersome.  
Also, in the current version of Rust (1.88), DSTs cannot yet live 
comfortably on the stack.

This patch can be dropped if it's not valuable enough to warrant the 
change, I only made it because of your comment here: 
https://gitlab.freedesktop.org/drm/nova/-/merge_requests/4#note_2999761