* [PATCH v4 01/16] rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
@ 2026-06-20 18:47 ` Danilo Krummrich
2026-06-20 18:57 ` sashiko-bot
2026-06-20 18:47 ` [PATCH v4 02/16] rust: drm: rename Uninit DeviceContext to Normal Danilo Krummrich
` (14 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:47 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux,
stable, sashiko-bot
References to dev, data, and file in the declare_drm_ioctls! macro are
created via unsafe pointer dereferences, producing unbounded lifetimes.
If an ioctl handler explicitly annotates its parameters with 'static,
the compiler accepts this, allowing the handler to stash references that
outlive the ioctl call.
Fix this by routing all references through a helper function whose
lifetime parameter 'a is tied to a local anchor variable. Since 'a is
bounded by the anchor's stack lifetime, handlers can no longer demand
'static on any parameter.
Cc: stable@vger.kernel.org
Fixes: 9a69570682b1 ("rust: drm: ioctl: Add DRM ioctl abstraction")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260620011346.A47D01F000E9@smtp.kernel.org/
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/ioctl.rs | 59 +++++++++++++++++++++++++++++++---------
1 file changed, 46 insertions(+), 13 deletions(-)
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index cf328101dde4..023e6da5c1e4 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -70,6 +70,39 @@ pub mod internal {
pub use bindings::drm_device;
pub use bindings::drm_file;
pub use bindings::drm_ioctl_desc;
+
+ /// Call an ioctl handler with lifetime-bounded references.
+ ///
+ /// The lifetime `'a` is tied to the `_anchor` parameter. This prevents handlers from
+ /// declaring `'static` on `dev`, `data`, or `file`.
+ ///
+ /// # Safety
+ ///
+ /// - `raw_data` must point to a valid, exclusively-owned instance of `Data` for the duration
+ /// of the call.
+ /// - `raw_file` must be a valid pointer to a `struct drm_file`.
+ #[doc(hidden)]
+ #[inline(always)]
+ pub unsafe fn __call_ioctl<
+ 'a,
+ Dev: 'a,
+ Data: 'a,
+ F: super::super::file::DriverFile + 'a,
+ Ret,
+ >(
+ _anchor: &'a (),
+ dev: &'a Dev,
+ raw_data: *mut ::core::ffi::c_void,
+ raw_file: *mut drm_file,
+ f: impl FnOnce(&'a Dev, &'a mut Data, &'a super::super::File<F>) -> Ret,
+ ) -> Ret {
+ // SAFETY: Caller guarantees raw_data points to a valid instance of Data with the correct
+ // size and alignment, exclusively owned for the duration of the ioctl call.
+ let data = unsafe { &mut *(raw_data.cast::<Data>()) };
+ // SAFETY: Caller guarantees raw_file is a valid pointer to a `struct drm_file`.
+ let file = unsafe { super::super::File::<F>::from_raw(raw_file) };
+ f(dev, data, file)
+ }
}
/// Declare the DRM ioctls for a driver.
@@ -135,19 +168,19 @@ macro_rules! declare_drm_ioctls {
// dev/file match the current driver these ioctls are being declared
// for, and it's not clear how to enforce this within the type system.
let dev = $crate::drm::device::Device::from_raw(raw_dev);
- // SAFETY: The ioctl argument has size `_IOC_SIZE(cmd)`, which we
- // asserted above matches the size of this type, and all bit patterns of
- // UAPI structs must be valid.
- // The `ioctl` argument is exclusively owned by the handler
- // and guaranteed by the C implementation (`drm_ioctl()`) to remain
- // valid for the entire lifetime of the reference taken here.
- // There is no concurrent access or aliasing; no other references
- // to this object exist during this call.
- let data = unsafe { &mut *(raw_data.cast::<$crate::uapi::$struct>()) };
- // SAFETY: This is just the DRM file structure
- let file = unsafe { $crate::drm::File::from_raw(raw_file) };
-
- match $func(dev, data, file) {
+ let __anchor = ();
+
+ // SAFETY:
+ // - The ioctl argument has size `_IOC_SIZE(cmd)`, which we asserted
+ // above matches the size of this type, and all bit patterns of UAPI
+ // structs must be valid. The argument is exclusively owned by this
+ // handler, guaranteed by `drm_ioctl()` to remain valid for the
+ // duration of the call.
+ // - `raw_file` is a valid `struct drm_file` pointer provided by the
+ // DRM core.
+ match unsafe { $crate::drm::ioctl::internal::__call_ioctl(
+ &__anchor, dev, raw_data, raw_file, $func,
+ ) } {
Err(e) => e.to_errno(),
Ok(i) => i.try_into()
.unwrap_or($crate::error::code::ERANGE.to_errno()),
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 01/16] rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments
2026-06-20 18:47 ` [PATCH v4 01/16] rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments Danilo Krummrich
@ 2026-06-20 18:57 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 18:57 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: Alexandre Courbot, dri-devel, ojeda
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch routes the ioctl data buffer to `__call_ioctl` as a generic void pointer, inferring the type of `Data` purely from the user-provided handler closure rather than enforcing it matches the UAPI struct. This removes compile-time type safety and allows an incorrect handler signature to cause a silent heap out-of-bounds access.
--
commit cb52c14a04f657a72b41bd28dae4125aa6d86907
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments
This patch fixes unbounded lifetimes in declare_drm_ioctls! by routing
references through a helper function __call_ioctl tied to a local anchor
variable.
Closes: https://lore.kernel.org/all/20260620011346.A47D01F000E9@smtp.kernel.org/
> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index cf328101dde4e..023e6da5c1e4b 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs
> @@ -70,6 +70,39 @@ pub mod internal {
[ ... ]
> + pub unsafe fn __call_ioctl<
> + 'a,
> + Dev: 'a,
> + Data: 'a,
> + F: super::super::file::DriverFile + 'a,
> + Ret,
> + >(
> + _anchor: &'a (),
> + dev: &'a Dev,
> + raw_data: *mut ::core::ffi::c_void,
> + raw_file: *mut drm_file,
> + f: impl FnOnce(&'a Dev, &'a mut Data, &'a super::super::File<F>) -> Ret,
> + ) -> Ret {
> + // SAFETY: Caller guarantees raw_data points to a valid instance of Data with the correct
> + // size and alignment, exclusively owned for the duration of the ioctl call.
> + let data = unsafe { &mut *(raw_data.cast::<Data>()) };
[Severity: High]
Does this type inference break the macro's memory safety guarantees?
Because Data is a generic type parameter and isn't explicitly specified, the
Rust compiler will infer it from the signature of the handler function. If a
driver author mistakenly writes an ioctl handler that takes a differently
sized or larger UAPI struct, Data will be inferred as that larger type.
Since raw_data points to a kernel buffer of size _IOC_SIZE(cmd) matching the
original macro struct, dereferencing it as a larger type would create a
reference that extends past the end of the allocated memory, leading to a
silent heap out-of-bounds access.
> + // SAFETY: Caller guarantees raw_file is a valid pointer to a `struct drm_file`.
> + let file = unsafe { super::super::File::<F>::from_raw(raw_file) };
> + f(dev, data, file)
> + }
> }
>
> /// Declare the DRM ioctls for a driver.
[ ... ]
> @@ -135,19 +168,19 @@ macro_rules! declare_drm_ioctls {
> - let data = unsafe { &mut *(raw_data.cast::<$crate::uapi::$struct>()) };
> - // SAFETY: This is just the DRM file structure
> - let file = unsafe { $crate::drm::File::from_raw(raw_file) };
> -
> - match $func(dev, data, file) {
> + let __anchor = ();
> +
> + // SAFETY:
> + // - The ioctl argument has size `_IOC_SIZE(cmd)`, which we asserted
> + // above matches the size of this type, and all bit patterns of UAPI
> + // structs must be valid. The argument is exclusively owned by this
> + // handler, guaranteed by `drm_ioctl()` to remain valid for the
> + // duration of the call.
> + // - `raw_file` is a valid `struct drm_file` pointer provided by the
> + // DRM core.
> + match unsafe { $crate::drm::ioctl::internal::__call_ioctl(
> + &__anchor, dev, raw_data, raw_file, $func,
> + ) } {
[Severity: High]
Is there a way to enforce the UAPI $struct type here?
Prior to this patch, declare_drm_ioctls! explicitly cast raw_data to the
declared UAPI struct, which forced a compile-time error if the user-provided
handler expected a different type.
By passing the handler function directly to __call_ioctl without constraining
the Data type to $crate::uapi::$struct, the compile-time type safety checking
appears to be bypassed, allowing the inference issue described above.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 02/16] rust: drm: rename Uninit DeviceContext to Normal
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
2026-06-20 18:47 ` [PATCH v4 01/16] rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments Danilo Krummrich
@ 2026-06-20 18:47 ` Danilo Krummrich
2026-06-20 18:47 ` [PATCH v4 03/16] rust: drm: Add Driver::ParentDevice associated type Danilo Krummrich
` (13 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:47 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Rename the Uninit DeviceContext to Normal to better reflect its purpose
as the general-purpose, reference-counted device context. The Uninit
name was a leftover from when DRM device private data initialization was
planned to split across UnregisteredDevice::new() and
Registration::new(); with the subsequent introduction of
RegistrationData, this distinction is no longer needed.
This also simplifies the DeviceContext documentation, trimming the
multi-stage initialization description that no longer applies.
Subsequent patches will refine the semantics of the Registered context
accordingly.
No functional change.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 92 ++++++++++++---------------------------
rust/kernel/drm/mod.rs | 2 +-
2 files changed, 28 insertions(+), 66 deletions(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 477cf771fb10..5e91474e6dbb 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -74,36 +74,22 @@ macro_rules! drm_legacy_fields {
/// A trait implemented by all possible contexts a [`Device`] can be used in.
///
-/// Setting up a new [`Device`] is a multi-stage process. Each step of the process that a user
-/// interacts with in Rust has a respective [`DeviceContext`] typestate. For example,
-/// `Device<T, Registered>` would be a [`Device`] that reached the [`Registered`] [`DeviceContext`].
+/// A [`Device`] can be in one of two contexts:
///
-/// Each stage of this process is described below:
-///
-/// ```text
-/// 1 2 3
-/// +--------------+ +------------------+ +-----------------------+
-/// |Device created| → |Device initialized| → |Registered w/ userspace|
-/// +--------------+ +------------------+ +-----------------------+
-/// (Uninit) (Registered)
-/// ```
-///
-/// 1. The [`Device`] is in the [`Uninit`] context and is not guaranteed to be initialized or
-/// registered with userspace. Only a limited subset of DRM core functionality is available.
-/// 2. The [`Device`] is guaranteed to be fully initialized, but is not guaranteed to be registered
-/// with userspace. All DRM core functionality which doesn't interact with userspace is
-/// available. We currently don't have a context for representing this.
-/// 3. The [`Device`] is guaranteed to be fully initialized, and is guaranteed to have been
-/// registered with userspace at some point - thus putting it in the [`Registered`] context.
-///
-/// An important caveat of [`DeviceContext`] which must be kept in mind: when used as a typestate
-/// for a reference type, it can only guarantee that a [`Device`] reached a particular stage in the
-/// initialization process _at the time the reference was taken_. No guarantee is made in regards to
-/// what stage of the process the [`Device`] is currently in. This means for instance that a
-/// `&Device<T, Uninit>` may actually be registered with userspace, it just wasn't known to be
-/// registered at the time the reference was taken.
+/// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
+/// or may not be registered with userspace.
+/// - [`Registered`]: The device has been registered with userspace at some point.
pub trait DeviceContext: Sealed + Send + Sync {}
+/// The general-purpose, reference-counted [`DeviceContext`].
+///
+/// A [`Device`] in this context may or may not be registered with userspace. This context is used
+/// for reference-counted device handles and during device setup via [`UnregisteredDevice`].
+pub struct Normal;
+
+impl Sealed for Normal {}
+impl DeviceContext for Normal {}
+
/// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point.
///
/// This represents a [`Device`] which is guaranteed to have been registered with userspace at
@@ -121,20 +107,6 @@ pub trait DeviceContext: Sealed + Send + Sync {}
impl Sealed for Registered {}
impl DeviceContext for Registered {}
-/// The [`DeviceContext`] of a [`Device`] that may be unregistered and partly uninitialized.
-///
-/// A [`Device`] in this context is only guaranteed to be partly initialized, and may or may not
-/// be registered with userspace. Thus operations which depend on the [`Device`] being fully
-/// initialized, or which depend on the [`Device`] being registered with userspace are not
-/// available through this [`DeviceContext`].
-///
-/// A [`Device`] in this context can be used to create a
-/// [`Registration`](drm::driver::Registration).
-pub struct Uninit;
-
-impl Sealed for Uninit {}
-impl DeviceContext for Uninit {}
-
/// A [`Device`] which is known at compile-time to be unregistered with userspace.
///
/// This type allows performing operations which are only safe to do before userspace registration,
@@ -147,10 +119,10 @@ impl DeviceContext for Uninit {}
///
/// The device in `self.0` is guaranteed to be a newly created [`Device`] that has not yet been
/// registered with userspace until this type is dropped.
-pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe);
+pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Normal>>, NotThreadSafe);
impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
- type Target = Device<T, Uninit>;
+ type Target = Device<T, Normal>;
fn deref(&self) -> &Self::Target {
&self.0
@@ -178,15 +150,13 @@ const fn compute_features() -> u32 {
master_drop: None,
debugfs_init: None,
- // Ignore the Uninit DeviceContext below. It is only provided because it is required by the
- // compiler, and it is not actually used by these functions.
- gem_create_object: T::Object::<Uninit>::ALLOC_OPS.gem_create_object,
- prime_handle_to_fd: T::Object::<Uninit>::ALLOC_OPS.prime_handle_to_fd,
- prime_fd_to_handle: T::Object::<Uninit>::ALLOC_OPS.prime_fd_to_handle,
- gem_prime_import: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import,
- gem_prime_import_sg_table: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import_sg_table,
- dumb_create: T::Object::<Uninit>::ALLOC_OPS.dumb_create,
- dumb_map_offset: T::Object::<Uninit>::ALLOC_OPS.dumb_map_offset,
+ gem_create_object: T::Object::<Normal>::ALLOC_OPS.gem_create_object,
+ prime_handle_to_fd: T::Object::<Normal>::ALLOC_OPS.prime_handle_to_fd,
+ prime_fd_to_handle: T::Object::<Normal>::ALLOC_OPS.prime_fd_to_handle,
+ gem_prime_import: T::Object::<Normal>::ALLOC_OPS.gem_prime_import,
+ gem_prime_import_sg_table: T::Object::<Normal>::ALLOC_OPS.gem_prime_import_sg_table,
+ dumb_create: T::Object::<Normal>::ALLOC_OPS.dumb_create,
+ dumb_map_offset: T::Object::<Normal>::ALLOC_OPS.dumb_map_offset,
show_fdinfo: None,
fbdev_probe: None,
@@ -211,7 +181,7 @@ const fn compute_features() -> u32 {
pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<Self> {
// `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
// compatible `Layout`.
- let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Uninit>>());
+ let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Normal>>());
// Use a temporary vtable without a `release` callback until `data` is initialized, so
// init failure can release the DRM device without dropping uninitialized fields.
@@ -223,12 +193,12 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
// SAFETY:
// - `alloc_vtable` reference remains valid until no longer used,
// - `dev` is valid by its type invarants,
- let raw_drm: *mut Device<T, Uninit> = unsafe {
+ let raw_drm: *mut Device<T, Normal> = unsafe {
bindings::__drm_dev_alloc(
dev.as_raw(),
&alloc_vtable,
layout.size(),
- mem::offset_of!(Device<T, Uninit>, dev),
+ mem::offset_of!(Device<T, Normal>, dev),
)
}
.cast();
@@ -264,16 +234,8 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
/// A typed DRM device with a specific [`drm::Driver`] implementation and [`DeviceContext`].
///
-/// Since DRM devices can be used before being fully initialized and registered with userspace, `C`
-/// represents the furthest [`DeviceContext`] we can guarantee that this [`Device`] has reached.
-///
-/// Keep in mind: this means that an unregistered device can still have the registration state
-/// [`Registered`] as long as it was registered with userspace once in the past, and that the
-/// behavior of such a device is still well-defined. Additionally, a device with the registration
-/// state [`Uninit`] simply does not have a guaranteed registration state at compile time, and could
-/// be either registered or unregistered. Since there is no way to guarantee a long-lived reference
-/// to an unregistered device would remain unregistered, we do not provide a [`DeviceContext`] for
-/// this.
+/// A device in the [`Registered`] context is guaranteed to have been registered with userspace
+/// at some point. The [`Normal`] context is the general-purpose, reference-counted context.
///
/// # Invariants
///
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index a66e7166f66b..e5bfaf130342 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -11,8 +11,8 @@
pub use self::device::Device;
pub use self::device::DeviceContext;
+pub use self::device::Normal;
pub use self::device::Registered;
-pub use self::device::Uninit;
pub use self::device::UnregisteredDevice;
pub use self::driver::Driver;
pub use self::driver::DriverInfo;
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 03/16] rust: drm: Add Driver::ParentDevice associated type
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
2026-06-20 18:47 ` [PATCH v4 01/16] rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments Danilo Krummrich
2026-06-20 18:47 ` [PATCH v4 02/16] rust: drm: rename Uninit DeviceContext to Normal Danilo Krummrich
@ 2026-06-20 18:47 ` Danilo Krummrich
2026-06-20 18:47 ` [PATCH v4 04/16] rust: drm: change default DeviceContext to Normal Danilo Krummrich
` (12 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:47 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Add a ParentDevice associated type to the Driver trait, allowing each
DRM driver to declare its parent bus device type (e.g.
auxiliary::Device, platform::Device).
Change UnregisteredDevice::new() to take &T::ParentDevice<Bound>,
ensuring at the type level that the DRM device's parent matches the
declared bus device type.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 8 ++++++--
drivers/gpu/drm/tyr/driver.rs | 6 ++++--
rust/kernel/drm/device.rs | 7 +++++--
rust/kernel/drm/driver.rs | 3 +++
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 48933d86ddda..c5b0313006bd 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -2,7 +2,10 @@
use kernel::{
auxiliary,
- device::Core,
+ device::{
+ Core,
+ DeviceContext, //
+ },
drm::{
self,
gem,
@@ -62,7 +65,7 @@ fn probe<'bound>(
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
let data = try_pin_init!(NovaData { adev: adev.into() });
- let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
+ let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
Ok(Nova { drm: drm.into() })
@@ -74,6 +77,7 @@ impl drm::Driver for NovaDriver {
type Data = NovaData;
type File = File;
type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
+ type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
const INFO: drm::DriverInfo = INFO;
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index d063bc664cc1..338c25ccc151 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -7,7 +7,8 @@
},
device::{
Core,
- Device, //
+ Device,
+ DeviceContext, //
},
dma::{
Device as DmaDevice,
@@ -148,7 +149,7 @@ fn probe<'bound>(
gpu_info,
});
- let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev.as_ref(), data)?;
+ let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
let driver = TyrPlatformDriverData {
@@ -182,6 +183,7 @@ impl drm::Driver for TyrDrmDriver {
type Data = TyrDrmDeviceData;
type File = TyrDrmFileData;
type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
+ type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
const INFO: drm::DriverInfo = INFO;
const FEAT_RENDER: bool = true;
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 5e91474e6dbb..23eb4c0a65ef 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -178,7 +178,10 @@ const fn compute_features() -> u32 {
/// Create a new `UnregisteredDevice` for a `drm::Driver`.
///
/// This can be used to create a [`Registration`](kernel::drm::Registration).
- pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<Self> {
+ pub fn new(
+ dev: &T::ParentDevice<device::Bound>,
+ data: impl PinInit<T::Data, Error>,
+ ) -> Result<Self> {
// `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
// compatible `Layout`.
let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Normal>>());
@@ -195,7 +198,7 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
// - `dev` is valid by its type invarants,
let raw_drm: *mut Device<T, Normal> = unsafe {
bindings::__drm_dev_alloc(
- dev.as_raw(),
+ dev.as_ref().as_raw(),
&alloc_vtable,
layout.size(),
mem::offset_of!(Device<T, Normal>, dev),
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 25f7e233884d..802e7fc13e30 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -116,6 +116,9 @@ pub trait Driver {
/// The type used to represent a DRM File (client)
type File: drm::file::DriverFile;
+ /// The bus device type of the parent device that the DRM device is associated with.
+ type ParentDevice<Ctx: device::DeviceContext>: device::AsBusDevice<Ctx>;
+
/// Driver metadata
const INFO: DriverInfo;
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 04/16] rust: drm: change default DeviceContext to Normal
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (2 preceding siblings ...)
2026-06-20 18:47 ` [PATCH v4 03/16] rust: drm: Add Driver::ParentDevice associated type Danilo Krummrich
@ 2026-06-20 18:47 ` Danilo Krummrich
2026-06-20 18:47 ` [PATCH v4 05/16] rust: drm: restrict AlwaysRefCounted to Normal Device context Danilo Krummrich
` (11 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:47 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Change the default DeviceContext from Registered to Normal for
drm::Device, gem::Object, gem::shmem::Object and
gem::shmem::ObjectConfig.
Normal is the general-purpose, reference-counted context suitable for
most uses; Registered represents a device that was registered with
userspace and will become a non-owning context obtained through a
RegistrationGuard.
Update the create_handle/lookup_handle bounds from Object<Registered> to
Object<Normal> to match the new default context of GEM objects, and
update the driver device type aliases (NovaDevice, TyrDrmDevice) to
default to Normal.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 2 +-
drivers/gpu/drm/tyr/driver.rs | 2 +-
rust/kernel/drm/device.rs | 2 +-
rust/kernel/drm/gem/mod.rs | 7 ++++---
rust/kernel/drm/gem/shmem.rs | 6 +++---
5 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index c5b0313006bd..8ddb81fd0c87 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -26,7 +26,7 @@ pub(crate) struct Nova {
}
/// Convienence type alias for the DRM device type for this driver
-pub(crate) type NovaDevice<Ctx = drm::Registered> = drm::Device<NovaDriver, Ctx>;
+pub(crate) type NovaDevice<Ctx = drm::Normal> = drm::Device<NovaDriver, Ctx>;
#[pin_data]
pub(crate) struct NovaData {
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 338c25ccc151..180631daff02 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -47,7 +47,7 @@
pub(crate) struct TyrDrmDriver;
/// Convenience type alias for the DRM device type for this driver.
-pub(crate) type TyrDrmDevice<Ctx = drm::Registered> = drm::Device<TyrDrmDriver, Ctx>;
+pub(crate) type TyrDrmDevice<Ctx = drm::Normal> = drm::Device<TyrDrmDriver, Ctx>;
pub(crate) struct TyrPlatformDriver;
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 23eb4c0a65ef..d712387707d2 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -246,7 +246,7 @@ pub fn new(
/// * The data layout of `Self` remains the same across all implementations of `C`.
/// * Any invariants for `C` also apply.
#[repr(C)]
-pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
+pub struct Device<T: drm::Driver, C: DeviceContext = Normal> {
dev: Opaque<bindings::drm_device>,
data: T::Data,
_ctx: PhantomData<C>,
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index c8b66d816871..1023ddccd785 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -10,6 +10,7 @@
self,
device::{
DeviceContext,
+ Normal,
Registered, //
},
driver::{
@@ -183,7 +184,7 @@ fn size(&self) -> usize {
fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
where
Self: AllocImpl<Driver = D>,
- D: drm::Driver<Object<Registered> = Self, File = F>,
+ D: drm::Driver<Object<Normal> = Self, File = F>,
F: drm::file::DriverFile<Driver = D>,
{
let mut handle: u32 = 0;
@@ -198,7 +199,7 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
where
Self: AllocImpl<Driver = D>,
- D: drm::Driver<Object<Registered> = Self, File = F>,
+ D: drm::Driver<Object<Normal> = Self, File = F>,
F: drm::file::DriverFile<Driver = D>,
{
// SAFETY: The arguments are all valid per the type invariants.
@@ -254,7 +255,7 @@ impl<T: IntoGEMObject> BaseObjectPrivate for T {}
/// * Any type invariants of `Ctx` apply to the parent DRM device for this GEM object.
#[repr(C)]
#[pin_data]
-pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Registered> {
+pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Normal> {
obj: Opaque<bindings::drm_gem_object>,
#[pin]
data: T,
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index 34af402899a0..f47a90cdb95b 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -17,7 +17,7 @@
private::Sealed,
Device,
DeviceContext,
- Registered, //
+ Normal, //
},
error::to_result,
prelude::*,
@@ -43,7 +43,7 @@
/// This is used with [`Object::new()`] to control various properties that can only be set when
/// initially creating a shmem-backed GEM object.
#[derive(Default)]
-pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Registered> {
+pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Normal> {
/// Whether to set the write-combine map flag.
pub map_wc: bool,
@@ -62,7 +62,7 @@ pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Registered> {
/// - Any type invariants of `C` apply to the parent DRM device for this GEM object.
#[repr(C)]
#[pin_data]
-pub struct Object<T: DriverObject, C: DeviceContext = Registered> {
+pub struct Object<T: DriverObject, C: DeviceContext = Normal> {
#[pin]
obj: Opaque<bindings::drm_gem_shmem_object>,
/// Parent object that owns this object's DMA reservation object.
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 05/16] rust: drm: restrict AlwaysRefCounted to Normal Device context
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (3 preceding siblings ...)
2026-06-20 18:47 ` [PATCH v4 04/16] rust: drm: change default DeviceContext to Normal Danilo Krummrich
@ 2026-06-20 18:47 ` Danilo Krummrich
2026-06-20 19:03 ` sashiko-bot
2026-06-20 18:47 ` [PATCH v4 06/16] rust: drm: restrict AlwaysRefCounted to Normal GEM Object context Danilo Krummrich
` (10 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:47 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Restrict the AlwaysRefCounted implementation for drm::Device to the
Normal context. Registered devices represent a non-owning view of a
device within a RegistrationGuard scope and must not be independently
reference-counted.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index d712387707d2..9825d52832af 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -85,6 +85,9 @@ pub trait DeviceContext: Sealed + Send + Sync {}
///
/// A [`Device`] in this context may or may not be registered with userspace. This context is used
/// for reference-counted device handles and during device setup via [`UnregisteredDevice`].
+///
+/// [`AlwaysRefCounted`] is only implemented for `Device<T, Normal>`, making this the required
+/// context for [`ARef`]-based device handles.
pub struct Normal;
impl Sealed for Normal {}
@@ -327,7 +330,7 @@ fn deref(&self) -> &Self::Target {
// SAFETY: DRM device objects are always reference counted and the get/put functions
// satisfy the requirements.
-unsafe impl<T: drm::Driver, C: DeviceContext> AlwaysRefCounted for Device<T, C> {
+unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
unsafe { bindings::drm_dev_get(self.as_raw()) };
@@ -357,12 +360,10 @@ unsafe impl<T: drm::Driver, C: DeviceContext> Send for Device<T, C> {}
// by the synchronization in `struct drm_device`.
unsafe impl<T: drm::Driver, C: DeviceContext> Sync for Device<T, C> {}
-impl<T, C, const ID: u64> WorkItem<ID> for Device<T, C>
+impl<T: drm::Driver, const ID: u64> WorkItem<ID> for Device<T>
where
- T: drm::Driver,
T::Data: WorkItem<ID, Pointer = ARef<Self>>,
T::Data: HasWork<Self, ID>,
- C: DeviceContext,
{
type Pointer = ARef<Self>;
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 05/16] rust: drm: restrict AlwaysRefCounted to Normal Device context
2026-06-20 18:47 ` [PATCH v4 05/16] rust: drm: restrict AlwaysRefCounted to Normal Device context Danilo Krummrich
@ 2026-06-20 19:03 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 19:03 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: ojeda, dri-devel, Alexandre Courbot
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect use of `#[inline(always)]` on `__call_ioctl` without `build_assert!()`.
--
commit 681c752cb3d2e7c1fa7dd279a52a3695aceaee3b
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: restrict AlwaysRefCounted to Normal Device context
This commit restricts the AlwaysRefCounted implementation for drm::Device to
the Normal context. It clarifies that registered devices represent non-owning
views and must not be independently reference-counted. The commit also
refactors DRM IOCTL handling to use a lifetime-bounded closure approach.
[ ... ]
> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index cf328101dde4e..023e6da5c1e4b 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs
> @@ -70,6 +70,39 @@ pub mod internal {
> pub use bindings::drm_device;
> pub use bindings::drm_file;
> pub use bindings::drm_ioctl_desc;
> +
> + /// Call an ioctl handler with lifetime-bounded references.
> + ///
> + /// The lifetime `'a` is tied to the `_anchor` parameter. This prevents handlers from
> + /// declaring `'static` on `dev`, `data`, or `file`.
> + ///
> + /// # Safety
> + ///
> + /// - `raw_data` must point to a valid, exclusively-owned instance of `Data` for the duration
> + /// of the call.
> + /// - `raw_file` must be a valid pointer to a `struct drm_file`.
> + #[doc(hidden)]
> + #[inline(always)]
> + pub unsafe fn __call_ioctl<
[Severity: Low]
This isn't a bug, but is the #[inline(always)] attribute intended here?
According to the Rust subsystem guidelines, #[inline(always)] should only be
used for functions using build_assert!() that depend on function parameters.
For forwarding abstractions like __call_ioctl(), the standard #[inline]
attribute is preferred since it lacks parameter-dependent build-time
assertions.
> + 'a,
> + Dev: 'a,
> + Data: 'a,
> + F: super::super::file::DriverFile + 'a,
> + Ret,
> + >(
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=5
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 06/16] rust: drm: restrict AlwaysRefCounted to Normal GEM Object context
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (4 preceding siblings ...)
2026-06-20 18:47 ` [PATCH v4 05/16] rust: drm: restrict AlwaysRefCounted to Normal Device context Danilo Krummrich
@ 2026-06-20 18:47 ` Danilo Krummrich
2026-06-20 21:00 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 07/16] rust: drm: split Deref for Device context typestates Danilo Krummrich
` (9 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:47 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Restrict AlwaysRefCounted for gem::Object and gem::shmem::Object to the
Normal context, since only Normal objects should be independently
reference-counted.
To avoid cascading through IntoGEMObject (which had AlwaysRefCounted as
a supertrait), remove AlwaysRefCounted from IntoGEMObject's supertraits
and instead add it as an explicit bound on lookup_handle(), which is the
only BaseObject method that returns an ARef.
Since Object::new() and shmem::Object::new() return ARef<Self>, move
them to Normal-only impl blocks. Similarly, simplify ObjectConfig and
shmem's parent_resv_obj field to the Normal context.
Remove the DeviceContext generic from DriverObject::new() and
Driver::Object, since GEM objects can only be constructed in the Normal
context. Simplify DriverAllocImpl accordingly.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 2 +-
drivers/gpu/drm/nova/gem.rs | 18 +++----
drivers/gpu/drm/tyr/driver.rs | 2 +-
drivers/gpu/drm/tyr/gem.rs | 11 +---
rust/kernel/drm/device.rs | 14 ++---
rust/kernel/drm/driver.rs | 2 +-
rust/kernel/drm/gem/mod.rs | 97 ++++++++++++++++------------------
rust/kernel/drm/gem/shmem.rs | 75 +++++++++++++-------------
8 files changed, 103 insertions(+), 118 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 8ddb81fd0c87..e3c54303d70e 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -76,7 +76,7 @@ fn probe<'bound>(
impl drm::Driver for NovaDriver {
type Data = NovaData;
type File = File;
- type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
+ type Object = gem::Object<NovaObject>;
type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
const INFO: drm::DriverInfo = INFO;
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index 9d8ff7de2c0f..2b6fe9dc0bfa 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -2,7 +2,10 @@
use kernel::{
drm,
- drm::{gem, gem::BaseObject, DeviceContext},
+ drm::{
+ gem,
+ gem::BaseObject, //
+ },
page,
prelude::*,
sync::aref::ARef,
@@ -21,27 +24,20 @@ impl gem::DriverObject for NovaObject {
type Driver = NovaDriver;
type Args = ();
- fn new<Ctx: DeviceContext>(
- _dev: &NovaDevice<Ctx>,
- _size: usize,
- _args: Self::Args,
- ) -> impl PinInit<Self, Error> {
+ fn new(_dev: &NovaDevice, _size: usize, _args: Self::Args) -> impl PinInit<Self, Error> {
try_pin_init!(NovaObject {})
}
}
impl NovaObject {
/// Create a new DRM GEM object.
- pub(crate) fn new<Ctx: DeviceContext>(
- dev: &NovaDevice<Ctx>,
- size: usize,
- ) -> Result<ARef<gem::Object<Self, Ctx>>> {
+ pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> {
if size == 0 {
return Err(EINVAL);
}
let aligned_size = page::page_align(size).ok_or(EINVAL)?;
- gem::Object::<Self, Ctx>::new(dev, aligned_size, ())
+ gem::Object::<Self>::new(dev, aligned_size, ())
}
/// Look up a GEM object handle for a `File` and return an `ObjectRef` for it.
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 180631daff02..7f082de6d6dc 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -182,7 +182,7 @@ fn drop(self: Pin<&mut Self>) {}
impl drm::Driver for TyrDrmDriver {
type Data = TyrDrmDeviceData;
type File = TyrDrmFileData;
- type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
+ type Object = drm::gem::shmem::Object<BoData>;
type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
const INFO: drm::DriverInfo = INFO;
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index c6d4d6f9bae3..1640a161754b 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -5,10 +5,7 @@
//! DRM's GEM subsystem with shmem backing.
use kernel::{
- drm::{
- gem,
- DeviceContext, //
- },
+ drm::gem,
prelude::*, //
};
@@ -33,11 +30,7 @@ impl gem::DriverObject for BoData {
type Driver = TyrDrmDriver;
type Args = BoCreateArgs;
- fn new<Ctx: DeviceContext>(
- _dev: &TyrDrmDevice<Ctx>,
- _size: usize,
- args: BoCreateArgs,
- ) -> impl PinInit<Self, Error> {
+ fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Self, Error> {
try_pin_init!(Self { flags: args.flags })
}
}
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 9825d52832af..6f3af46ff647 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -153,13 +153,13 @@ const fn compute_features() -> u32 {
master_drop: None,
debugfs_init: None,
- gem_create_object: T::Object::<Normal>::ALLOC_OPS.gem_create_object,
- prime_handle_to_fd: T::Object::<Normal>::ALLOC_OPS.prime_handle_to_fd,
- prime_fd_to_handle: T::Object::<Normal>::ALLOC_OPS.prime_fd_to_handle,
- gem_prime_import: T::Object::<Normal>::ALLOC_OPS.gem_prime_import,
- gem_prime_import_sg_table: T::Object::<Normal>::ALLOC_OPS.gem_prime_import_sg_table,
- dumb_create: T::Object::<Normal>::ALLOC_OPS.dumb_create,
- dumb_map_offset: T::Object::<Normal>::ALLOC_OPS.dumb_map_offset,
+ gem_create_object: T::Object::ALLOC_OPS.gem_create_object,
+ prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd,
+ prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle,
+ gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import,
+ gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_sg_table,
+ dumb_create: T::Object::ALLOC_OPS.dumb_create,
+ dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset,
show_fdinfo: None,
fbdev_probe: None,
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 802e7fc13e30..5152a18a8312 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -111,7 +111,7 @@ pub trait Driver {
type Data: Sync + Send;
/// The type used to manage memory for this driver.
- type Object<Ctx: drm::DeviceContext>: AllocImpl;
+ type Object: AllocImpl;
/// The type used to represent a DRM File (client)
type File: drm::file::DriverFile;
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 1023ddccd785..d56cbe2663e2 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -10,8 +10,7 @@
self,
device::{
DeviceContext,
- Normal,
- Registered, //
+ Normal, //
},
driver::{
AllocImpl,
@@ -82,8 +81,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
/// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
///
/// [`Driver`]: drm::Driver
-pub type DriverAllocImpl<T, Ctx = Registered> =
- <<T as DriverObject>::Driver as drm::Driver>::Object<Ctx>;
+pub type DriverAllocImpl<T> = <<T as DriverObject>::Driver as drm::Driver>::Object;
/// GEM object functions, which must be implemented by drivers.
pub trait DriverObject: Sync + Send + Sized {
@@ -94,8 +92,8 @@ pub trait DriverObject: Sync + Send + Sized {
type Args;
/// Create a new driver data object for a GEM object of a given size.
- fn new<Ctx: DeviceContext>(
- dev: &drm::Device<Self::Driver, Ctx>,
+ fn new(
+ dev: &drm::Device<Self::Driver>,
size: usize,
args: Self::Args,
) -> impl PinInit<Self, Error>;
@@ -110,7 +108,7 @@ fn close(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) {}
}
/// Trait that represents a GEM object subtype
-pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted {
+pub trait IntoGEMObject: Sized + super::private::Sealed {
/// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as
/// this owning object is valid.
fn as_raw(&self) -> *mut bindings::drm_gem_object;
@@ -184,7 +182,7 @@ fn size(&self) -> usize {
fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
where
Self: AllocImpl<Driver = D>,
- D: drm::Driver<Object<Normal> = Self, File = F>,
+ D: drm::Driver<Object = Self, File = F>,
F: drm::file::DriverFile<Driver = D>,
{
let mut handle: u32 = 0;
@@ -198,8 +196,8 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
/// Looks up an object by its handle for a given `File`.
fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
where
- Self: AllocImpl<Driver = D>,
- D: drm::Driver<Object<Normal> = Self, File = F>,
+ Self: AllocImpl<Driver = D> + AlwaysRefCounted,
+ D: drm::Driver<Object = Self, File = F>,
F: drm::file::DriverFile<Driver = D>,
{
// SAFETY: The arguments are all valid per the type invariants.
@@ -281,12 +279,43 @@ impl<T: DriverObject, Ctx: DeviceContext> Object<T, Ctx> {
rss: None,
};
+ /// Returns the `Device` that owns this GEM object.
+ pub fn dev(&self) -> &drm::Device<T::Driver, Ctx> {
+ // SAFETY:
+ // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM
+ // object lives.
+ // - The device we used for creating the gem object is passed as &drm::Device<T::Driver> to
+ // Object::<T>::new(), so we know that `T::Driver` is the right generic parameter to use
+ // here.
+ // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we
+ // return.
+ unsafe { drm::Device::from_raw((*self.as_raw()).dev) }
+ }
+
+ fn as_raw(&self) -> *mut bindings::drm_gem_object {
+ self.obj.get()
+ }
+
+ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
+ let ptr: *mut Opaque<bindings::drm_gem_object> = obj.cast();
+
+ // SAFETY: All of our objects are of type `Object<T>`.
+ let this = unsafe { crate::container_of!(ptr, Self, obj) };
+
+ // SAFETY: The C code only ever calls this callback with a valid pointer to a `struct
+ // drm_gem_object`.
+ unsafe { bindings::drm_gem_object_release(obj) };
+
+ // SAFETY: All of our objects are allocated via `KBox`, and we're in the
+ // free callback which guarantees this object has zero remaining references,
+ // so we can drop it.
+ let _ = unsafe { KBox::from_raw(this) };
+ }
+}
+
+impl<T: DriverObject> Object<T> {
/// Create a new GEM object.
- pub fn new(
- dev: &drm::Device<T::Driver, Ctx>,
- size: usize,
- args: T::Args,
- ) -> Result<ARef<Self>> {
+ pub fn new(dev: &drm::Device<T::Driver>, size: usize, args: T::Args) -> Result<ARef<Self>> {
let obj: Pin<KBox<Self>> = KBox::pin_init(
try_pin_init!(Self {
obj: Opaque::new(bindings::drm_gem_object::default()),
@@ -322,46 +351,12 @@ pub fn new(
// SAFETY: We take over the initial reference count from `drm_gem_object_init()`.
Ok(unsafe { ARef::from_raw(ptr) })
}
-
- /// Returns the `Device` that owns this GEM object.
- pub fn dev(&self) -> &drm::Device<T::Driver, Ctx> {
- // SAFETY:
- // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM
- // object lives.
- // - The device we used for creating the gem object is passed as &drm::Device<T::Driver> to
- // Object::<T>::new(), so we know that `T::Driver` is the right generic parameter to use
- // here.
- // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we
- // return.
- unsafe { drm::Device::from_raw((*self.as_raw()).dev) }
- }
-
- fn as_raw(&self) -> *mut bindings::drm_gem_object {
- self.obj.get()
- }
-
- extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
- let ptr: *mut Opaque<bindings::drm_gem_object> = obj.cast();
-
- // SAFETY: All of our objects are of type `Object<T>`.
- let this = unsafe { crate::container_of!(ptr, Self, obj) };
-
- // SAFETY: The C code only ever calls this callback with a valid pointer to a `struct
- // drm_gem_object`.
- unsafe { bindings::drm_gem_object_release(obj) };
-
- // SAFETY: All of our objects are allocated via `KBox`, and we're in the
- // free callback which guarantees this object has zero remaining references,
- // so we can drop it.
- let _ = unsafe { KBox::from_raw(this) };
- }
}
impl_aref_for_gem_obj! {
- impl<T, C> for Object<T, C>
+ impl<T> for Object<T>
where
- T: DriverObject,
- C: DeviceContext
+ T: DriverObject
}
impl<T: DriverObject, Ctx: DeviceContext> super::private::Sealed for Object<T, Ctx> {}
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index f47a90cdb95b..146e8cfc2cdf 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -43,14 +43,14 @@
/// This is used with [`Object::new()`] to control various properties that can only be set when
/// initially creating a shmem-backed GEM object.
#[derive(Default)]
-pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Normal> {
+pub struct ObjectConfig<'a, T: DriverObject> {
/// Whether to set the write-combine map flag.
pub map_wc: bool,
/// Reuse the DMA reservation from another GEM object.
///
/// The newly created [`Object`] will hold an owned refcount to `parent_resv_obj` if specified.
- pub parent_resv_obj: Option<&'a Object<T, C>>,
+ pub parent_resv_obj: Option<&'a Object<T>>,
}
/// A shmem-backed GEM object.
@@ -66,17 +66,16 @@ pub struct Object<T: DriverObject, C: DeviceContext = Normal> {
#[pin]
obj: Opaque<bindings::drm_gem_shmem_object>,
/// Parent object that owns this object's DMA reservation object.
- parent_resv_obj: Option<ARef<Object<T, C>>>,
+ parent_resv_obj: Option<ARef<Object<T>>>,
#[pin]
inner: T,
_ctx: PhantomData<C>,
}
super::impl_aref_for_gem_obj! {
- impl<T, C> for Object<T, C>
+ impl<T> for Object<T>
where
- T: DriverObject,
- C: DeviceContext
+ T: DriverObject
}
// SAFETY: All GEM objects are thread-safe.
@@ -112,13 +111,43 @@ fn as_raw_shmem(&self) -> *mut bindings::drm_gem_shmem_object {
self.obj.get()
}
+ /// Returns the `Device` that owns this GEM object.
+ pub fn dev(&self) -> &Device<T::Driver, C> {
+ // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`.
+ unsafe { Device::from_raw((*self.as_raw()).dev) }
+ }
+
+ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
+ // SAFETY:
+ // - DRM always passes a valid gem object here
+ // - We used drm_gem_shmem_create() in our create_gem_object callback, so we know that
+ // `obj` is contained within a drm_gem_shmem_object
+ let this = unsafe { container_of!(obj, bindings::drm_gem_shmem_object, base) };
+
+ // SAFETY:
+ // - We're in free_callback - so this function is safe to call.
+ // - We won't be using the gem resources on `this` after this call.
+ unsafe { bindings::drm_gem_shmem_release(this) };
+
+ // SAFETY:
+ // - We verified above that `obj` is valid, which makes `this` valid
+ // - This function is set in AllocOps, so we know that `this` is contained within a
+ // `Object<T, C>`
+ let this = unsafe { container_of!(Opaque::cast_from(this), Self, obj) }.cast_mut();
+
+ // SAFETY: We're recovering the Kbox<> we created in gem_create_object()
+ let _ = unsafe { KBox::from_raw(this) };
+ }
+}
+
+impl<T: DriverObject> Object<T> {
/// Create a new shmem-backed DRM object of the given size.
///
/// Additional config options can be specified using `config`.
pub fn new(
- dev: &Device<T::Driver, C>,
+ dev: &Device<T::Driver>,
size: usize,
- config: ObjectConfig<'_, T, C>,
+ config: ObjectConfig<'_, T>,
args: T::Args,
) -> Result<ARef<Self>> {
let new: Pin<KBox<Self>> = KBox::try_pin_init(
@@ -126,7 +155,7 @@ pub fn new(
obj <- Opaque::init_zeroed(),
parent_resv_obj: config.parent_resv_obj.map(|p| p.into()),
inner <- T::new(dev, size, args),
- _ctx: PhantomData::<C>,
+ _ctx: PhantomData,
}),
GFP_KERNEL,
)?;
@@ -157,34 +186,6 @@ pub fn new(
Ok(obj)
}
-
- /// Returns the `Device` that owns this GEM object.
- pub fn dev(&self) -> &Device<T::Driver, C> {
- // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`.
- unsafe { Device::from_raw((*self.as_raw()).dev) }
- }
-
- extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
- // SAFETY:
- // - DRM always passes a valid gem object here
- // - We used drm_gem_shmem_create() in our create_gem_object callback, so we know that
- // `obj` is contained within a drm_gem_shmem_object
- let this = unsafe { container_of!(obj, bindings::drm_gem_shmem_object, base) };
-
- // SAFETY:
- // - We're in free_callback - so this function is safe to call.
- // - We won't be using the gem resources on `this` after this call.
- unsafe { bindings::drm_gem_shmem_release(this) };
-
- // SAFETY:
- // - We verified above that `obj` is valid, which makes `this` valid
- // - This function is set in AllocOps, so we know that `this` is contained within a
- // `Object<T, C>`
- let this = unsafe { container_of!(Opaque::cast_from(this), Self, obj) }.cast_mut();
-
- // SAFETY: We're recovering the Kbox<> we created in gem_create_object()
- let _ = unsafe { KBox::from_raw(this) };
- }
}
impl<T: DriverObject, C: DeviceContext> Deref for Object<T, C> {
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 06/16] rust: drm: restrict AlwaysRefCounted to Normal GEM Object context
2026-06-20 18:47 ` [PATCH v4 06/16] rust: drm: restrict AlwaysRefCounted to Normal GEM Object context Danilo Krummrich
@ 2026-06-20 21:00 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 21:00 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: ojeda, dri-devel, Alexandre Courbot
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing `gem_create_object` callback in `shmem::Object` causes the DRM core to allocate bare C structs, leading to out-of-bounds reads/writes when Rust drivers access them via `lookup_handle`.
--
commit 21c57b4cda527522a0ac16331bcf2d0d3de21563
Author: Danilo Krummrich <dakr@kernel.org>
Subject: rust: drm: restrict AlwaysRefCounted to Normal GEM Object context
Restrict AlwaysRefCounted for gem::Object and gem::shmem::Object to the Normal
context. This simplifies generic parameters, removes AlwaysRefCounted from
IntoGEMObject's supertraits, and adds it as an explicit bound on
lookup_handle().
> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index 1023ddccd7857..d56cbe2663e29 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
[ ... ]
> @@ -198,8 +196,8 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
> /// Looks up an object by its handle for a given `File`.
> fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
> where
> - Self: AllocImpl<Driver = D>,
> - D: drm::Driver<Object<Normal> = Self, File = F>,
> + Self: AllocImpl<Driver = D> + AlwaysRefCounted,
> + D: drm::Driver<Object = Self, File = F>,
> F: drm::file::DriverFile<Driver = D>,
> {
[Severity: Critical]
This is a pre-existing issue, but since we are modifying the bounds on
lookup_handle(), could the current implementation of shmem::Object lead to
out-of-bounds heap accesses?
In rust/kernel/drm/gem/shmem.rs, ALLOC_OPS sets gem_create_object to None
while enabling dumb_create and gem_prime_import_sg_table:
rust/kernel/drm/gem/shmem.rs:ALLOC_OPS {
gem_create_object: None,
...
gem_prime_import_sg_table: Some(bindings::drm_gem_shmem_prime_import_sg_table),
dumb_create: Some(bindings::drm_gem_shmem_dumb_create),
}
When userspace creates a dumb buffer, __drm_gem_shmem_create() sees a NULL
gem_create_object and falls back to a pure C allocation using
kzalloc(sizeof(*shmem), GFP_KERNEL). This allocation lacks the trailing
Rust wrapper fields (inner and parent_resv_obj).
If a Rust driver later retrieves this handle via Object::lookup_handle(),
it casts the C-allocated struct back to the larger Rust wrapper:
rust/kernel/drm/gem/mod.rs:lookup_handle() {
let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) };
if ptr.is_null() {
return Err(ENOENT);
}
let obj = unsafe { Self::from_raw(ptr) };
...
}
Does this cause any subsequent access to obj.inner or obj.parent_resv_obj
by the Rust driver to read or write out of bounds on the kernel heap?
Should shmem::Object provide a gem_create_object callback to ensure the
full Rust struct is always allocated?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=6
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 07/16] rust: drm: split Deref for Device context typestates
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (5 preceding siblings ...)
2026-06-20 18:47 ` [PATCH v4 06/16] rust: drm: restrict AlwaysRefCounted to Normal GEM Object context Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 18:48 ` [PATCH v4 08/16] rust: drm: pin ioctl Device reference to Normal context Danilo Krummrich
` (8 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Split the Deref implementation for drm::Device by context:
- Device<T> (Normal) dereferences to T::Data.
- Device<T, Registered> dereferences to Device<T> (Normal).
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 6f3af46ff647..86a7fca1d33f 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -79,6 +79,9 @@ macro_rules! drm_legacy_fields {
/// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
/// or may not be registered with userspace.
/// - [`Registered`]: The device has been registered with userspace at some point.
+///
+/// `Device<T, Registered>` dereferences to `Device<T>` ([`Normal`]), so any method available on a
+/// [`Normal`] device is also available on a [`Registered`] one.
pub trait DeviceContext: Sealed + Send + Sync {}
/// The general-purpose, reference-counted [`DeviceContext`].
@@ -320,7 +323,7 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
}
}
-impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> {
+impl<T: drm::Driver> Deref for Device<T> {
type Target = T::Data;
fn deref(&self) -> &Self::Target {
@@ -328,6 +331,17 @@ fn deref(&self) -> &Self::Target {
}
}
+impl<T: drm::Driver> Deref for Device<T, Registered> {
+ type Target = Device<T>;
+
+ #[inline]
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: The caller holds a `Device<T, Registered>`, which guarantees all invariants
+ // of the weaker `Normal` context.
+ unsafe { self.assume_ctx() }
+ }
+}
+
// SAFETY: DRM device objects are always reference counted and the get/put functions
// satisfy the requirements.
unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 08/16] rust: drm: pin ioctl Device reference to Normal context
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (6 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 07/16] rust: drm: split Deref for Device context typestates Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 18:59 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 09/16] rust: drm: add Ioctl device context typestate Danilo Krummrich
` (7 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Explicitly annotate the Device reference produced by from_raw() in the
ioctl dispatch macro as Device<_, Normal>. Without this annotation, the
context is inferred from the handler's first parameter type, which would
allow a handler declaring &Device<T, Registered> to obtain a Registered
reference without runtime proof via RegistrationGuard.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/ioctl.rs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 023e6da5c1e4..70cf1aa4d788 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -167,7 +167,8 @@ macro_rules! declare_drm_ioctls {
// FIXME: Currently there is nothing enforcing that the types of the
// dev/file match the current driver these ioctls are being declared
// for, and it's not clear how to enforce this within the type system.
- let dev = $crate::drm::device::Device::from_raw(raw_dev);
+ let dev: &$crate::drm::device::Device<_, $crate::drm::Normal> =
+ $crate::drm::device::Device::from_raw(raw_dev);
let __anchor = ();
// SAFETY:
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 08/16] rust: drm: pin ioctl Device reference to Normal context
2026-06-20 18:48 ` [PATCH v4 08/16] rust: drm: pin ioctl Device reference to Normal context Danilo Krummrich
@ 2026-06-20 18:59 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 18:59 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: dri-devel, ojeda, Alexandre Courbot
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-Bounds Memory Access via Type Inference in `declare_drm_ioctls!`
- [High] Type Confusion via Unconstrained Device and File Generic Parameters
--
commit 0938877e05968d385af3ba72b727ba8164977c11
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: pin ioctl Device reference to Normal context
Explicitly annotate the Device reference produced by from_raw() in the
ioctl dispatch macro as Device<_, Normal>. Without this annotation, the
context is inferred from the handler's first parameter type, which would
allow a handler declaring &Device<T, Registered> to obtain a Registered
reference without runtime proof via RegistrationGuard.
> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index 023e6da5c1e4b..70cf1aa4d788f 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs
> @@ -167,7 +167,8 @@ macro_rules! declare_drm_ioctls {
> // FIXME: Currently there is nothing enforcing that the types of the
> // dev/file match the current driver these ioctls are being declared
> // for, and it's not clear how to enforce this within the type system.
> - let dev = $crate::drm::device::Device::from_raw(raw_dev);
> + let dev: &$crate::drm::device::Device<_, $crate::drm::Normal> =
> + $crate::drm::device::Device::from_raw(raw_dev);
[Severity: High]
This isn't a bug introduced by this patch, but looking at the FIXME comment
and the use of the generic wildcard for the driver type in
Device<_, Normal>, could this lead to type confusion?
If a developer provides a handler expecting a Device for DriverB but
registers it in DriverA's ioctl array, it seems Device::from_raw() will cast
the C pointers for DriverA into references for DriverB. Any subsequent access
to the device or file private data would then cause memory corruption.
Also, for the File type:
rust/kernel/drm/ioctl.rs:internal::__call_ioctl() {
...
let file = unsafe { super::super::File::<F>::from_raw(raw_file) };
...
}
Is there a way to enforce that the Device and File generic parameters of the
user-provided ioctl handler match the actual driver registering the ioctls?
> let __anchor = ();
>
> // SAFETY:
[Severity: High]
This is a pre-existing issue, but does declare_drm_ioctls! constrain the
inferred Data type for the ioctl payload?
Looking at __call_ioctl(), the Data generic type is inferred from the second
parameter of the user-provided handler:
rust/kernel/drm/ioctl.rs:internal::__call_ioctl() {
...
let data = unsafe { &mut *(raw_data.cast::<Data>()) };
...
}
If a handler expects a mutable reference to a data type larger than the
declared struct (for example, a very large array), __call_ioctl() casts
raw_data to that larger type. Since the DRM core allocates exactly
_IOC_SIZE(cmd) bytes based on the macro's struct, could this result in an
out-of-bounds heap read/write?
Could the type-inference anchor be updated to explicitly constrain the data
parameter, perhaps using let mut data: $crate::uapi::$struct = unreachable!();
instead of unreachable!()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=8
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 09/16] rust: drm: add Ioctl device context typestate
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (7 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 08/16] rust: drm: pin ioctl Device reference to Normal context Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 18:48 ` [PATCH v4 10/16] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections Danilo Krummrich
` (6 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Add the Ioctl DeviceContext for DRM devices that have been registered
with userspace previously.
A Device<T, Ioctl> has been registered at some point, but may be
concurrently unregistering or already unregistered. drm_dev_enter() can
guard against this, ensuring the device remains registered for the
duration of the critical section.
This typestate will be used in ioctl dispatch context where registration
is guaranteed by the DRM core, and RegistrationGuard can safely be
acquired.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 34 +++++++++++++++++++++++++++++++---
rust/kernel/drm/mod.rs | 1 +
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 86a7fca1d33f..d429b4655449 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -74,14 +74,16 @@ macro_rules! drm_legacy_fields {
/// A trait implemented by all possible contexts a [`Device`] can be used in.
///
-/// A [`Device`] can be in one of two contexts:
+/// A [`Device`] can be in one of the following contexts:
///
/// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
/// or may not be registered with userspace.
+/// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl
+/// dispatch context.
/// - [`Registered`]: The device has been registered with userspace at some point.
///
-/// `Device<T, Registered>` dereferences to `Device<T>` ([`Normal`]), so any method available on a
-/// [`Normal`] device is also available on a [`Registered`] one.
+/// Both `Device<T, Ioctl>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
+/// so any method available on a [`Normal`] device is also available in the other contexts.
pub trait DeviceContext: Sealed + Send + Sync {}
/// The general-purpose, reference-counted [`DeviceContext`].
@@ -113,6 +115,21 @@ impl DeviceContext for Normal {}
impl Sealed for Registered {}
impl DeviceContext for Registered {}
+/// The [`DeviceContext`] of a [`Device`] that has been registered with userspace previously.
+///
+/// A [`Device`] in this context has been registered at some point, but may be concurrently
+/// unregistering or already unregistered. `drm_dev_enter()` can guard against this, ensuring the
+/// device remains registered for the duration of the critical section.
+///
+/// # Invariants
+///
+/// A [`Device`] in this context has been registered with userspace via `drm_dev_register()` at
+/// some point.
+pub struct Ioctl;
+
+impl Sealed for Ioctl {}
+impl DeviceContext for Ioctl {}
+
/// A [`Device`] which is known at compile-time to be unregistered with userspace.
///
/// This type allows performing operations which are only safe to do before userspace registration,
@@ -342,6 +359,17 @@ fn deref(&self) -> &Self::Target {
}
}
+impl<T: drm::Driver> Deref for Device<T, Ioctl> {
+ type Target = Device<T>;
+
+ #[inline]
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: The caller holds a `Device<T, Ioctl>`, which guarantees all invariants
+ // of the weaker `Normal` context.
+ unsafe { self.assume_ctx() }
+ }
+}
+
// SAFETY: DRM device objects are always reference counted and the get/put functions
// satisfy the requirements.
unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index e5bfaf130342..a6693d2b84b8 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -11,6 +11,7 @@
pub use self::device::Device;
pub use self::device::DeviceContext;
+pub use self::device::Ioctl;
pub use self::device::Normal;
pub use self::device::Registered;
pub use self::device::UnregisteredDevice;
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 10/16] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (8 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 09/16] rust: drm: add Ioctl device context typestate Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 19:03 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 11/16] rust: drm: Wrap ioctl dispatch in RegistrationGuard Danilo Krummrich
` (5 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
DRM ioctls do not guarantee that the parent bus device is still bound.
However, since DRM device registration is managed through Devres, using
drm_dev_unplug() on unregistration ensures that between drm_dev_enter()
and drm_dev_exit() the parent device must be bound.
Add RegistrationGuard, a guard object representing a drm_dev_enter/exit
SRCU critical section that dereferences to &Device<T, Registered>. The
guard is obtained from Device<T, Ioctl> and proves at runtime that the
device is still registered.
Switch Registration::drop from drm_dev_unregister() to drm_dev_unplug()
to provide the SRCU barrier that RegistrationGuard's safety argument
relies on.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 85 ++++++++++++++++++++++++++++++++++-----
rust/kernel/drm/driver.rs | 10 ++++-
rust/kernel/drm/mod.rs | 1 +
3 files changed, 83 insertions(+), 13 deletions(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index d429b4655449..c32cc0f0eba0 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -80,7 +80,8 @@ macro_rules! drm_legacy_fields {
/// or may not be registered with userspace.
/// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl
/// dispatch context.
-/// - [`Registered`]: The device has been registered with userspace at some point.
+/// - [`Registered`]: The device is currently registered with userspace and the parent bus device
+/// is bound.
///
/// Both `Device<T, Ioctl>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
/// so any method available on a [`Normal`] device is also available in the other contexts.
@@ -98,18 +99,15 @@ pub trait DeviceContext: Sealed + Send + Sync {}
impl Sealed for Normal {}
impl DeviceContext for Normal {}
-/// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point.
+/// The [`DeviceContext`] of a [`Device`] that is currently registered with userspace.
///
-/// This represents a [`Device`] which is guaranteed to have been registered with userspace at
-/// some point in time. Such a DRM device is guaranteed to have been fully-initialized.
-///
-/// Note: A device in this context is not guaranteed to remain registered with userspace for its
-/// entire lifetime, as this is impossible to guarantee at compile-time.
+/// A [`Device`] in this context is guaranteed to be registered and its parent bus device is
+/// guaranteed to be bound. This is enforced at runtime by [`RegistrationGuard`], which holds a
+/// `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
///
/// # Invariants
///
-/// A [`Device`] in this [`DeviceContext`] is guaranteed to have been registered with userspace
-/// at some point in time.
+/// The parent bus device is bound for the duration of any reference to a `Device<T, Registered>`.
pub struct Registered;
impl Sealed for Registered {}
@@ -260,8 +258,8 @@ pub fn new(
/// A typed DRM device with a specific [`drm::Driver`] implementation and [`DeviceContext`].
///
-/// A device in the [`Registered`] context is guaranteed to have been registered with userspace
-/// at some point. The [`Normal`] context is the general-purpose, reference-counted context.
+/// A device in the [`Registered`] context is currently registered with userspace and its parent
+/// bus device is bound. The [`Normal`] context is the general-purpose, reference-counted context.
///
/// # Invariants
///
@@ -340,6 +338,71 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
}
}
+impl<T: drm::Driver> Device<T, Ioctl> {
+ /// Guard against the parent bus device being unbound.
+ ///
+ /// Returns a [`RegistrationGuard`] if the device has not been unplugged, [`None`] otherwise.
+ ///
+ /// While [`RegistrationGuard`] is held the parent device is guaranteed to be bound.
+ #[must_use]
+ pub fn registration_guard(&self) -> Option<RegistrationGuard<'_, T>> {
+ let mut idx: i32 = 0;
+ // SAFETY: `self.as_raw()` is a valid pointer to a `struct drm_device`.
+ if unsafe { bindings::drm_dev_enter(self.as_raw(), &mut idx) } {
+ // INVARIANT:
+ // - `idx` is the SRCU index from the successful `drm_dev_enter()` above.
+ // - The parent bus device is bound: `drm_dev_enter()` succeeded, meaning
+ // `drm_dev_unplug()` has not completed; since it is only called from
+ // `Registration::drop()` during parent unbind, the parent is still bound.
+ Some(RegistrationGuard {
+ // SAFETY: See INVARIANT above; the `Registered` context invariant holds.
+ dev: unsafe { self.assume_ctx() },
+ idx,
+ _not_send: NotThreadSafe,
+ })
+ } else {
+ None
+ }
+ }
+}
+
+/// A guard proving the DRM device is registered and the parent bus device is bound.
+///
+/// The guard dereferences to [`Device<T, Registered>`], providing access to the DRM device with
+/// the guarantee that the parent bus device is bound for the entire duration of the critical
+/// section.
+///
+/// Internally this is backed by a `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
+///
+/// # Invariants
+///
+/// - `idx` is the SRCU read lock index returned by a successful `drm_dev_enter()` call.
+/// - The parent bus device of `dev` is bound for the lifetime of this guard.
+#[must_use]
+pub struct RegistrationGuard<'a, T: drm::Driver> {
+ dev: &'a Device<T, Registered>,
+ idx: i32,
+ _not_send: NotThreadSafe,
+}
+
+impl<T: drm::Driver> Deref for RegistrationGuard<'_, T> {
+ type Target = Device<T, Registered>;
+
+ #[inline]
+ fn deref(&self) -> &Self::Target {
+ self.dev
+ }
+}
+
+impl<T: drm::Driver> Drop for RegistrationGuard<'_, T> {
+ #[inline]
+ fn drop(&mut self) {
+ // SAFETY: `self.idx` was returned by a successful `drm_dev_enter()` call, as guaranteed
+ // by the type invariants of `RegistrationGuard`.
+ unsafe { bindings::drm_dev_exit(self.idx) };
+ }
+}
+
impl<T: drm::Driver> Deref for Device<T> {
type Target = T::Data;
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 5152a18a8312..3cda8dceb498 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -199,8 +199,14 @@ unsafe impl<T: Driver> Send for Registration<T> {}
impl<T: Driver> Drop for Registration<T> {
fn drop(&mut self) {
+ // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
+ // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
+ // is required for the safety of `RegistrationGuard`, which relies on the SRCU barrier in
+ // `drm_dev_unplug()` to guarantee that the parent device is still bound within the
+ // critical section.
+ //
// SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
- // `Registration` also guarantees the this `drm::Device` is actually registered.
- unsafe { bindings::drm_dev_unregister(self.0.as_raw()) };
+ // `Registration` also guarantees that this `drm::Device` is actually registered.
+ unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
}
}
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index a6693d2b84b8..fd6ed35bc35a 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -14,6 +14,7 @@
pub use self::device::Ioctl;
pub use self::device::Normal;
pub use self::device::Registered;
+pub use self::device::RegistrationGuard;
pub use self::device::UnregisteredDevice;
pub use self::driver::Driver;
pub use self::driver::DriverInfo;
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 10/16] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections
2026-06-20 18:48 ` [PATCH v4 10/16] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections Danilo Krummrich
@ 2026-06-20 19:03 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 19:03 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: dri-devel, Alexandre Courbot, ojeda
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RegistrationGuard fails to protect driver unbind/teardown from concurrent IOCTLs because drm_dev_unplug() runs too late.
--
commit b8b80ca9bb704803d61dd7a9db3bfa5325217fcc
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections
This commit introduces RegistrationGuard, which uses drm_dev_enter() and
drm_dev_exit() to represent an SRCU critical section ensuring the parent bus
device remains bound. It updates Registration::drop() to use
drm_dev_unplug() instead of drm_dev_unregister() to provide the necessary
SRCU barrier.
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index d429b4655449f..c32cc0f0eba03 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
[ ... ]
> +/// A guard proving the DRM device is registered and the parent bus device is bound.
> +///
> +/// The guard dereferences to [`Device<T, Registered>`], providing access to the DRM device with
> +/// the guarantee that the parent bus device is bound for the entire duration of the critical
> +/// section.
> +///
> +/// Internally this is backed by a `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
> +///
> +/// # Invariants
> +///
> +/// - `idx` is the SRCU read lock index returned by a successful `drm_dev_enter()` call.
> +/// - The parent bus device of `dev` is bound for the lifetime of this guard.
> +#[must_use]
> +pub struct RegistrationGuard<'a, T: drm::Driver> {
[Severity: High]
Does this invariant hold true during the entire driver unbind lifecycle?
Since drm_dev_unplug() is what prevents new drm_dev_enter() critical
sections, the safety of this guard relies on exactly when drm_dev_unplug()
is called.
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index 5152a18a8312b..3cda8dceb498d 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
[ ... ]
> impl<T: Driver> Drop for Registration<T> {
> fn drop(&mut self) {
> + // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
> + // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
> + // is required for the safety of `RegistrationGuard`, which relies on the SRCU barrier in
> + // `drm_dev_unplug()` to guarantee that the parent device is still bound within the
> + // critical section.
> + //
> // SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
> - // `Registration` also guarantees the this `drm::Device` is actually registered.
> - unsafe { bindings::drm_dev_unregister(self.0.as_raw()) };
> + // `Registration` also guarantees that this `drm::Device` is actually registered.
> + unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
> }
> }
[Severity: High]
Does placing drm_dev_unplug() in the Drop implementation of Registration
leave a window where concurrent IOCTLs can access hardware during driver
unbind?
In the driver model, Registration is stored in the driver's private data,
which is dropped during devres teardown or via post_unbind_callback. This
happens strictly after the driver core invokes the driver's unbind() or
remove() callback.
Since the unbind() callback is the documented place for drivers to perform
graceful teardown and hardware I/O, does this mean drm_dev_enter() will
still return true during and immediately after unbind()?
If so, concurrent IOCTLs could successfully acquire RegistrationGuard and
attempt to access the hardware while it is actively being torn down, which
seems to violate the safety invariant and risk a hardware hang or
use-after-free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=10
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 11/16] rust: drm: Wrap ioctl dispatch in RegistrationGuard
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (9 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 10/16] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 18:58 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 12/16] rust: drm: return ParentDevice from Device AsRef Danilo Krummrich
` (4 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Ioctl handlers now receive a &Device<T, Registered> reference, proving
at the type level that the device is registered and its parent bus
device is bound.
This is achieved by calling registration_guard() on the Device<T, Ioctl>
obtained in ioctl dispatch context. If the device has been unplugged,
the ioctl returns -ENODEV without calling the handler.
To resolve the driver type parameter T for type inference, which the
compiler cannot propagate through method resolution and associated-type
projections alone, a dead-code closure and a helper function are used as
a type-inference anchor.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/file.rs | 12 ++++++----
drivers/gpu/drm/tyr/file.rs | 7 ++++--
rust/kernel/drm/ioctl.rs | 45 +++++++++++++++++++++++++++++++++---
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index a3b7bd36792c..19fb89b28984 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -4,7 +4,11 @@
use crate::gem::NovaObject;
use kernel::{
alloc::flags::*,
- drm::{self, gem::BaseObject},
+ drm::{
+ self,
+ gem::BaseObject,
+ Registered, //
+ },
pci,
prelude::*,
uapi,
@@ -23,7 +27,7 @@ fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
impl File {
/// IOCTL: get_param: Query GPU / driver metadata.
pub(crate) fn get_param(
- dev: &NovaDevice,
+ dev: &NovaDevice<Registered>,
getparam: &mut uapi::drm_nova_getparam,
_file: &drm::File<File>,
) -> Result<u32> {
@@ -43,7 +47,7 @@ pub(crate) fn get_param(
/// IOCTL: gem_create: Create a new DRM GEM object.
pub(crate) fn gem_create(
- dev: &NovaDevice,
+ dev: &NovaDevice<Registered>,
req: &mut uapi::drm_nova_gem_create,
file: &drm::File<File>,
) -> Result<u32> {
@@ -56,7 +60,7 @@ pub(crate) fn gem_create(
/// IOCTL: gem_info: Query GEM metadata.
pub(crate) fn gem_info(
- _dev: &NovaDevice,
+ _dev: &NovaDevice<Registered>,
req: &mut uapi::drm_nova_gem_info,
file: &drm::File<File>,
) -> Result<u32> {
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 31411da203c5..fb9233eae01c 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -1,7 +1,10 @@
// SPDX-License-Identifier: GPL-2.0 or MIT
use kernel::{
- drm,
+ drm::{
+ self,
+ Registered, //
+ },
prelude::*,
uaccess::UserSlice,
uapi, //
@@ -28,7 +31,7 @@ fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
impl TyrDrmFileData {
pub(crate) fn dev_query(
- ddev: &TyrDrmDevice,
+ ddev: &TyrDrmDevice<Registered>,
devquery: &mut uapi::drm_panthor_dev_query,
_file: &TyrDrmFile,
) -> Result<u32> {
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 70cf1aa4d788..6cefd26b31f9 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -71,6 +71,18 @@ pub mod internal {
pub use bindings::drm_file;
pub use bindings::drm_ioctl_desc;
+ /// Reinterpret a pointer to a DRM device with a different [`DeviceContext`], preserving the
+ /// driver type parameter `T`.
+ ///
+ /// Used by [`declare_drm_ioctls!`] to anchor type inference.
+ #[doc(hidden)]
+ #[inline(always)]
+ pub const fn __dev_ctx_cast<T: super::super::Driver>(
+ ptr: *const super::super::device::Device<T, super::super::Ioctl>,
+ ) -> *const super::super::device::Device<T, super::super::Registered> {
+ ptr.cast()
+ }
+
/// Call an ioctl handler with lifetime-bounded references.
///
/// The lifetime `'a` is tied to the `_anchor` parameter. This prevents handlers from
@@ -115,7 +127,7 @@ pub unsafe fn __call_ioctl<
/// `user_callback` should have the following prototype:
///
/// ```ignore
-/// fn foo(device: &kernel::drm::Device<Self>,
+/// fn foo(device: &kernel::drm::Device<Self, kernel::drm::Registered>,
/// data: &mut uapi::argument_type,
/// file: &kernel::drm::File<Self::File>,
/// ) -> Result<u32>
@@ -164,11 +176,38 @@ macro_rules! declare_drm_ioctls {
// - The DRM device must have been registered when we're called through
// an IOCTL.
//
+ // INVARIANT: The `Ioctl` context requires that the device has been
+ // registered via `drm_dev_register()` at some point; the DRM core
+ // guarantees this for ioctl dispatch callbacks.
+ //
// FIXME: Currently there is nothing enforcing that the types of the
// dev/file match the current driver these ioctls are being declared
// for, and it's not clear how to enforce this within the type system.
- let dev: &$crate::drm::device::Device<_, $crate::drm::Normal> =
+ let dev: &$crate::drm::device::Device<_, $crate::drm::Ioctl> =
$crate::drm::device::Device::from_raw(raw_dev);
+ // Cast to Registered preserving the driver type parameter.
+ let __ptr = $crate::drm::ioctl::internal::__dev_ctx_cast(
+ ::core::ptr::from_ref(dev),
+ );
+
+ // Type-inference anchor: the closure is never called but ties `dev`'s
+ // type to `$func`'s first parameter, which the compiler cannot infer
+ // through method resolution and associated-type projections alone.
+ #[allow(unreachable_code)]
+ let _ = || {
+ $func(
+ // SAFETY: This closure is never executed; the dereference
+ // exists purely to unify the type parameter with `$func`.
+ // The pointer is valid regardless.
+ unsafe { &*__ptr },
+ unreachable!(),
+ unreachable!(),
+ )
+ };
+
+ let Some(guard) = dev.registration_guard() else {
+ return $crate::error::code::ENODEV.to_errno();
+ };
let __anchor = ();
// SAFETY:
@@ -180,7 +219,7 @@ macro_rules! declare_drm_ioctls {
// - `raw_file` is a valid `struct drm_file` pointer provided by the
// DRM core.
match unsafe { $crate::drm::ioctl::internal::__call_ioctl(
- &__anchor, dev, raw_data, raw_file, $func,
+ &__anchor, &*guard, raw_data, raw_file, $func,
) } {
Err(e) => e.to_errno(),
Ok(i) => i.try_into()
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 11/16] rust: drm: Wrap ioctl dispatch in RegistrationGuard
2026-06-20 18:48 ` [PATCH v4 11/16] rust: drm: Wrap ioctl dispatch in RegistrationGuard Danilo Krummrich
@ 2026-06-20 18:58 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 18:58 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: ojeda, Alexandre Courbot, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The `declare_drm_ioctls!` macro fails to constrain the ioctl handler's data argument type to the declared UAPI `$struct`, which allows handlers to silently accept larger types and causes out-of-bounds memory accesses.
--
commit aaaa8c4ed4a35894ea9d49cf012ca359ca7c8ab5
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: Wrap ioctl dispatch in RegistrationGuard
This patch ensures that ioctl handlers receive a reference to a registered
device, proving at the type level that the device has not been unplugged.
It achieves this by calling `registration_guard()` on the device and using a
dead-code closure as a type-inference anchor to resolve the driver type
parameter.
> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index 70cf1aa4d788f..6cefd26b31f91 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs
[ ... ]
> @@ -164,11 +176,38 @@ macro_rules! declare_drm_ioctls {
[ ... ]
> + // Type-inference anchor: the closure is never called but ties `dev`'s
> + // type to `$func`'s first parameter, which the compiler cannot infer
> + // through method resolution and associated-type projections alone.
> + #[allow(unreachable_code)]
> + let _ = || {
> + $func(
> + // SAFETY: This closure is never executed; the dereference
> + // exists purely to unify the type parameter with `$func`.
> + // The pointer is valid regardless.
> + unsafe { &*__ptr },
> + unreachable!(),
> + unreachable!(),
[Severity: Critical]
Does using unreachable!() here drop the type constraint on the UAPI struct?
Because unreachable!() coerces to any type, it appears this closure fails to
anchor the data argument to the expected UAPI struct.
This allows the compiler to infer the data type entirely from the user-provided
handler signature without enforcing that it matches the declared UAPI struct.
> + )
> + };
> +
> + let Some(guard) = dev.registration_guard() else {
> + return $crate::error::code::ENODEV.to_errno();
> + };
> let __anchor = ();
>
> // SAFETY:
> @@ -180,7 +219,7 @@ macro_rules! declare_drm_ioctls {
> // - `raw_file` is a valid `struct drm_file` pointer provided by the
> // DRM core.
> match unsafe { $crate::drm::ioctl::internal::__call_ioctl(
> - &__anchor, dev, raw_data, raw_file, $func,
> + &__anchor, &*guard, raw_data, raw_file, $func,
[Severity: Critical]
If the handler author accidentally specifies a larger type for the data
argument, could this lead to out-of-bounds memory access?
Since __call_ioctl() infers its generic data type directly from the $func
signature, it will cast the raw_data pointer to that inferred type.
The DRM core allocates the raw_data buffer exactly based on the expected UAPI
struct size. If the handler silently accepts a larger struct type, casting and
dereferencing it in __call_ioctl() would cause out-of-bounds memory accesses.
> ) } {
> Err(e) => e.to_errno(),
> Ok(i) => i.try_into()
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=11
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 12/16] rust: drm: return ParentDevice from Device AsRef
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (10 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 11/16] rust: drm: Wrap ioctl dispatch in RegistrationGuard Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 19:01 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 13/16] rust: drm: add AsRef<ParentDevice<Bound>> for Device<Registered> Danilo Krummrich
` (3 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Change AsRef for drm::Device to return &T::ParentDevice<device::Normal>
instead of &device::Device, and restrict it to the Normal context.
Device<T, Registered> still gets this through Deref coercion.
This provides access to the typed parent bus device rather than the raw
base device.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 10 +++++++---
rust/kernel/drm/driver.rs | 2 +-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index c32cc0f0eba0..45bacd7b227f 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -450,11 +450,15 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
-impl<T: drm::Driver, C: DeviceContext> AsRef<device::Device> for Device<T, C> {
- fn as_ref(&self) -> &device::Device {
+impl<T: drm::Driver> AsRef<T::ParentDevice<device::Normal>> for Device<T> {
+ fn as_ref(&self) -> &T::ParentDevice<device::Normal> {
// SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
// which is guaranteed by the type invariant.
- unsafe { device::Device::from_raw((*self.as_raw()).dev) }
+ let dev = unsafe { device::Device::from_raw((*self.as_raw()).dev) };
+
+ // SAFETY: By the type invariant of `Device`, the parent device is embedded in
+ // `T::ParentDevice`.
+ unsafe { device::AsBusDevice::from_device(dev) }
}
}
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 3cda8dceb498..ceb2829985c7 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -170,7 +170,7 @@ pub fn new_foreign_owned<'a>(
where
T: 'static,
{
- if drm.as_ref().as_raw() != dev.as_raw() {
+ if drm.as_ref().as_ref().as_raw() != dev.as_raw() {
return Err(EINVAL);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 12/16] rust: drm: return ParentDevice from Device AsRef
2026-06-20 18:48 ` [PATCH v4 12/16] rust: drm: return ParentDevice from Device AsRef Danilo Krummrich
@ 2026-06-20 19:01 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 19:01 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: dri-devel, Alexandre Courbot, ojeda
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Safety comment relies on an undocumented type invariant regarding the parent device.
--
commit b94604b443e7e44bdf48a3790177f869613bf287
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: return ParentDevice from Device AsRef
This commit changes the AsRef implementation for drm::Device to return
a reference to the typed parent bus device rather than the raw base device.
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index c32cc0f0eba03..45bacd7b227f0 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
[ ... ]
> @@ -450,11 +450,15 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> -impl<T: drm::Driver, C: DeviceContext> AsRef<device::Device> for Device<T, C> {
> - fn as_ref(&self) -> &device::Device {
> +impl<T: drm::Driver> AsRef<T::ParentDevice<device::Normal>> for Device<T> {
> + fn as_ref(&self) -> &T::ParentDevice<device::Normal> {
> // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
> // which is guaranteed by the type invariant.
> - unsafe { device::Device::from_raw((*self.as_raw()).dev) }
> + let dev = unsafe { device::Device::from_raw((*self.as_raw()).dev) };
> +
> + // SAFETY: By the type invariant of `Device`, the parent device is embedded in
> + // `T::ParentDevice`.
> + unsafe { device::AsBusDevice::from_device(dev) }
[Severity: Low]
This isn't a bug, but the SAFETY comment references a type invariant of
Device that doesn't appear to be documented.
Looking at the # Invariants section for the Device struct, it only states
that self.dev is a valid instance of a struct device, its data layout
remains the same across contexts, and context invariants apply.
Should the # Invariants section of Device be updated to explicitly state
that the parent device is embedded in T::ParentDevice to properly support
this safety proof?
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=12
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 13/16] rust: drm: add AsRef<ParentDevice<Bound>> for Device<Registered>
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (11 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 12/16] rust: drm: return ParentDevice from Device AsRef Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 19:02 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 14/16] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
` (2 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Implement AsRef<T::ParentDevice<Bound>> for Device<T, Registered>,
providing access to the bound parent bus device for registered DRM
devices.
Since a Device<T, Registered> guarantees that the parent bus device is
bound, the conversion to T::ParentDevice<Bound> is safe.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 45bacd7b227f..8f63276c9b62 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -462,6 +462,19 @@ fn as_ref(&self) -> &T::ParentDevice<device::Normal> {
}
}
+impl<T: drm::Driver> AsRef<T::ParentDevice<device::Bound>> for Device<T, Registered> {
+ fn as_ref(&self) -> &T::ParentDevice<device::Bound> {
+ let dev = (**self).as_ref().as_ref();
+
+ // SAFETY: A `Device<T, Registered>` guarantees that the parent device is bound.
+ let dev = unsafe { dev.as_bound() };
+
+ // SAFETY: By the type invariant of `Device`, the parent device is embedded in
+ // `T::ParentDevice`.
+ unsafe { device::AsBusDevice::from_device(dev) }
+ }
+}
+
// SAFETY: A `drm::Device` can be released from any thread.
unsafe impl<T: drm::Driver, C: DeviceContext> Send for Device<T, C> {}
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 13/16] rust: drm: add AsRef<ParentDevice<Bound>> for Device<Registered>
2026-06-20 18:48 ` [PATCH v4 13/16] rust: drm: add AsRef<ParentDevice<Bound>> for Device<Registered> Danilo Krummrich
@ 2026-06-20 19:02 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 19:02 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: dri-devel, ojeda, Alexandre Courbot
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `#[inline]` annotation on the small forwarding function `as_ref` in an abstraction crate.
--
commit 61d54b007aa9572681d918fcd5c48802182d8282
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: add AsRef<ParentDevice<Bound>> for Device<Registered>
This commit implements AsRef<T::ParentDevice<Bound>> for Device<T, Registered>.
It provides safe access to the bound parent bus device for registered DRM
devices.
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 45bacd7b227f0..8f63276c9b62f 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -462,6 +462,19 @@ fn as_ref(&self) -> &T::ParentDevice<device::Normal> {
> }
> }
>
> +impl<T: drm::Driver> AsRef<T::ParentDevice<device::Bound>> for Device<T, Registered> {
> + fn as_ref(&self) -> &T::ParentDevice<device::Bound> {
[Severity: Low]
This isn't a bug, but should this small forwarding function in an abstraction
crate have an #[inline] annotation?
The Rust subsystem coding guidelines mention that functions in abstractions
that are small or simply forwarding calls should be annotated with #[inline].
Since as_ref() performs purely type-level conversions and pointer coercions,
adding the annotation might help the compiler reliably optimize these across
crate boundaries.
> + let dev = (**self).as_ref().as_ref();
> +
> + // SAFETY: A `Device<T, Registered>` guarantees that the parent device is bound.
> + let dev = unsafe { dev.as_bound() };
> +
> + // SAFETY: By the type invariant of `Device`, the parent device is embedded in
> + // `T::ParentDevice`.
> + unsafe { device::AsBusDevice::from_device(dev) }
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=13
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 14/16] rust: drm: Add RegistrationData to drm::Driver
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (12 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 13/16] rust: drm: add AsRef<ParentDevice<Bound>> for Device<Registered> Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 19:06 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 15/16] rust: drm: Pass registration data to ioctl handlers Danilo Krummrich
2026-06-20 18:48 ` [PATCH v4 16/16] drm: nova: Use drm::Device<Registered> to access the parent bus device Danilo Krummrich
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Add a RegistrationData associated type to drm::Driver. This is a ForLt
type whose lifetime is tied to the parent bus device binding scope.
Registration<'a, T> takes ownership of the data via Pin<KBox<_>>,
storing it with its real lifetime. The pointer is written to drm::Device
before drm_dev_register() to ensure it is already in place when ioctls
arrive.
Device<T, Registered>::registration_data_with() provides access with the
lifetime shortened from 'static via a pointer cast. Since
Registration::drop() calls drm_dev_unplug(), which performs an SRCU
barrier waiting for all drm_dev_enter() critical sections to complete,
the data is guaranteed to remain valid for the duration of any
RegistrationGuard.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 20 +++++--
drivers/gpu/drm/tyr/driver.rs | 18 ++++--
rust/kernel/drm/device.rs | 49 +++++++++++++++
rust/kernel/drm/driver.rs | 106 +++++++++++++++++++++------------
4 files changed, 143 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index e3c54303d70e..131212df94d3 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -12,7 +12,8 @@
ioctl, //
},
prelude::*,
- sync::aref::ARef, //
+ sync::aref::ARef,
+ types::ForLt, //
};
use crate::file::File;
@@ -20,9 +21,10 @@
pub(crate) struct NovaDriver;
-pub(crate) struct Nova {
+pub(crate) struct Nova<'bound> {
#[expect(unused)]
drm: ARef<drm::Device<NovaDriver>>,
+ _reg: drm::Registration<'bound, NovaDriver>,
}
/// Convienence type alias for the DRM device type for this driver
@@ -56,7 +58,7 @@ pub(crate) struct NovaData {
impl auxiliary::Driver for NovaDriver {
type IdInfo = ();
- type Data<'bound> = Nova;
+ type Data<'bound> = Nova<'bound>;
const ID_TABLE: auxiliary::IdTable<Self::IdInfo> = &AUX_TABLE;
fn probe<'bound>(
@@ -66,15 +68,21 @@ fn probe<'bound>(
let data = try_pin_init!(NovaData { adev: adev.into() });
let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
- let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
-
- Ok(Nova { drm: drm.into() })
+ // SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is
+ // never forgotten.
+ let reg = unsafe { drm::Registration::new(adev.as_ref(), drm, (), 0)? };
+
+ Ok(Nova {
+ drm: reg.device().into(),
+ _reg: reg,
+ })
}
}
#[vtable]
impl drm::Driver for NovaDriver {
type Data = NovaData;
+ type RegistrationData = ForLt!(());
type File = File;
type Object = gem::Object<NovaObject>;
type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 7f082de6d6dc..e017448aabab 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -31,7 +31,8 @@
aref::ARef,
Mutex, //
},
- time, //
+ time,
+ types::ForLt, //
};
use crate::{
@@ -52,8 +53,9 @@
pub(crate) struct TyrPlatformDriver;
#[pin_data(PinnedDrop)]
-pub(crate) struct TyrPlatformDriverData {
+pub(crate) struct TyrPlatformDriverData<'bound> {
_device: ARef<TyrDrmDevice>,
+ _reg: drm::Registration<'bound, TyrDrmDriver>,
}
#[pin_data]
@@ -98,7 +100,7 @@ fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
impl platform::Driver for TyrPlatformDriver {
type IdInfo = ();
- type Data<'bound> = TyrPlatformDriverData;
+ type Data<'bound> = TyrPlatformDriverData<'bound>;
const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
fn probe<'bound>(
@@ -150,10 +152,13 @@ fn probe<'bound>(
});
let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
- let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
+ // SAFETY: `reg` is stored in `TyrPlatformDriverData` and dropped when the driver is
+ // unbound; it is never forgotten.
+ let reg = unsafe { drm::Registration::new(pdev.as_ref(), tdev, (), 0)? };
let driver = TyrPlatformDriverData {
- _device: tdev.into(),
+ _device: reg.device().into(),
+ _reg: reg,
};
// We need this to be dev_info!() because dev_dbg!() does not work at
@@ -164,7 +169,7 @@ fn probe<'bound>(
}
#[pinned_drop]
-impl PinnedDrop for TyrPlatformDriverData {
+impl PinnedDrop for TyrPlatformDriverData<'_> {
fn drop(self: Pin<&mut Self>) {}
}
@@ -181,6 +186,7 @@ fn drop(self: Pin<&mut Self>) {}
#[vtable]
impl drm::Driver for TyrDrmDriver {
type Data = TyrDrmDeviceData;
+ type RegistrationData = ForLt!(());
type File = TyrDrmFileData;
type Object = drm::gem::shmem::Object<BoData>;
type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 8f63276c9b62..fc79f90a197d 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -20,6 +20,7 @@
AlwaysRefCounted, //
},
types::{
+ ForLt,
NotThreadSafe,
Opaque, //
},
@@ -32,6 +33,7 @@
};
use core::{
alloc::Layout,
+ cell::UnsafeCell,
marker::PhantomData,
mem,
ops::Deref,
@@ -247,6 +249,9 @@ pub fn new(
// SAFETY: `drm_dev` is still private to this function.
unsafe { (*drm_dev).driver = const { &Self::VTABLE } };
+ // SAFETY: `raw_drm` is valid; no concurrent access before registration.
+ unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
+
// SAFETY: The reference count is one, and now we take ownership of that reference as a
// `drm::Device`.
// INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
@@ -270,6 +275,7 @@ pub fn new(
pub struct Device<T: drm::Driver, C: DeviceContext = Normal> {
dev: Opaque<bindings::drm_device>,
data: T::Data,
+ pub(super) registration_data: UnsafeCell<NonNull<<T::RegistrationData as ForLt>::Of<'static>>>,
_ctx: PhantomData<C>,
}
@@ -278,6 +284,28 @@ pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
self.dev.get()
}
+ /// Returns a reference to the registration data with lifetime shortened from `'static`.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that:
+ ///
+ /// - The parent bus device is bound (e.g. by holding an active `drm_dev_enter()` critical
+ /// section via [`RegistrationGuard`]).
+ /// - The returned reference is not exposed to code that can choose a concrete lifetime for
+ /// it, as that would be unsound for types that are invariant over their lifetime parameter
+ /// (e.g. it must be passed through an HRTB-bounded closure).
+ #[inline]
+ unsafe fn registration_data_unchecked(&self) -> &<T::RegistrationData as ForLt>::Of<'_> {
+ // SAFETY:
+ // - Caller guarantees the parent bus device is bound, hence the pointer is valid.
+ // - The pointer cast from `Of<'static>` to `Of<'_>` is layout-compatible since lifetimes
+ // are erased at runtime.
+ // - Caller guarantees the reference is only used behind an HRTB, making the lifetime
+ // shortening sound regardless of variance.
+ unsafe { (*self.registration_data.get()).cast::<_>().as_ref() }
+ }
+
/// # Safety
///
/// `ptr` must be a valid pointer to a `struct device` embedded in `Self`.
@@ -385,6 +413,27 @@ pub struct RegistrationGuard<'a, T: drm::Driver> {
_not_send: NotThreadSafe,
}
+impl<T: drm::Driver> Device<T, Registered> {
+ /// Access the registration data through a closure, with the lifetime tied to the closure
+ /// scope.
+ ///
+ /// The data is owned by [`Registration`](drm::Registration) and is guaranteed to remain valid
+ /// as long as the device is registered, since [`Registration`](drm::Registration)'s `drop`
+ /// calls `drm_dev_unplug()` which waits for all `drm_dev_enter()` critical sections to
+ /// complete.
+ #[inline]
+ pub fn registration_data_with<R, F>(&self, f: F) -> R
+ where
+ F: for<'a> FnOnce(&'a <T::RegistrationData as ForLt>::Of<'a>) -> R,
+ {
+ // SAFETY: `Registered` guarantees the device is registered and the parent bus device is
+ // bound. The closure's HRTB `for<'a>` prevents the caller from smuggling in references
+ // with a concrete short lifetime, satisfying the lifetime requirement of
+ // `registration_data_unchecked`.
+ f(unsafe { self.registration_data_unchecked() })
+ }
+}
+
impl<T: drm::Driver> Deref for RegistrationGuard<'_, T> {
type Target = Device<T, Registered>;
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index ceb2829985c7..e42b48e5cad2 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -7,11 +7,11 @@
use crate::{
bindings,
device,
- devres,
drm,
error::to_result,
prelude::*,
- sync::aref::ARef, //
+ sync::aref::ARef,
+ types::ForLt, //
};
use core::{
mem,
@@ -110,6 +110,16 @@ pub trait Driver {
/// Context data associated with the DRM driver
type Data: Sync + Send;
+ /// Data owned by the [`Registration`] and accessible within a
+ /// [`RegistrationGuard`](drm::RegistrationGuard) critical section via
+ /// [`Device::registration_data_with()`](drm::Device::registration_data_with).
+ ///
+ /// This is a [`ForLt`](trait@ForLt) type whose lifetime is tied to the parent bus
+ /// device binding scope. References handed out by
+ /// [`Device::registration_data_with()`](drm::Device::registration_data_with) are tied to
+ /// the closure scope.
+ type RegistrationData: ForLt;
+
/// The type used to manage memory for this driver.
type Object: AllocImpl;
@@ -139,65 +149,82 @@ pub trait Driver {
/// The registration type of a `drm::Device`.
///
/// Once the `Registration` structure is dropped, the device is unregistered.
-pub struct Registration<T: Driver>(ARef<drm::Device<T>>);
-
-impl<T: Driver> Registration<T> {
- fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
- // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`.
- to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?;
-
- // SAFETY: We just called `drm_dev_register` above
- let new = NonNull::from(unsafe { drm.assume_ctx() });
-
- // Leak the ARef from UnregisteredDevice in preparation for transferring its ownership.
- mem::forget(drm);
-
- // SAFETY: `drm`'s `Drop` constructor was never called, ensuring that there remains at least
- // one reference to the device - which we take ownership over here.
- let new = unsafe { ARef::from_raw(new) };
-
- Ok(Self(new))
- }
+pub struct Registration<'a, T: Driver> {
+ drm: ARef<drm::Device<T>>,
+ _reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>>,
+}
- /// Registers a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
+impl<'a, T: Driver> Registration<'a, T>
+where
+ for<'b> <T::RegistrationData as ForLt>::Of<'b>: Send + Sync,
+{
+ /// Register a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
///
- /// Ownership of the [`Registration`] object is passed to [`devres::register`].
- pub fn new_foreign_owned<'a>(
- drm: drm::UnregisteredDevice<T>,
+ /// # Safety
+ ///
+ /// The caller must not `mem::forget()` the returned [`Registration`] or otherwise prevent its
+ /// [`Drop`] implementation from running, since the registration data may contain borrowed
+ /// references that become invalid after `'a` ends.
+ pub unsafe fn new<E>(
dev: &'a device::Device<device::Bound>,
+ drm: drm::UnregisteredDevice<T>,
+ reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, E>,
flags: usize,
- ) -> Result<&'a drm::Device<T>>
+ ) -> Result<Self>
where
- T: 'static,
+ Error: From<E>,
{
if drm.as_ref().as_ref().as_raw() != dev.as_raw() {
return Err(EINVAL);
}
- let reg = Registration::<T>::new(drm, flags)?;
- let drm = NonNull::from(reg.device());
+ let reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>> =
+ KBox::pin_init(reg_data, GFP_KERNEL)?;
- devres::register(dev, reg, GFP_KERNEL)?;
+ // Store the registration data pointer in the device before registration, so that it is
+ // visible once ioctls can be called.
+ //
+ // SAFETY: Lifetimes do not affect layout, so the pointer cast is sound.
+ let ptr: NonNull<<T::RegistrationData as ForLt>::Of<'static>> =
+ unsafe { mem::transmute(NonNull::from(Pin::get_ref(reg_data.as_ref()))) };
+
+ // SAFETY: No concurrent access; the device is not yet registered.
+ unsafe { *drm.registration_data.get() = ptr };
+
+ // SAFETY: `drm` is a valid, initialized but not yet registered DRM device.
+ let ret = unsafe { bindings::drm_dev_register(drm.as_raw(), flags) };
+ if let Err(e) = to_result(ret) {
+ // SAFETY: No concurrent access; registration failed.
+ unsafe { *drm.registration_data.get() = NonNull::dangling() };
+ return Err(e);
+ }
- // SAFETY: Since `reg` was passed to devres::register(), the device now owns the lifetime
- // of the DRM registration - ensuring that this references lives for at least as long as 'a.
- Ok(unsafe { drm.as_ref() })
+ Ok(Self {
+ drm: (&*drm).into(),
+ _reg_data: reg_data,
+ })
}
/// Returns a reference to the `Device` instance for this registration.
pub fn device(&self) -> &drm::Device<T> {
- &self.0
+ &self.drm
}
}
// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between
// threads, hence it's safe to share it.
-unsafe impl<T: Driver> Sync for Registration<T> {}
+unsafe impl<T: Driver> Sync for Registration<'_, T> where
+ for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send + Sync
+{
+}
// SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread.
-unsafe impl<T: Driver> Send for Registration<T> {}
+unsafe impl<T: Driver> Send for Registration<'_, T> where
+ for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send + Sync
+{
+}
-impl<T: Driver> Drop for Registration<T> {
+impl<T: Driver> Drop for Registration<'_, T> {
fn drop(&mut self) {
// Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
// `drm_dev_enter()` critical sections complete before unregistration proceeds. This
@@ -207,6 +234,9 @@ fn drop(&mut self) {
//
// SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
// `Registration` also guarantees that this `drm::Device` is actually registered.
- unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
+ unsafe { bindings::drm_dev_unplug(self.drm.as_raw()) };
+ // After drm_dev_unplug(), the SRCU barrier guarantees that all RegistrationGuard critical
+ // sections have completed, so no one holds a reference to reg_data anymore.
+ // reg_data is dropped here automatically.
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 14/16] rust: drm: Add RegistrationData to drm::Driver
2026-06-20 18:48 ` [PATCH v4 14/16] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
@ 2026-06-20 19:06 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 19:06 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: Alexandre Courbot, ojeda, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Unsound lifetime shortening of `RegistrationData` allows safe code to trigger a use-after-free.
- [High] Data race and use-after-free on `RegistrationData` when `drm_dev_register()` fails.
--
commit 4c4fafa27855b4e677d80a9c3bf6cb9acd303180
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: Add RegistrationData to drm::Driver
This patch introduces a RegistrationData associated type for drm::Driver
whose lifetime is tied to the parent bus device. It manages the registration
scope and provides safe access to this data within a RegistrationGuard.
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 8f63276c9b62f..fc79f90a197d9 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
[ ... ]
> + #[inline]
> + unsafe fn registration_data_unchecked(&self) -> &<T::RegistrationData as ForLt>::Of<'_> {
> + // SAFETY:
> + // - Caller guarantees the parent bus device is bound, hence the pointer is valid.
> + // - The pointer cast from `Of<'static>` to `Of<'_>` is layout-compatible since lifetimes
> + // are erased at runtime.
> + // - Caller guarantees the reference is only used behind an HRTB, making the lifetime
> + // shortening sound regardless of variance.
> + unsafe { (*self.registration_data.get()).cast::<_>().as_ref() }
> + }
[ ... ]
> + pub fn registration_data_with<R, F>(&self, f: F) -> R
> + where
> + F: for<'a> FnOnce(&'a <T::RegistrationData as ForLt>::Of<'a>) -> R,
> + {
> + // SAFETY: `Registered` guarantees the device is registered and the parent bus device is
> + // bound. The closure's HRTB `for<'a>` prevents the caller from smuggling in references
> + // with a concrete short lifetime, satisfying the lifetime requirement of
> + // `registration_data_unchecked`.
> + f(unsafe { self.registration_data_unchecked() })
> + }
[Severity: Critical]
Does this lifetime shortening cast violate soundness if the Of<'a> type is
invariant (e.g., Cell<&'a u32> or Mutex<Option<&'a u32>>)?
Even with the HRTB bound in registration_data_with(), safe driver code could
do something like device.registration_data_with(|data| data.set(&local_var))
where local_var outlives the closure but not the device's 'bound lifetime.
Because the lifetime of the invariant container was shortened to the closure's
scope, the compiler will accept this and allow writing a short-lived reference
into it.
When the function returns and local_var is dropped, the physical
RegistrationData container living in the device will hold a dangling pointer,
leading to a use-after-free when the data is accessed again.
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index ceb2829985c78..e42b48e5cad28 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
[ ... ]
> + // SAFETY: `drm` is a valid, initialized but not yet registered DRM device.
> + let ret = unsafe { bindings::drm_dev_register(drm.as_raw(), flags) };
> + if let Err(e) = to_result(ret) {
> + // SAFETY: No concurrent access; registration failed.
> + unsafe { *drm.registration_data.get() = NonNull::dangling() };
> + return Err(e);
> + }
[Severity: High]
Can this error path trigger a use-after-free if drm_dev_register() fails
partway through its initialization?
During drm_dev_register(), minor nodes (such as the render node) are
registered sequentially via drm_minor_register(). As soon as a minor node is
registered, userspace can open it and begin executing ioctls. These concurrent
ioctls will successfully enter the RegistrationGuard critical section since
dev->unplugged is false.
If drm_dev_register() subsequently fails on the primary node, it returns an
error without calling drm_dev_unplug() (which would otherwise perform an SRCU
barrier to wait for existing readers). Returning Err(e) here causes the
RegistrationData to be dropped and freed, while a concurrent ioctl still
inside its drm_dev_enter() critical section could access this freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=14
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 15/16] rust: drm: Pass registration data to ioctl handlers
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (13 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 14/16] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
2026-06-20 19:13 ` sashiko-bot
2026-06-20 18:48 ` [PATCH v4 16/16] drm: nova: Use drm::Device<Registered> to access the parent bus device Danilo Krummrich
15 siblings, 1 reply; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Pass registration data to ioctl handlers via
drm::Device<Registered>::registration_data_with(). The closure's HRTB
ties the lifetime to the closure scope, and the pointer cast shortens it
from 'static internally. The reference is valid for the duration of the
drm_dev_enter/exit critical section held by RegistrationGuard.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/file.rs | 3 +++
drivers/gpu/drm/tyr/file.rs | 1 +
rust/kernel/drm/ioctl.rs | 31 ++++++++++++++++++-------------
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 19fb89b28984..208be4e38188 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -28,6 +28,7 @@ impl File {
/// IOCTL: get_param: Query GPU / driver metadata.
pub(crate) fn get_param(
dev: &NovaDevice<Registered>,
+ _reg_data: &(),
getparam: &mut uapi::drm_nova_getparam,
_file: &drm::File<File>,
) -> Result<u32> {
@@ -48,6 +49,7 @@ pub(crate) fn get_param(
/// IOCTL: gem_create: Create a new DRM GEM object.
pub(crate) fn gem_create(
dev: &NovaDevice<Registered>,
+ _reg_data: &(),
req: &mut uapi::drm_nova_gem_create,
file: &drm::File<File>,
) -> Result<u32> {
@@ -61,6 +63,7 @@ pub(crate) fn gem_create(
/// IOCTL: gem_info: Query GEM metadata.
pub(crate) fn gem_info(
_dev: &NovaDevice<Registered>,
+ _reg_data: &(),
req: &mut uapi::drm_nova_gem_info,
file: &drm::File<File>,
) -> Result<u32> {
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index fb9233eae01c..b686041d5d6b 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -32,6 +32,7 @@ fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
impl TyrDrmFileData {
pub(crate) fn dev_query(
ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &(),
devquery: &mut uapi::drm_panthor_dev_query,
_file: &TyrDrmFile,
) -> Result<u32> {
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 6cefd26b31f9..75944e72d7af 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -97,12 +97,13 @@ pub const fn __dev_ctx_cast<T: super::super::Driver>(
#[inline(always)]
pub unsafe fn __call_ioctl<
'a,
+ Anchor,
Dev: 'a,
Data: 'a,
F: super::super::file::DriverFile + 'a,
Ret,
>(
- _anchor: &'a (),
+ _anchor: &'a Anchor,
dev: &'a Dev,
raw_data: *mut ::core::ffi::c_void,
raw_file: *mut drm_file,
@@ -128,6 +129,7 @@ pub unsafe fn __call_ioctl<
///
/// ```ignore
/// fn foo(device: &kernel::drm::Device<Self, kernel::drm::Registered>,
+/// reg_data: &<Self::RegistrationData as kernel::types::ForLt>::Of<'_>,
/// data: &mut uapi::argument_type,
/// file: &kernel::drm::File<Self::File>,
/// ) -> Result<u32>
@@ -202,25 +204,28 @@ macro_rules! declare_drm_ioctls {
unsafe { &*__ptr },
unreachable!(),
unreachable!(),
+ unreachable!(),
)
};
let Some(guard) = dev.registration_guard() else {
return $crate::error::code::ENODEV.to_errno();
};
- let __anchor = ();
- // SAFETY:
- // - The ioctl argument has size `_IOC_SIZE(cmd)`, which we asserted
- // above matches the size of this type, and all bit patterns of UAPI
- // structs must be valid. The argument is exclusively owned by this
- // handler, guaranteed by `drm_ioctl()` to remain valid for the
- // duration of the call.
- // - `raw_file` is a valid `struct drm_file` pointer provided by the
- // DRM core.
- match unsafe { $crate::drm::ioctl::internal::__call_ioctl(
- &__anchor, &*guard, raw_data, raw_file, $func,
- ) } {
+ match guard.registration_data_with(|reg_data| {
+ // SAFETY:
+ // - The ioctl argument has size `_IOC_SIZE(cmd)`, which we asserted
+ // above matches the size of this type, and all bit patterns of
+ // UAPI structs must be valid. The argument is exclusively owned
+ // by this handler, guaranteed by `drm_ioctl()` to remain valid
+ // for the duration of the call.
+ // - `raw_file` is a valid `struct drm_file` pointer provided by
+ // the DRM core.
+ unsafe { $crate::drm::ioctl::internal::__call_ioctl(
+ reg_data, &*guard, raw_data, raw_file,
+ |dev, data, file| $func(dev, reg_data, data, file),
+ ) }
+ }) {
Err(e) => e.to_errno(),
Ok(i) => i.try_into()
.unwrap_or($crate::error::code::ERANGE.to_errno()),
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 15/16] rust: drm: Pass registration data to ioctl handlers
2026-06-20 18:48 ` [PATCH v4 15/16] rust: drm: Pass registration data to ioctl handlers Danilo Krummrich
@ 2026-06-20 19:13 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-06-20 19:13 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: ojeda, Alexandre Courbot, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `declare_drm_ioctls!` macro fails to constrain the `Data` type parameter in `__call_ioctl`, allowing type inference to deduce an arbitrarily large struct size that leads to out-of-bounds memory accesses.
--
commit d383776ed6368e3f51c2779206ce9abab5ecfc2d
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: Pass registration data to ioctl handlers
This commit passes registration data to ioctl handlers via
drm::Device<Registered>::registration_data_with(). It modifies the ioctl
dispatching macro declare_drm_ioctls! and the internal __call_ioctl helper
to thread the registration data through to the ioctl callbacks.
> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index 6cefd26b31f91..75944e72d7af4 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs
[ ... ]
> @@ -202,25 +204,28 @@ macro_rules! declare_drm_ioctls {
> unsafe { &*__ptr },
> unreachable!(),
> unreachable!(),
> + unreachable!(),
> )
> };
>
> let Some(guard) = dev.registration_guard() else {
> return $crate::error::code::ENODEV.to_errno();
> };
> - let __anchor = ();
>
> - // SAFETY:
> - // - The ioctl argument has size `_IOC_SIZE(cmd)`, which we asserted
> - // above matches the size of this type, and all bit patterns of UAPI
> - // structs must be valid. The argument is exclusively owned by this
> - // handler, guaranteed by `drm_ioctl()` to remain valid for the
> - // duration of the call.
> - // - `raw_file` is a valid `struct drm_file` pointer provided by the
> - // DRM core.
> - match unsafe { $crate::drm::ioctl::internal::__call_ioctl(
> - &__anchor, &*guard, raw_data, raw_file, $func,
> - ) } {
> + match guard.registration_data_with(|reg_data| {
> + // SAFETY:
> + // - The ioctl argument has size `_IOC_SIZE(cmd)`, which we asserted
> + // above matches the size of this type, and all bit patterns of
> + // UAPI structs must be valid. The argument is exclusively owned
> + // by this handler, guaranteed by `drm_ioctl()` to remain valid
> + // for the duration of the call.
> + // - `raw_file` is a valid `struct drm_file` pointer provided by
> + // the DRM core.
> + unsafe { $crate::drm::ioctl::internal::__call_ioctl(
> + reg_data, &*guard, raw_data, raw_file,
> + |dev, data, file| $func(dev, reg_data, data, file),
> + ) }
[Severity: High]
In the declare_drm_ioctls! macro in rust/kernel/drm/ioctl.rs, does this
closure allow type inference to deduce an arbitrarily large struct size that
leads to out-of-bounds memory accesses?
Because Data is not explicitly constrained here, its type is inferred
backward from the user-provided $func. If an ioctl handler is written to
accept a larger mutable reference, it seems Data would be inferred as that
larger type.
The __call_ioctl helper will then cast the raw_data pointer to that larger
type and safely dereference it. Since the DRM core only allocates
_IOC_SIZE(cmd) bytes for the ioctl payload, couldn't this allow Safe Rust to
read and write past the allocated kernel buffer?
Should the closure explicitly annotate the type of data (e.g.,
|dev, data: &mut $crate::uapi::$struct, file|) to prevent type inference
from breaking the size constraint?
> + }) {
> Err(e) => e.to_errno(),
> Ok(i) => i.try_into()
> .unwrap_or($crate::error::code::ERANGE.to_errno()),
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620184924.2247517-1-dakr@kernel.org?part=15
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 16/16] drm: nova: Use drm::Device<Registered> to access the parent bus device
2026-06-20 18:47 [PATCH v4 00/16] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (14 preceding siblings ...)
2026-06-20 18:48 ` [PATCH v4 15/16] rust: drm: Pass registration data to ioctl handlers Danilo Krummrich
@ 2026-06-20 18:48 ` Danilo Krummrich
15 siblings, 0 replies; 27+ messages in thread
From: Danilo Krummrich @ 2026-06-20 18:48 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon, lyude
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
The get_param ioctl needs access to the parent auxiliary device. Since
ioctl handlers run inside a RegistrationGuard, accept
&NovaDevice<Registered> to obtain &auxiliary::Device<Bound> via as_ref()
directly. This removes the need for drm::Device data, hence set it to
().
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 11 ++---------
drivers/gpu/drm/nova/file.rs | 7 ++++---
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 131212df94d3..81df8158d83b 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -30,11 +30,6 @@ pub(crate) struct Nova<'bound> {
/// Convienence type alias for the DRM device type for this driver
pub(crate) type NovaDevice<Ctx = drm::Normal> = drm::Device<NovaDriver, Ctx>;
-#[pin_data]
-pub(crate) struct NovaData {
- pub(crate) adev: ARef<auxiliary::Device>,
-}
-
const INFO: drm::DriverInfo = drm::DriverInfo {
major: 0,
minor: 0,
@@ -65,9 +60,7 @@ fn probe<'bound>(
adev: &'bound auxiliary::Device<Core<'_>>,
_info: &'bound Self::IdInfo,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
- let data = try_pin_init!(NovaData { adev: adev.into() });
-
- let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
+ let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
// SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is
// never forgotten.
let reg = unsafe { drm::Registration::new(adev.as_ref(), drm, (), 0)? };
@@ -81,7 +74,7 @@ fn probe<'bound>(
#[vtable]
impl drm::Driver for NovaDriver {
- type Data = NovaData;
+ type Data = ();
type RegistrationData = ForLt!(());
type File = File;
type Object = gem::Object<NovaObject>;
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 208be4e38188..298c02bacb4b 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -4,6 +4,8 @@
use crate::gem::NovaObject;
use kernel::{
alloc::flags::*,
+ auxiliary,
+ device::Bound,
drm::{
self,
gem::BaseObject,
@@ -32,9 +34,8 @@ pub(crate) fn get_param(
getparam: &mut uapi::drm_nova_getparam,
_file: &drm::File<File>,
) -> Result<u32> {
- let adev = &dev.adev;
- let parent = adev.parent();
- let pdev: &pci::Device = parent.try_into()?;
+ let adev: &auxiliary::Device<Bound> = dev.as_ref();
+ let pdev: &pci::Device<Bound> = adev.parent().try_into()?;
let value = match getparam.param as u32 {
uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
--
2.54.0
^ permalink raw reply related [flat|nested] 27+ messages in thread