linux-kernel - Re: [PATCH v3] rust: check type of `$ptr` in `container

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: <CANiq72kQWNfSV0KK6qs6oJt+aGdgY=hXg=wJcmK3zYcokY1LNw@mail.gmail.com>
Date: Thu, 24 Apr 2025 14:48:19 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Tamir Duberstein <tamird@...il.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] rust: check type of `$ptr` in `container_of!`

On Wed, Apr 23, 2025 at 7:40 PM Tamir Duberstein <tamird@...il.com> wrote:
>
> +        if false {
> +            let container_field_ptr = ::core::ptr::addr_of!((*container_ptr).$($fields)*).cast_mut();
> +            [field_ptr, container_field_ptr]; // typeof(`$field_ptr`) == typeof(`$Container.$($fields)*`)

If I understand correctly, we keep the `// typeof ...` in the same
line so that it appears in the error message and thus it is clearer to
the user, right?

In that case, could we nevertheless please clarify things a bit at the
top of the `if false` block, i.e. something like:

    // Ensure that both types are equal while avoiding codegen .....
    // ... i.e. effectively compare `typeof(...) == ...`.
    if false {
        ...

etc.?

Or, perhaps even better, we move this into its own macro, so that we
document it there and why we chose this particular approach, assuming
the error message still prints the right thing.

Speaking of magic, to be honest, is this approach worth it? I liked v1
quite more. The error seems concise enough, and the first line that
the compiler points out is `assert_same_type` which makes it super
clear, and showed the actual expressions involved without using a
comment.

With v1, we could also just put `assert_same_type` outside as a
utility for others to use, i.e. in the `kernel` crate, which
simplifies things and makes the error a bit shorter. Moving the
function out makes the error slightly shorter, would also allow us to
document its usage, including the suggestion to use `if false` in an
example.

Regarding the `if false`, the kernel is always built with at least
-O2. Benno mentioned debug performance -- was that related to
something like debug assertions being enabled or just optimization
level? Either way, even with the assertions enabled, I don't see it in
codegen.

Am I missing something?

Thanks!

Cheers,
Miguel