linux-kernel - Re: [PATCH] rust: types: add Opaque::from

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: <CAJ-ks9=Sy3S8Rir-RftkF7_ZCV4WVsCOPHYX3WN41MY1ukFwKQ@mail.gmail.com>
Date: Tue, 17 Jun 2025 10:40:09 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Miguel Ojeda <ojeda@...nel.org>, Danilo Krummrich <dakr@...nel.org>, Boqun Feng <boqun.feng@...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>, 
	Trevor Gross <tmgross@...ch.edu>, dri-devel@...ts.freedesktop.org, 
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rust: types: add Opaque::from_raw

On Tue, Jun 17, 2025 at 9:38 AM Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
> `container_of!`") we have enforced that the field pointer passed to
> container_of! must match the declared field. This caused mismatches when
> using a pointer to bindings::x for fields of type Opaque<bindings::x>.
>
> This situation encourages the user to simply pass field.cast() to the
> container_of! macro, but this is not great because you might
> accidentally pass a *mut bindings::y when the field type is
> Opaque<bindings::x>, which would be wrong.
>
> To help catch this kind of mistake, add a new Opaque::from_raw that
> wraps a raw pointer in Opaque without changing the inner type.
>
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>

Reviewed-by: Tamir Duberstein <tamird@...il.com>

> ---
>  rust/kernel/drm/device.rs  | 4 +---
>  rust/kernel/drm/gem/mod.rs | 4 +---
>  rust/kernel/lib.rs         | 7 +++++++
>  rust/kernel/types.rs       | 5 +++++
>  4 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 624d7a4c83ead64b93325189f481d9b37c3c6eae..763c825d53aaba4f874361b785587b2c5129d49a 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -135,11 +135,9 @@ pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
>      ///
>      /// `ptr` must be a valid pointer to a `struct device` embedded in `Self`.
>      unsafe fn from_drm_device(ptr: *const bindings::drm_device) -> *mut Self {
> -        let ptr: *const Opaque<bindings::drm_device> = ptr.cast();
> -
>          // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a
>          // `struct drm_device` embedded in `Self`.
> -        unsafe { crate::container_of!(ptr, Self, dev) }.cast_mut()
> +        unsafe { crate::container_of!(Opaque::from_raw(ptr), Self, dev) }.cast_mut()
>      }
>
>      /// Not intended to be called externally, except via declare_drm_ioctls!()
> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index 4cd69fa84318c3ff2cec57949e9bab05559a3c2f..5b80c248761bb39914a63ad7947aa8d3779054ef 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -125,11 +125,9 @@ fn as_raw(&self) -> *mut bindings::drm_gem_object {
>      }
>
>      unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self {
> -        let self_ptr: *mut Opaque<bindings::drm_gem_object> = self_ptr.cast();
> -
>          // SAFETY: `obj` is guaranteed to be in an `Object<T>` via the safety contract of this
>          // function
> -        unsafe { &*crate::container_of!(self_ptr, Object<T>, obj) }
> +        unsafe { &*crate::container_of!(Opaque::from_raw(self_ptr), Object<T>, obj) }
>      }
>  }
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 6b4774b2b1c37f4da1866e993be6230bc6715841..d2402d42b8776c9399a7dfdbe7bd61de7ef8dba3 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -204,6 +204,13 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
>
>  /// Produces a pointer to an object from a pointer to one of its fields.
>  ///
> +/// If you encounter a type mismatch due to the [`Opaque`] type, then use [`Opaque::raw_get`] or
> +/// [`Opaque::from_raw`] to resolve the mismatch.
> +///
> +/// [`Opaque`]: crate::types::Opaque
> +/// [`Opaque::raw_get`]: crate::types::Opaque::raw_get
> +/// [`Opaque::from_raw`]: crate::types::Opaque::from_raw
> +///
>  /// # Safety
>  ///
>  /// The pointer passed to this macro, and the pointer returned by this macro, must both be in
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 22985b6f69820d6df8ff3aae0bf815fad36a9d92..a79295500b3c812326cea8a9d339a8545a7f457d 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -413,6 +413,11 @@ pub const fn get(&self) -> *mut T {
>      pub const fn raw_get(this: *const Self) -> *mut T {
>          UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()
>      }
> +
> +    /// The opposite operation of [`Opaque::raw_get`].
> +    pub const fn from_raw(this: *const T) -> *const Self {
> +        this.cast()
> +    }
>  }
>
>  /// Types that are _always_ reference counted.
>
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250617-opaque-from-raw-ac5b8ef6faa2
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@...gle.com>