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: <509c3fb5-310a-43ab-ab84-75207e0c577e@nvidia.com>
Date: Mon, 28 Apr 2025 12:54:19 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Tamir Duberstein <tamird@...il.com>, 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>, 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 4/28/25 2:40 AM, Alice Ryhl wrote:
> On Sun, Apr 27, 2025 at 03:59:48PM -0700, John Hubbard wrote:
>> On 4/23/25 10:40 AM, Tamir Duberstein wrote:
>> ...
>>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>>> index 1df11156302a..d14ed86efb68 100644
>>> --- a/rust/kernel/lib.rs
>>> +++ b/rust/kernel/lib.rs
>>> @@ -198,9 +198,15 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
>>>   /// ```
>>>   #[macro_export]
>>>   macro_rules! container_of {
>>> -    ($ptr:expr, $type:ty, $($f:tt)*) => {{
>>> -        let offset: usize = ::core::mem::offset_of!($type, $($f)*);
>>> -        $ptr.byte_sub(offset).cast::<$type>()
>>> +    ($field_ptr:expr, $Container:ty, $($fields:tt)*) => {{
>>> +        let offset: usize = ::core::mem::offset_of!($Container, $($fields)*);
>>> +        let field_ptr = $field_ptr;
>>> +        let container_ptr = field_ptr.byte_sub(offset).cast::<$Container>();
>>> +        if false {
>>
>> This jumped out at me. It's something that I'd like to recommend NOT
>> doing, here or anywhere else, because:
>>
>>     a) Anything of the form "if false" will get removed by any compiler
>>        worthy of the name, especially in kernel builds.
> 
> The `if false` branch is used to trigger a compilation failure when the
> macro is used incorrectly. The intent is that the compiler should
> optimize it out. I don't think there's anything wrong with that pattern.

OK...probably best to either encapsulate that, or at least comment
it. I'm accustomed to seeing that pattern in cases where people
expected the code to *not* get optimized out, so it triggers me. :)

thanks,
-- 
John Hubbard