linux-kernel - Re: [PATCH v5 10/17] rust: workqueue: replace `raw

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>] [day] [month] [year] [list]

Message-ID: <D8NKWUQK703Y.38SYPI7LCXKO3@proton.me>
Date: Sun, 23 Mar 2025 10:38:17 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Antonio Hickey <contact@...oniohickey.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>, Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 10/17] rust: workqueue: replace `raw_get` with pointer cast

On Thu Mar 20, 2025 at 3:07 AM CET, Antonio Hickey wrote:
> Now that `Work` is initialized via `pin-init`, the `work` field
> is always used. This allows us to replace the use of unsafe
> `Opaque::raw_get` with direct pointer casting.
>
> Suggested-by: Boqun Feng <boqun.feng@...il.com>
> Link: https://lore.kernel.org/all/20250316061429.817126-1-contact@antoniohickey.com/T/#mc7a4757e8c132f84228b728c7d123d73841501d6
> Signed-off-by: Antonio Hickey <contact@...oniohickey.com>

One nit below, but regardless:

Reviewed-by: Benno Lossin <benno.lossin@...ton.me>

> ---
>  rust/kernel/workqueue.rs | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 4e27df324d26..5cc259c1e502 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -399,12 +399,8 @@ pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self
>      /// need not be initialized.)
>      #[inline]
>      pub unsafe fn raw_get(ptr: *const Self) -> *mut bindings::work_struct {
> -        // SAFETY: The caller promises that the pointer is aligned and not dangling.
> -        //
> -        // A pointer cast would also be ok due to `#[repr(transparent)]`. We use
> -        // `&raw const (*ptr).work` so that the compiler does not complain that the
> -        // `work` field is unused.
> -        unsafe { Opaque::raw_get(&raw const (*ptr).work) }
> +        // CAST: `Work` is transparent to `bindings::work_struct`.

I usually write:

    // CAST: `Work` is transparently wrapping `bindings::work_struct`.

So feel free to use that. (I find the "transparent to" a bit strange,
but this isn't a huge complaint)

---
Cheers,
Benno

> +        ptr.cast_mut().cast()
>      }
>  }
>