[PATCH 0/7] lifetime-parameterized DRM File private data

[PATCH 0/7] lifetime-parameterized DRM File private data

[Danilo Krummrich]

DRM file private data is currently 'static, preventing drivers from
borrowing registration-scoped resources in their per-file state. This
series makes Driver::File lifetime-parameterized through ForLt, so file
data can borrow from RegistrationData.

File data is guaranteed to be dropped before registration data, either
when the file is closed normally or when the device is unregistered,
whichever comes first. To enforce this during unbind, Registration::drop()
revokes all open file data and waits for in-flight closes before dropping
registration data.

fops.open is wrapped in a RegistrationGuard (drm_dev_enter/exit) so that
after drm_dev_unplug() all successfully opened files are accounted for.
GEM open/close callbacks are similarly gated to prevent access to stale
file data after unbind.

The series is based on drm-rust-next with [1] applied and [2] merged, a branch
with the patches is available in [3].

[1] https://lore.kernel.org/all/20260726223613.1242940-1-dakr@kernel.org/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/tag/?h=dd-lifetimes-7.3-rc1
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=drm/file

Danilo Krummrich (7):
  rust: drm: rename Ioctl device context to Userspace
  rust: drm: gem: gate open/close callbacks with RegistrationGuard
  rust: drm: move file_operations from gem to device
  rust: fs: add iminor() helper
  rust: drm: wrap fops open with RegistrationGuard
  rust: drm: make Driver::File lifetime-parameterized
  rust: drm: return impl PinInit from DriverFile::open()

 drivers/gpu/drm/nova/driver.rs |   5 +-
 drivers/gpu/drm/nova/file.rs   |  13 ++-
 drivers/gpu/drm/nova/gem.rs    |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |   5 +-
 drivers/gpu/drm/tyr/file.rs    |  13 +--
 rust/helpers/fs.c              |   5 ++
 rust/kernel/drm/device.rs      | 139 ++++++++++++++++++++++++++----
 rust/kernel/drm/driver.rs      |  69 ++++++++++++++-
 rust/kernel/drm/file.rs        | 151 ++++++++++++++++++++++-----------
 rust/kernel/drm/gem/mod.rs     |  48 +++++------
 rust/kernel/drm/gem/shmem.rs   |  14 +--
 rust/kernel/drm/ioctl.rs       |  14 +--
 rust/kernel/drm/mod.rs         |   2 +-
 13 files changed, 356 insertions(+), 130 deletions(-)

-- 
2.55.0

[PATCH 1/7] rust: drm: rename Ioctl device context to Userspace

[Danilo Krummrich]

The Ioctl DeviceContext typestate represents a device that has been
registered with userspace at some point. This context is not specific to
ioctl dispatch; it applies equally to GEM handle callbacks, mmap,
fdinfo, and any other operation triggered by userspace on a registered
device.

Rename it to Userspace to accurately reflect its semantics.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs | 21 ++++++++++++---------
 rust/kernel/drm/ioctl.rs  | 12 ++++++------
 rust/kernel/drm/mod.rs    |  2 +-
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index f43c6887ad23..be83287fe161 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -79,12 +79,12 @@ 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.
-/// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl
-///   dispatch context.
+/// - [`Userspace`]: The device has been registered with userspace at some point; used in
+///   callbacks triggered by userspace operations.
 /// - [`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`]),
+/// Both `Device<T, Userspace>` 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 + 'static {}
 
@@ -120,14 +120,17 @@ impl DeviceContext for Registered {}
 /// unregistering or already unregistered. `drm_dev_enter()` can guard against this, ensuring the
 /// device remains registered for the duration of the critical section.
 ///
+/// This context is used for all callbacks triggered by userspace operations: ioctls, GEM handle
+/// management, mmap, fdinfo, etc.
+///
 /// # Invariants
 ///
 /// A [`Device`] in this context has been registered with userspace via `drm_dev_register()` at
 /// some point.
-pub struct Ioctl;
+pub struct Userspace;
 
-impl Sealed for Ioctl {}
-impl DeviceContext for Ioctl {}
+impl Sealed for Userspace {}
+impl DeviceContext for Userspace {}
 
 /// A [`Device`] which is known at compile-time to be unregistered with userspace.
 ///
@@ -343,7 +346,7 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
     }
 }
 
-impl<T: drm::Driver> Device<T, Ioctl> {
+impl<T: drm::Driver> Device<T, Userspace> {
     /// Guard against the parent bus device being unbound.
     ///
     /// Returns a [`RegistrationGuard`] if the device has not been unplugged, [`None`] otherwise.
@@ -466,12 +469,12 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: drm::Driver> Deref for Device<T, Ioctl> {
+impl<T: drm::Driver> Deref for Device<T, Userspace> {
     type Target = Device<T>;
 
     #[inline]
     fn deref(&self) -> &Self::Target {
-        // SAFETY: The caller holds a `Device<T, Ioctl>`, which guarantees all invariants
+        // SAFETY: The caller holds a `Device<T, Userspace>`, which guarantees all invariants
         // of the weaker `Normal` context.
         unsafe { self.assume_ctx() }
     }
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 64af9eacc306..9934b23c36eb 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -71,14 +71,14 @@ pub mod internal {
     pub use bindings::drm_file;
     pub use bindings::drm_ioctl_desc;
 
-    /// Cast an [`Ioctl`] DRM device pointer to [`Registered`], preserving the driver type
+    /// Cast a [`Userspace`] DRM device pointer to [`Registered`], preserving the driver type
     /// parameter `T`.
     ///
     /// Used by [`declare_drm_ioctls!`] to anchor type inference.
     #[doc(hidden)]
     #[inline]
     pub const fn __dev_ctx_cast<T: crate::drm::Driver>(
-        ptr: *const crate::drm::Device<T, crate::drm::Ioctl>,
+        ptr: *const crate::drm::Device<T, crate::drm::Userspace>,
     ) -> *const crate::drm::Device<T, crate::drm::Registered> {
         ptr.cast()
     }
@@ -144,14 +144,14 @@ 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.
+                            // INVARIANT: The `Userspace` 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::Ioctl> =
+                            let dev: &$crate::drm::device::Device<_, $crate::drm::Userspace> =
                                 $crate::drm::device::Device::from_raw(raw_dev);
 
                             // Type-inference anchor: the closure is never called but ties `dev`'s
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index fd6ed35bc35a..7fcf2465a82e 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -11,11 +11,11 @@
 
 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::RegistrationGuard;
 pub use self::device::UnregisteredDevice;
+pub use self::device::Userspace;
 pub use self::driver::Driver;
 pub use self::driver::DriverInfo;
 pub use self::driver::Registration;
-- 
2.55.0

[PATCH 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard

[Danilo Krummrich]

Wrap the GEM object open and close callbacks with a RegistrationGuard
(drm_dev_enter / drm_dev_exit) to ensure the driver callbacks only run
while the parent bus device is bound.

If the device has been unbound, open returns -ENODEV and close silently
returns. This prevents driver code from accessing device resources after
unbind and is a prerequisite for making drm::Driver::File
lifetime-parameterized, since GEM callbacks receive a &drm::File that
could otherwise be used to access invalidated file private data.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/gem/mod.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 80d8f524f9d5..560403ca8e38 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -128,6 +128,14 @@ extern "C" fn open_callback<T: DriverObject>(
     raw_obj: *mut bindings::drm_gem_object,
     raw_file: *mut bindings::drm_file,
 ) -> core::ffi::c_int {
+    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
+    let dev: &drm::Device<T::Driver, drm::Userspace> =
+        unsafe { drm::Device::from_raw((*raw_obj).dev) };
+
+    let Some(_guard) = dev.registration_guard() else {
+        return ENODEV.to_errno();
+    };
+
     // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
@@ -148,6 +156,14 @@ extern "C" fn close_callback<T: DriverObject>(
     raw_obj: *mut bindings::drm_gem_object,
     raw_file: *mut bindings::drm_file,
 ) {
+    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
+    let dev: &drm::Device<T::Driver, drm::Userspace> =
+        unsafe { drm::Device::from_raw((*raw_obj).dev) };
+
+    let Some(_guard) = dev.registration_guard() else {
+        return;
+    };
+
     // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
-- 
2.55.0

Re: [PATCH 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Bypassing `T::close` when the DRM device is unregistered leaks driver-specific resources and software state allocated in `T::open`.
- [High] Wrapping the entire `open` and `close` callbacks in `RegistrationGuard` introduces a severe AB-BA deadlock hazard with the device unplug path.
--

--- Patch [2]: [PATCH 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard ---
commit 56b4431da0d3b7c0798814690d882352e01b7d71
Author: Danilo Krummrich <dakr@kernel.org>

rust: drm: gem: gate open/close callbacks with RegistrationGuard
    
Wrap the GEM object open and close callbacks with a RegistrationGuard
(drm_dev_enter / drm_dev_exit) to ensure the driver callbacks only run
while the parent bus device is bound.

If the device has been unbound, open returns -ENODEV and close silently
returns. This prevents driver code from accessing device resources after
unbind and is a prerequisite for making drm::Driver::File
lifetime-parameterized.

> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index 80d8f524f9d57..560403ca8e38f 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -128,6 +128,14 @@ extern "C" fn open_callback<T: DriverObject>(
>      raw_obj: *mut bindings::drm_gem_object,
>      raw_file: *mut bindings::drm_file,
>  ) -> core::ffi::c_int {
> +    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
> +    let dev: &drm::Device<T::Driver, drm::Userspace> =
> +        unsafe { drm::Device::from_raw((*raw_obj).dev) };
> +
> +    let Some(_guard) = dev.registration_guard() else {

[Severity: High]
Could this cause an AB-BA deadlock with the device unplug path?

If an unbind thread acquires a driver-specific lock and then calls
drm_dev_unplug(), it will block in synchronize_srcu(&drm_unplug_srcu)
waiting for SRCU read sections to finish.

Concurrently, if a userspace thread opens a GEM handle, this
registration_guard() call takes the drm_unplug_srcu read lock.

When the callback proceeds to call T::open(), the driver might try to
acquire the same driver-specific lock held by the unbind thread, deadlocking
the system.

> +        return ENODEV.to_errno();
> +    };
> +
>      // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
>      let file = unsafe { DriverFile::<T>::from_raw(raw_file) };

[ ... ]

> @@ -148,6 +156,14 @@ extern "C" fn close_callback<T: DriverObject>(
>      raw_obj: *mut bindings::drm_gem_object,
>      raw_file: *mut bindings::drm_file,
>  ) {
> +    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
> +    let dev: &drm::Device<T::Driver, drm::Userspace> =
> +        unsafe { drm::Device::from_raw((*raw_obj).dev) };
> +
> +    let Some(_guard) = dev.registration_guard() else {
> +        return;

[Severity: High]
Is it safe to return early here without calling T::close()?

If a GEM object is created while the device is bound, and the device is then
hot-unplugged, the guard check will fail.

Returning early skips the software cleanup in T::close(), which could
permanently leak driver-specific memory and state (like tracking lists or IDR
entries) allocated during T::open().

> +    };
> +
>      // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
>      let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814230923.1292966-1-dakr@kernel.org?part=2

[PATCH 3/7] rust: drm: move file_operations from gem to device

[Danilo Krummrich]

Move the file_operations definition from drm::gem::create_fops() into
UnregisteredDevice in drm::device. This is the file_operations of the
DRM minor device, set through the drm_driver struct. It is not specific
to GEM.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs  | 22 ++++++++++++++++++++--
 rust/kernel/drm/gem/mod.rs | 20 --------------------
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index be83287fe161..a2940e172073 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -195,10 +195,28 @@ const fn compute_features() -> u32 {
         driver_features: Self::compute_features(),
         ioctls: T::IOCTLS.as_ptr(),
         num_ioctls: T::IOCTLS.len() as i32,
-        fops: &Self::GEM_FOPS,
+        fops: &Self::FOPS,
     };
 
-    const GEM_FOPS: bindings::file_operations = drm::gem::create_fops();
+    const FOPS: bindings::file_operations = {
+        let mut fops: bindings::file_operations = pin_init::zeroed();
+
+        fops.owner = core::ptr::null_mut();
+        fops.open = Some(bindings::drm_open);
+        fops.release = Some(bindings::drm_release);
+        fops.unlocked_ioctl = Some(bindings::drm_ioctl);
+        #[cfg(CONFIG_COMPAT)]
+        {
+            fops.compat_ioctl = Some(bindings::drm_compat_ioctl);
+        }
+        fops.poll = Some(bindings::drm_poll);
+        fops.read = Some(bindings::drm_read);
+        fops.llseek = Some(bindings::noop_llseek);
+        fops.mmap = Some(bindings::drm_gem_mmap);
+        fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET;
+
+        fops
+    };
 
     /// Create a new `UnregisteredDevice` for a `drm::Driver`.
     ///
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 560403ca8e38..ee048febfbdc 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -399,23 +399,3 @@ impl<T: DriverObject, Ctx: DeviceContext> AllocImpl for Object<T, Ctx> {
         dumb_map_offset: None,
     };
 }
-
-pub(super) const fn create_fops() -> bindings::file_operations {
-    let mut fops: bindings::file_operations = pin_init::zeroed();
-
-    fops.owner = core::ptr::null_mut();
-    fops.open = Some(bindings::drm_open);
-    fops.release = Some(bindings::drm_release);
-    fops.unlocked_ioctl = Some(bindings::drm_ioctl);
-    #[cfg(CONFIG_COMPAT)]
-    {
-        fops.compat_ioctl = Some(bindings::drm_compat_ioctl);
-    }
-    fops.poll = Some(bindings::drm_poll);
-    fops.read = Some(bindings::drm_read);
-    fops.llseek = Some(bindings::noop_llseek);
-    fops.mmap = Some(bindings::drm_gem_mmap);
-    fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET;
-
-    fops
-}
-- 
2.55.0

Re: [PATCH 3/7] rust: drm: move file_operations from gem to device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing module reference counting allows driver unload while files are open, leading to Use-After-Free
--

--- Patch [3]: [PATCH 3/7] rust: drm: move file_operations from gem to device ---
commit b811d43b7a27d7b6e1655586be106750c9eefba2
Author: Danilo Krummrich <dakr@kernel.org>

rust: drm: move file_operations from gem to device

Move the file_operations definition from drm::gem::create_fops() into
UnregisteredDevice in drm::device. This is the file_operations of the
DRM minor device, set through the drm_driver struct. It is not specific
to GEM.

> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index be83287fe161d..a2940e172073a 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -195,10 +195,28 @@ const fn compute_features() -> u32 {
>          driver_features: Self::compute_features(),
>          ioctls: T::IOCTLS.as_ptr(),
>          num_ioctls: T::IOCTLS.len() as i32,
> -        fops: &Self::GEM_FOPS,
> +        fops: &Self::FOPS,
>      };
>  
> -    const GEM_FOPS: bindings::file_operations = drm::gem::create_fops();
> +    const FOPS: bindings::file_operations = {
> +        let mut fops: bindings::file_operations = pin_init::zeroed();
> +
> +        fops.owner = core::ptr::null_mut();

[Severity: High]
This is a pre-existing issue, but does setting fops.owner to null_mut()
allow the driver module to be unloaded while file descriptors remain open?

If fops.owner is null, fops_get() in drm_stub_open() won't increment the
driver module's reference count. A user could open the DRM device node,
the module could be unloaded via rmmod (unmapping its .rodata section
where FOPS resides), and subsequent ioctl or close operations would
dereference filp->f_op into unmapped memory, causing a panic.

Should this be updated to use the Rust equivalent of THIS_MODULE
instead of null_mut() to ensure VFS module reference counting works
correctly?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814230923.1292966-1-dakr@kernel.org?part=3

[PATCH 4/7] rust: fs: add iminor() helper

[Danilo Krummrich]

Add a Rust helper for iminor(), which extracts the minor number from a
struct inode. This is a static inline function in the C headers, so
bindgen does not generate a binding for it.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/helpers/fs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/helpers/fs.c b/rust/helpers/fs.c
index 789d60fb8908..d6f9782db178 100644
--- a/rust/helpers/fs.c
+++ b/rust/helpers/fs.c
@@ -10,3 +10,8 @@ __rust_helper struct file *rust_helper_get_file(struct file *f)
 {
 	return get_file(f);
 }
+
+__rust_helper unsigned int rust_helper_iminor(const struct inode *inode)
+{
+	return iminor(inode);
+}
-- 
2.55.0

[PATCH 5/7] rust: drm: wrap fops open with RegistrationGuard

[Danilo Krummrich]

Add a Minor abstraction with RAII release and a fops_open() wrapper that
holds a RegistrationGuard (drm_dev_enter / drm_dev_exit) across the
entire drm_open() call.

This guarantees that drm_dev_unplug() in Registration::drop() waits for
the full open sequence to complete, so all files are visible in the
filelist when iterating for cleanup.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs | 63 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index a2940e172073..09903ed783e1 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -42,6 +42,42 @@
     },
 };
 
+/// A reference to a `struct drm_minor` with RAII release.
+struct Minor(NonNull<bindings::drm_minor>);
+
+// Methods use `#[inline(never)]` to prevent the unexported `drm_minor_acquire()` /
+// `drm_minor_release()` symbols from being inlined into driver modules.
+impl Minor {
+    /// Acquire a minor by ID. Increments the underlying device's refcount.
+    #[inline(never)]
+    fn acquire(minor_id: u32) -> Result<Self> {
+        // SAFETY: `drm_minors_xa` is a valid global xarray; any `minor_id` is safe to
+        // look up (returns ERR_PTR on failure).
+        let ptr =
+            unsafe { bindings::drm_minor_acquire(&raw mut bindings::drm_minors_xa, minor_id) };
+        Ok(Self(NonNull::new(from_err_ptr(ptr)?).ok_or(ENODEV)?))
+    }
+
+    /// Returns a reference to the DRM device for this minor.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the minor belongs to a `Device<T>`.
+    unsafe fn device<T: drm::Driver>(&self) -> &Device<T, Userspace> {
+        // SAFETY: The minor is valid (from `drm_minor_acquire()`) and `minor->dev`
+        // is a valid `drm_device`. The caller guarantees it is a `Device<T>`.
+        unsafe { Device::from_raw((*self.0.as_ptr()).dev) }
+    }
+}
+
+impl Drop for Minor {
+    #[inline(never)]
+    fn drop(&mut self) {
+        // SAFETY: `self.0` came from `drm_minor_acquire()` and has not been released yet.
+        unsafe { bindings::drm_minor_release(self.0.as_ptr()) }
+    }
+}
+
 #[cfg(CONFIG_DRM_LEGACY)]
 macro_rules! drm_legacy_fields {
     ( $($field:ident: $val:expr),* $(,)? ) => {
@@ -198,11 +234,36 @@ const fn compute_features() -> u32 {
         fops: &Self::FOPS,
     };
 
+    /// Wrapper for `fops.open` that holds a [`RegistrationGuard`] across the entire `drm_open()`
+    /// call. This guarantees that `drm_dev_unplug()` in `Registration::drop()` waits for the full
+    /// open sequence.
+    extern "C" fn fops_open(inode: *mut bindings::inode, filp: *mut bindings::file) -> c_int {
+        let f = || -> Result<c_int> {
+            // SAFETY: `inode` is valid.
+            let minor_id = unsafe { bindings::iminor(inode) };
+            let minor = Minor::acquire(minor_id)?;
+
+            // SAFETY: `fops_open` is only installed for devices of type `T` (via `FOPS`).
+            let _guard = (unsafe { minor.device::<T>() })
+                .registration_guard()
+                .ok_or(ENODEV)?;
+
+            // SAFETY: `inode` and `filp` are valid. The RegistrationGuard ensures the entire
+            // `drm_open()` runs within the SRCU critical section.
+            Ok(unsafe { bindings::drm_open(inode, filp) })
+        };
+
+        match f() {
+            Ok(ret) => ret,
+            Err(e) => e.to_errno(),
+        }
+    }
+
     const FOPS: bindings::file_operations = {
         let mut fops: bindings::file_operations = pin_init::zeroed();
 
         fops.owner = core::ptr::null_mut();
-        fops.open = Some(bindings::drm_open);
+        fops.open = Some(Self::fops_open);
         fops.release = Some(bindings::drm_release);
         fops.unlocked_ioctl = Some(bindings::drm_ioctl);
         #[cfg(CONFIG_COMPAT)]
-- 
2.55.0

Re: [PATCH 5/7] rust: drm: wrap fops open with RegistrationGuard

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Re-acquiring the DRM minor by ID in `fops_open` is vulnerable to a TOCTOU race, breaking type safety invariants.
--

--- Patch [5]: [PATCH 5/7] rust: drm: wrap fops open with RegistrationGuard ---
commit f19fe1023e3626e0af6b5b4f7cdaf8f59acc5f98
Author: Danilo Krummrich <dakr@kernel.org>

rust: drm: wrap fops open with RegistrationGuard

Add a Minor abstraction with RAII release and a fops_open() wrapper that
holds a RegistrationGuard (drm_dev_enter / drm_dev_exit) across the
entire drm_open() call. This guarantees that drm_dev_unplug() in 
Registration::drop() waits for the full open sequence to complete.

> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index a2940e172073a..09903ed783e15 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs

[ ... ]

> +    /// Wrapper for `fops.open` that holds a [`RegistrationGuard`] across the entire `drm_open()`
> +    /// call. This guarantees that `drm_dev_unplug()` in `Registration::drop()` waits for the full
> +    /// open sequence.
> +    extern "C" fn fops_open(inode: *mut bindings::inode, filp: *mut bindings::file) -> c_int {
> +        let f = || -> Result<c_int> {
> +            // SAFETY: `inode` is valid.
> +            let minor_id = unsafe { bindings::iminor(inode) };
> +            let minor = Minor::acquire(minor_id)?;
> +
> +            // SAFETY: `fops_open` is only installed for devices of type `T` (via `FOPS`).
> +            let _guard = (unsafe { minor.device::<T>() })

[Severity: High]
Can this sequence cause type confusion if the DRM minor ID is reassigned?

If a device T is unplugged concurrently with the open call, the minor_id
can be freed and reassigned to a different device U.

Since the VFS already resolved filp->f_op to T::fops_open during
drm_stub_open, the following sequence could occur:

1. VFS calls chrdev_open which resolves to drm_stub_open
2. drm_stub_open looks up minor_id, gets device T, and sets filp->f_op to
   T::FOPS
3. Device T is unplugged, and its minor_id is freed
4. Device U is hot-plugged, and drm_minor_alloc reuses the same minor_id
5. T::fops_open is called by VFS and invokes Minor::acquire(minor_id)
6. The returned minor now belongs to device U, not T

When minor.device::<T>() is called here, it unsafely downcasts Device<U>
to Device<T>. This violates Rust's type safety and aliasing invariants.
Is there a way to avoid looking up the minor by ID again to prevent this
race condition?

> +                .registration_guard()
> +                .ok_or(ENODEV)?;
> +
> +            // SAFETY: `inode` and `filp` are valid. The RegistrationGuard ensures the entire
> +            // `drm_open()` runs within the SRCU critical section.
> +            Ok(unsafe { bindings::drm_open(inode, filp) })
> +        };
> +
> +        match f() {
> +            Ok(ret) => ret,
> +            Err(e) => e.to_errno(),
> +        }
> +    }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814230923.1292966-1-dakr@kernel.org?part=5

[PATCH 6/7] rust: drm: make Driver::File lifetime-parameterized

[Danilo Krummrich]

Make the DRM file private data lifetime-parameterized, allowing drivers
to borrow from RegistrationData in their per-file data.

Introduce DriverFile<'a> as a lifetime-parameterized trait that receives
both the device and registration data in open(). Parametrize File<D> on
the driver type rather than the file data type, deriving the concrete
file type through ForLt.

Add inner() for covariant file types and inner_with() for invariant ones
to access the driver file data from a File<D> reference.

Ensure file data is always dropped before registration data:

  - In Registration::drop(), iterate the filelist under filelist_mutex
    and drop driver_priv for all open files, then wait for in-flight
    postclose_callback() calls to complete via an open_count / WaitQueue
    pair on drm::Device.

  - In postclose_callback(), skip the drop if driver_priv has already
    been NULLed by the filelist iteration. The NULL write is visible
    through the filelist_mutex acquire/release chain in
    drm_close_helper().

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/drm/nova/driver.rs |   5 +-
 drivers/gpu/drm/nova/file.rs   |  11 ++-
 drivers/gpu/drm/nova/gem.rs    |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |   5 +-
 drivers/gpu/drm/tyr/file.rs    |  11 ++-
 rust/kernel/drm/device.rs      |  35 ++++++--
 rust/kernel/drm/driver.rs      |  69 ++++++++++++++-
 rust/kernel/drm/file.rs        | 150 ++++++++++++++++++++++-----------
 rust/kernel/drm/gem/mod.rs     |  12 ++-
 rust/kernel/drm/gem/shmem.rs   |  12 ++-
 rust/kernel/drm/ioctl.rs       |   2 +-
 11 files changed, 231 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 739690bc2db5..a8f61086773d 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::CovariantForLt, //
 };
 
 use crate::file::File;
@@ -75,7 +76,7 @@ fn probe<'bound>(
 impl drm::Driver for NovaDriver {
     type Data = ();
     type RegistrationData<'a> = ();
-    type File = File;
+    type File = CovariantForLt!(File);
     type Object = gem::Object<NovaObject>;
     type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
 
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 298c02bacb4b..1f94201af92b 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -3,7 +3,6 @@
 use crate::driver::{NovaDevice, NovaDriver};
 use crate::gem::NovaObject;
 use kernel::{
-    alloc::flags::*,
     auxiliary,
     device::Bound,
     drm::{
@@ -18,10 +17,10 @@
 
 pub(crate) struct File;
 
-impl drm::file::DriverFile for File {
+impl drm::file::DriverFile<'_> for File {
     type Driver = NovaDriver;
 
-    fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
+    fn open(_device: &NovaDevice<Registered>, _reg_data: &()) -> Result<Pin<KBox<Self>>> {
         Ok(KBox::new(Self, GFP_KERNEL)?.into())
     }
 }
@@ -32,7 +31,7 @@ pub(crate) fn get_param(
         dev: &NovaDevice<Registered>,
         _reg_data: &(),
         getparam: &mut uapi::drm_nova_getparam,
-        _file: &drm::File<File>,
+        _file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let adev: &auxiliary::Device<Bound> = dev.as_ref();
         let pdev: &pci::Device<Bound> = adev.parent().try_into()?;
@@ -52,7 +51,7 @@ pub(crate) fn gem_create(
         dev: &NovaDevice<Registered>,
         _reg_data: &(),
         req: &mut uapi::drm_nova_gem_create,
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let obj = NovaObject::new(dev, req.size.try_into()?)?;
 
@@ -66,7 +65,7 @@ pub(crate) fn gem_info(
         _dev: &NovaDevice<Registered>,
         _reg_data: &(),
         req: &mut uapi::drm_nova_gem_info,
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let bo = NovaObject::lookup_handle(file, req.handle)?;
 
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index 2b6fe9dc0bfa..2a21ff8ba579 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -11,9 +11,9 @@
     sync::aref::ARef,
 };
 
-use crate::{
-    driver::{NovaDevice, NovaDriver},
-    file::File,
+use crate::driver::{
+    NovaDevice,
+    NovaDriver, //
 };
 
 /// GEM Object inner driver data
@@ -43,7 +43,7 @@ pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self
     /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it.
     #[inline]
     pub(crate) fn lookup_handle(
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
         handle: u32,
     ) -> Result<ARef<gem::Object<Self>>> {
         gem::Object::lookup_handle(file, handle)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index d78ad9d292ff..94bc85635725 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -32,7 +32,8 @@
         Arc,
         Mutex, //
     },
-    time, //
+    time,
+    types::CovariantForLt, //
 };
 
 use crate::{
@@ -206,7 +207,7 @@ fn drop(self: Pin<&mut Self>) {}
 impl drm::Driver for TyrDrmDriver {
     type Data = ();
     type RegistrationData<'drm> = TyrDrmRegistrationData<'drm>;
-    type File = TyrDrmFileData;
+    type File = CovariantForLt!(TyrDrmFileData);
     type Object = Bo;
     type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
 
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 9f60a90d4948..0e0878090de6 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -19,13 +19,16 @@
 #[pin_data]
 pub(crate) struct TyrDrmFileData {}
 
-/// Convenience type alias for our DRM `File` type
-pub(crate) type TyrDrmFile = drm::file::File<TyrDrmFileData>;
+/// Convenience type alias for our DRM `File` type.
+pub(crate) type TyrDrmFile = drm::file::File<TyrDrmDriver>;
 
-impl drm::file::DriverFile for TyrDrmFileData {
+impl drm::file::DriverFile<'_> for TyrDrmFileData {
     type Driver = TyrDrmDriver;
 
-    fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
+    fn open(
+        _device: &TyrDrmDevice<Registered>,
+        _reg_data: &TyrDrmRegistrationData<'_>,
+    ) -> Result<Pin<KBox<Self>>> {
         KBox::try_pin_init(try_pin_init!(Self {}), GFP_KERNEL)
     }
 }
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 09903ed783e1..29512d0e2ddb 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -15,11 +15,16 @@
     },
     error::from_err_ptr,
     prelude::*,
-    sync::aref::{
-        ARef,
-        AlwaysRefCounted, //
+    sync::{
+        aref::{
+            ARef,
+            AlwaysRefCounted, //
+        },
+        atomic::Atomic,
+        WaitQueue,
     },
     types::{
+        ForLt,
         NotThreadSafe,
         Opaque, //
     },
@@ -190,7 +195,10 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: drm::Driver> UnregisteredDevice<T> {
+impl<T: drm::Driver> UnregisteredDevice<T>
+where
+    for<'a> <T::File as ForLt>::Of<'a>: drm::file::DriverFile<'a, Driver = T>,
+{
     const fn compute_features() -> u32 {
         let mut features = drm::driver::FEAT_GEM;
 
@@ -203,8 +211,8 @@ const fn compute_features() -> u32 {
 
     const VTABLE: bindings::drm_driver = drm_legacy_fields! {
         load: None,
-        open: Some(drm::File::<T::File>::open_callback),
-        postclose: Some(drm::File::<T::File>::postclose_callback),
+        open: Some(drm::File::<T>::open_callback),
+        postclose: Some(drm::File::<T>::postclose_callback),
         unload: None,
         release: Some(Device::<T>::release),
         master_set: None,
@@ -333,6 +341,19 @@ pub fn new(
         // SAFETY: `raw_drm` is valid; no concurrent access before registration.
         unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
 
+        // SAFETY: `raw_drm` is valid; no concurrent access before registration.
+        unsafe { (*raw_drm.as_ptr()).open_count = Atomic::new(0) };
+
+        // SAFETY:
+        // - `raw_drm` is valid; no concurrent access before registration.
+        // - The field is pinned because the Device is pinned (refcounted, allocated by
+        //   `__drm_dev_alloc()`, never moved).
+        // - The init is infallible.
+        let Ok(()) = unsafe {
+            crate::new_waitqueue!("drm_open_count")
+                .__pinned_init(&raw mut (*raw_drm.as_ptr()).open_count_wq)
+        };
+
         // 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`.
@@ -357,6 +378,8 @@ 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<'static>>>,
+    pub(super) open_count: Atomic<i32>,
+    pub(super) open_count_wq: WaitQueue,
     _ctx: PhantomData<C>,
 }
 
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 74f6ed690d8b..2aa534149d78 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -9,8 +9,17 @@
     device,
     drm,
     error::to_result,
+    interop::list::clist_create,
     prelude::*,
-    sync::aref::ARef, //
+    sync::{
+        aref::ARef,
+        atomic::{
+            Acquire,
+            Release, //
+        },
+        Mutex, //
+    },
+    types::ForLt, //
 };
 use core::ptr::NonNull;
 
@@ -117,8 +126,15 @@ pub trait Driver {
     /// The type used to manage memory for this driver.
     type Object: AllocImpl;
 
-    /// The type used to represent a DRM File (client)
-    type File: drm::file::DriverFile;
+    /// The type used to represent a DRM File (client).
+    ///
+    /// File data may borrow from [`RegistrationData`](Driver::RegistrationData). File data is
+    /// guaranteed to be dropped before registration data, either when the file is closed or
+    /// when the device is unregistered, whichever comes first.
+    ///
+    /// Drivers set this to `CovariantForLt!(MyFileData)` (or `ForLt!` for invariant types)
+    /// and implement [`DriverFile`](drm::file::DriverFile) for their file data type.
+    type File: ForLt + 'static;
 
     /// The bus device type of the parent device that the DRM device is associated with.
     type ParentDevice<Ctx: device::DeviceContext>: device::AsBusDevice<Ctx>;
@@ -221,6 +237,51 @@ fn drop(&mut self) {
         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.
+
+        // Drop all remaining file private data before dropping registration data. This guarantees
+        // that file data (which may borrow from RegistrationData) is always dropped first.
+        let raw = self.drm.as_raw();
+
+        // SAFETY: `filelist_mutex` is initialized by `drm_dev_init()` and remains valid for
+        // the lifetime of the `struct drm_device`.
+        let filelist_mutex = unsafe { Mutex::from_raw(&raw mut (*raw).filelist_mutex) };
+        {
+            let _guard = filelist_mutex.lock();
+
+            // SAFETY: `filelist` is a valid, initialized sentinel `list_head`; the mutex
+            // guard prevents concurrent modification.
+            let filelist = unsafe {
+                clist_create!(
+                    &raw mut (*raw).filelist,
+                    drm::File<T>,
+                    bindings::drm_file,
+                    lhead
+                )
+            };
+
+            for file in filelist.iter() {
+                // SAFETY: `file` is a valid `drm_file` on this device's filelist.
+                let priv_ptr = unsafe { (*file.as_raw()).driver_priv };
+
+                // SAFETY: Setting `driver_priv` to NULL is visible to `postclose_callback()`
+                // through the `filelist_mutex` acquire/release chain in `drm_close_helper()`.
+                unsafe { (*file.as_raw()).driver_priv = core::ptr::null_mut() };
+
+                // SAFETY: `driver_priv` was created by `open_callback()` via `KBox::into_raw` and
+                // has not yet been freed (the file is still in the list, so `postclose_callback()`
+                // has not run).
+                drop(unsafe { KBox::from_raw(priv_ptr.cast::<<T::File as ForLt>::Of<'static>>()) });
+
+                self.drm.open_count.fetch_sub(1, Release);
+            }
+        }
+
+        // Wait for in-flight `postclose_callback()` calls to complete. After `drm_dev_unplug()`, no
+        // new opens can succeed, so `open_count` is monotonically decreasing.
+        self.drm
+            .open_count_wq
+            .wait_event(|| self.drm.open_count.load(Acquire) == 0);
+
+        // `_reg_data` is dropped here automatically, after all file data has been dropped.
     }
 }
diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
index 10160601ce5a..6491ec5707a0 100644
--- a/rust/kernel/drm/file.rs
+++ b/rust/kernel/drm/file.rs
@@ -8,17 +8,34 @@
     bindings,
     drm,
     prelude::*,
-    types::Opaque, //
+    sync::atomic::{
+        Relaxed,
+        Release, //
+    },
+    types::{
+        CovariantForLt,
+        ForLt,
+        Opaque, //
+    }, //
 };
 use core::marker::PhantomData;
 
 /// Trait that must be implemented by DRM drivers to represent a DRM File (a client instance).
-pub trait DriverFile {
+///
+/// The lifetime `'a` allows the file data to borrow from
+/// [`RegistrationData`](drm::Driver::RegistrationData).
+pub trait DriverFile<'a>: Sized {
     /// The parent `Driver` implementation for this `DriverFile`.
     type Driver: drm::Driver;
 
-    /// Open a new file (called when a client opens the DRM device).
-    fn open(device: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>>;
+    /// Open a new DRM file, creating the per-file driver data.
+    ///
+    /// Called when a client opens the DRM device. The returned file data may borrow from
+    /// `reg_data` with lifetime `'a`.
+    fn open(
+        device: &drm::Device<Self::Driver, drm::Registered>,
+        reg_data: &'a <Self::Driver as drm::Driver>::RegistrationData<'a>,
+    ) -> Result<Pin<KBox<Self>>>;
 }
 
 /// An open DRM File.
@@ -27,17 +44,17 @@ pub trait DriverFile {
 ///
 /// `self.0` is a valid instance of a `struct drm_file`.
 #[repr(transparent)]
-pub struct File<T: DriverFile>(Opaque<bindings::drm_file>, PhantomData<T>);
+pub struct File<D: drm::Driver>(Opaque<bindings::drm_file>, PhantomData<D>);
 
-impl<T: DriverFile> File<T> {
+impl<D: drm::Driver> File<D> {
     #[doc(hidden)]
     /// Not intended to be called externally, except via declare_drm_ioctls!()
     ///
     /// # Safety
     ///
-    /// `raw_file` must be a valid pointer to an open `struct drm_file`, opened through `T::open`.
-    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_file) -> &'a File<T> {
-        // SAFETY: `raw_file` is valid by the safety requirements of this function.
+    /// `ptr` must be a valid pointer to an open `struct drm_file`.
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_file) -> &'a File<D> {
+        // SAFETY: `ptr` is valid by the safety requirements of this function.
         unsafe { &*ptr.cast() }
     }
 
@@ -45,59 +62,94 @@ pub(super) fn as_raw(&self) -> *mut bindings::drm_file {
         self.0.get()
     }
 
-    fn driver_priv(&self) -> *mut T {
-        // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid.
-        unsafe { (*self.as_raw()).driver_priv }.cast()
+    /// Return a pinned reference to the driver file data.
+    ///
+    /// Only available when `D::File` implements [`trait@CovariantForLt`]. For invariant types, use
+    /// [`inner_with()`](Self::inner_with).
+    pub fn inner(&self) -> Pin<&<D::File as ForLt>::Of<'_>>
+    where
+        D::File: CovariantForLt,
+    {
+        // SAFETY: `driver_priv` was initialized by `open_callback()`. `CovariantForLt` guarantees
+        // the lifetime shortening from `'static` to `'_` is sound.
+        unsafe { Pin::new_unchecked(&*(*self.as_raw()).driver_priv.cast_const().cast()) }
     }
 
-    /// Return a pinned reference to the driver file structure.
-    pub fn inner(&self) -> Pin<&T> {
-        // SAFETY: By the type invariant the pointer `self.as_raw()` points to a valid and opened
-        // `struct drm_file`, hence `driver_priv` has been properly initialized by `open_callback`.
-        unsafe { Pin::new_unchecked(&*(self.driver_priv())) }
+    /// Access the driver file data through a closure.
+    ///
+    /// This works for all file data types, including invariant ones. For covariant types,
+    /// [`inner()`](Self::inner) provides direct access without a closure.
+    pub fn inner_with<R, F>(&self, f: F) -> R
+    where
+        F: for<'a> FnOnce(Pin<&'a <D::File as ForLt>::Of<'a>>) -> R,
+    {
+        // SAFETY: `driver_priv` was initialized by `open_callback()`. The HRTB `for<'a>` prevents
+        // the caller from choosing a concrete lifetime, making the lifetime shortening sound
+        // regardless of variance.
+        f(unsafe { Pin::new_unchecked(&*(*self.as_raw()).driver_priv.cast_const().cast()) })
     }
 
     /// The open callback of a `struct drm_file`.
+    ///
+    /// Called from `drm_open()`, which is itself called from `fops_open()`. The latter holds a
+    /// `RegistrationGuard`, so the device is guaranteed to be registered for the duration of this
+    /// callback.
     pub(crate) extern "C" fn open_callback(
         raw_dev: *mut bindings::drm_device,
         raw_file: *mut bindings::drm_file,
-    ) -> core::ffi::c_int {
-        // SAFETY: A callback from `struct drm_driver::open` guarantees that
-        // - `raw_dev` is valid pointer to a `struct drm_device`,
-        // - the corresponding `struct drm_device` has been registered.
-        let drm = unsafe { drm::Device::from_raw(raw_dev) };
-
-        // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
-        let file = unsafe { File::<T>::from_raw(raw_file) };
-
-        let inner = match T::open(drm) {
-            Err(e) => {
-                return e.to_errno();
-            }
-            Ok(i) => i,
-        };
-
-        // SAFETY: This pointer is treated as pinned, and the Drop guarantee is upheld in
-        // `postclose_callback()`.
-        let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) });
-
-        // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid.
-        unsafe { (*file.as_raw()).driver_priv = driver_priv.cast() };
-
-        0
+    ) -> core::ffi::c_int
+    where
+        for<'a> <D::File as ForLt>::Of<'a>: DriverFile<'a, Driver = D>,
+    {
+        // SAFETY: The DRM core guarantees that `raw_dev` is valid. `fops_open()` holds a
+        // `RegistrationGuard`, so the device is registered and the `Registered` context holds.
+        let dev: &drm::device::Device<D, drm::Registered> =
+            unsafe { drm::device::Device::from_raw(raw_dev) };
+
+        dev.registration_data_with(|reg_data| {
+            let inner = match <<D::File as ForLt>::Of<'_> as DriverFile<'_>>::open(dev, reg_data) {
+                Err(e) => return e.to_errno(),
+                Ok(i) => i,
+            };
+
+            // SAFETY: This pointer is treated as pinned, and the Drop guarantee is upheld in
+            // `postclose_callback()` or the filelist iteration in `Registration::drop()`.
+            let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) });
+
+            dev.open_count.fetch_add(1, Relaxed);
+
+            // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
+            unsafe { (*raw_file).driver_priv = driver_priv.cast() };
+
+            0
+        })
     }
 
     /// The postclose callback of a `struct drm_file`.
     pub(crate) extern "C" fn postclose_callback(
-        _raw_dev: *mut bindings::drm_device,
+        raw_dev: *mut bindings::drm_device,
         raw_file: *mut bindings::drm_file,
-    ) {
-        // SAFETY: This reference won't escape this function
-        let file = unsafe { File::<T>::from_raw(raw_file) };
-
-        // SAFETY: `file.driver_priv` has been created in `open_callback` through `KBox::into_raw`.
-        let _ = unsafe { KBox::from_raw(file.driver_priv()) };
+    ) where
+        for<'a> <D::File as ForLt>::Of<'a>: DriverFile<'a, Driver = D>,
+    {
+        // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
+        let driver_priv = unsafe { (*raw_file).driver_priv };
+
+        if driver_priv.is_null() {
+            return;
+        }
+
+        // SAFETY: `driver_priv` was created in `open_callback()` through `KBox::into_raw` and has
+        // not been dropped yet (the NULL check above guards against double-free from the filelist
+        // iteration in `Registration::drop()`).
+        let _ = unsafe { KBox::from_raw(driver_priv.cast::<<D::File as ForLt>::Of<'static>>()) };
+
+        // SAFETY: `raw_dev` is valid for the lifetime of the `struct drm_file`.
+        let dev: &drm::device::Device<D> = unsafe { drm::device::Device::from_raw(raw_dev) };
+        if dev.open_count.fetch_sub(1, Release) == 1 {
+            dev.open_count_wq.wake_up();
+        }
     }
 }
 
-impl<T: DriverFile> super::private::Sealed for File<T> {}
+impl<D: drm::Driver> super::private::Sealed for File<D> {}
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index ee048febfbdc..1aa18f0b172c 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -76,7 +76,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
 ///
 /// [`Driver`]: drm::Driver
 /// [`DriverFile`]: drm::file::DriverFile
-pub type DriverFile<T> = drm::File<<<T as DriverObject>::Driver as drm::Driver>::File>;
+pub type DriverFile<T> = drm::File<<T as DriverObject>::Driver>;
 
 /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
 ///
@@ -196,11 +196,10 @@ fn size(&self) -> usize {
 
     /// Creates a new handle for the object associated with a given `File`
     /// (or returns an existing one).
-    fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
+    fn create_handle<D>(&self, file: &drm::File<D>) -> Result<u32>
     where
         Self: AllocImpl<Driver = D>,
-        D: drm::Driver<Object = Self, File = F>,
-        F: drm::file::DriverFile<Driver = D>,
+        D: drm::Driver<Object = Self>,
     {
         let mut handle: u32 = 0;
         // SAFETY: The arguments are all valid per the type invariants.
@@ -211,11 +210,10 @@ 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>>
+    fn lookup_handle<D>(file: &drm::File<D>, handle: u32) -> Result<ARef<Self>>
     where
         Self: AllocImpl<Driver = D> + AlwaysRefCounted,
-        D: drm::Driver<Object = Self, File = F>,
-        F: drm::file::DriverFile<Driver = D>,
+        D: drm::Driver<Object = Self>,
     {
         // SAFETY: The arguments are all valid per the type invariants.
         let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) };
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index a687d46d170d..793b4883de3a 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -584,7 +584,8 @@ mod tests {
         },
         faux,
         io::Io,
-        page::PAGE_SIZE, //
+        page::PAGE_SIZE,
+        types::CovariantForLt, //
     };
 
     // The bare minimum needed to create a fake drm driver for kunit
@@ -604,10 +605,13 @@ struct KunitObject {}
         desc: c"Kunit",
     };
 
-    impl drm::file::DriverFile for KunitFile {
+    impl drm::file::DriverFile<'_> for KunitFile {
         type Driver = KunitDriver;
 
-        fn open(_dev: &drm::Device<KunitDriver>) -> Result<Pin<KBox<Self>>> {
+        fn open(
+            _device: &drm::Device<KunitDriver, drm::Registered>,
+            _reg_data: &(),
+        ) -> Result<Pin<KBox<Self>>> {
             Ok(KBox::new(Self, GFP_KERNEL)?.into())
         }
     }
@@ -629,7 +633,7 @@ fn new(
     impl drm::Driver for KunitDriver {
         type Data = KunitData;
         type RegistrationData<'a> = ();
-        type File = KunitFile;
+        type File = CovariantForLt!(KunitFile);
         type Object = Object<KunitObject>;
         type ParentDevice<Ctx: device::DeviceContext> = faux::Device<Ctx>;
 
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 9934b23c36eb..8ac7fb1d4ec2 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -97,7 +97,7 @@ pub const fn __dev_ctx_cast<T: crate::drm::Driver>(
 /// fn foo(device: &kernel::drm::Device<Self, kernel::drm::Registered>,
 ///        reg_data: &Self::RegistrationData<'_>,
 ///        data: &mut uapi::argument_type,
-///        file: &kernel::drm::File<Self::File>,
+///        file: &kernel::drm::File<Self>,
 /// ) -> Result<u32>
 /// ```
 /// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within.
-- 
2.55.0

Re: [PATCH 6/7] rust: drm: make Driver::File lifetime-parameterized

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Lock inversion in Registration::drop() when freeing file data under filelist_mutex
- [Low] Missing #[inline] annotations on small abstraction functions
--

--- Patch [6]: [PATCH 6/7] rust: drm: make Driver::File lifetime-parameterized ---
commit f0644f05187ec5973de59ac9760eaeeee5140b6a
Author: Danilo Krummrich <dakr@kernel.org>

rust: drm: make Driver::File lifetime-parameterized

Make the DRM file private data lifetime-parameterized, allowing drivers
to borrow from RegistrationData in their per-file data.

> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
[ ... ]
> @@ -221,6 +237,51 @@ fn drop(&mut self) {
>          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.
> +
> +        // Drop all remaining file private data before dropping registration data. This guarantees
> +        // that file data (which may borrow from RegistrationData) is always dropped first.
> +        let raw = self.drm.as_raw();
> +
> +        // SAFETY: `filelist_mutex` is initialized by `drm_dev_init()` and remains valid for
> +        // the lifetime of the `struct drm_device`.
> +        let filelist_mutex = unsafe { Mutex::from_raw(&raw mut (*raw).filelist_mutex) };
> +        {
> +            let _guard = filelist_mutex.lock();
> +
> +            // SAFETY: `filelist` is a valid, initialized sentinel `list_head`; the mutex
> +            // guard prevents concurrent modification.
> +            let filelist = unsafe {
> +                clist_create!(
> +                    &raw mut (*raw).filelist,
> +                    drm::File<T>,
> +                    bindings::drm_file,
> +                    lhead
> +                )
> +            };
> +
> +            for file in filelist.iter() {
> +                // SAFETY: `file` is a valid `drm_file` on this device's filelist.
> +                let priv_ptr = unsafe { (*file.as_raw()).driver_priv };
> +
> +                // SAFETY: Setting `driver_priv` to NULL is visible to `postclose_callback()`
> +                // through the `filelist_mutex` acquire/release chain in `drm_close_helper()`.
> +                unsafe { (*file.as_raw()).driver_priv = core::ptr::null_mut() };
> +
> +                // SAFETY: `driver_priv` was created by `open_callback()` via `KBox::into_raw` and
> +                // has not yet been freed (the file is still in the list, so `postclose_callback()`
> +                // has not run).
> +                drop(unsafe { KBox::from_raw(priv_ptr.cast::<<T::File as ForLt>::Of<'static>>()) });

[Severity: High]
Does dropping the file data while holding `filelist_mutex` introduce a lock
inversion risk?

If the driver's File Drop implementation acquires a driver-specific lock, this
creates a `filelist_mutex` -> driver lock dependency.

Since VFS operations routinely take driver locks before the DRM core takes
`filelist_mutex` (e.g., in `drm_file_free`), this could violate core kernel
locking rules and cause an AB-BA deadlock on device unplug.

> +
> +                self.drm.open_count.fetch_sub(1, Release);
> +            }
> +        }
> +
> +        // Wait for in-flight `postclose_callback()` calls to complete. After `drm_dev_unplug()`, no
> +        // new opens can succeed, so `open_count` is monotonically decreasing.
> +        self.drm
> +            .open_count_wq
> +            .wait_event(|| self.drm.open_count.load(Acquire) == 0);
> +
> +        // `_reg_data` is dropped here automatically, after all file data has been dropped.
>      }
>  }
> diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
> --- a/rust/kernel/drm/file.rs
> +++ b/rust/kernel/drm/file.rs
[ ... ]
> @@ -45,59 +62,94 @@ pub(super) fn as_raw(&self) -> *mut bindings::drm_file {
>          self.0.get()
>      }
>  
> -    fn driver_priv(&self) -> *mut T {
> -        // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid.
> -        unsafe { (*self.as_raw()).driver_priv }.cast()
> +    /// Return a pinned reference to the driver file data.
> +    ///
> +    /// Only available when `D::File` implements [`trait@CovariantForLt`]. For invariant types, use
> +    /// [`inner_with()`](Self::inner_with).
> +    pub fn inner(&self) -> Pin<&<D::File as ForLt>::Of<'_>>
> +    where
> +        D::File: CovariantForLt,

[Severity: Low]
This isn't a bug, but shouldn't this small accessor abstraction function be
annotated with #[inline]?

> +    {
> +        // SAFETY: `driver_priv` was initialized by `open_callback()`. `CovariantForLt` guarantees
> +        // the lifetime shortening from `'static` to `'_` is sound.
> +        unsafe { Pin::new_unchecked(&*(*self.as_raw()).driver_priv.cast_const().cast()) }
>      }
>  
> -    /// Return a pinned reference to the driver file structure.
> -    pub fn inner(&self) -> Pin<&T> {
> -        // SAFETY: By the type invariant the pointer `self.as_raw()` points to a valid and opened
> -        // `struct drm_file`, hence `driver_priv` has been properly initialized by `open_callback`.
> -        unsafe { Pin::new_unchecked(&*(self.driver_priv())) }
> +    /// Access the driver file data through a closure.
> +    ///
> +    /// This works for all file data types, including invariant ones. For covariant types,
> +    /// [`inner()`](Self::inner) provides direct access without a closure.
> +    pub fn inner_with<R, F>(&self, f: F) -> R
> +    where
> +        F: for<'a> FnOnce(Pin<&'a <D::File as ForLt>::Of<'a>>) -> R,

[Severity: Low]
This isn't a bug, but shouldn't this abstraction function also be annotated
with #[inline]?

> +    {
> +        // SAFETY: `driver_priv` was initialized by `open_callback()`. The HRTB `for<'a>` prevents
> +        // the caller from choosing a concrete lifetime, making the lifetime shortening sound
> +        // regardless of variance.
> +        f(unsafe { Pin::new_unchecked(&*(*self.as_raw()).driver_priv.cast_const().cast()) })
>      }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814230923.1292966-1-dakr@kernel.org?part=6

[PATCH 7/7] rust: drm: return impl PinInit from DriverFile::open()

[Danilo Krummrich]

Change DriverFile::open() to return impl PinInit<Self, Error> instead of
Result<Pin<KBox<Self>>>, consistent with how bus device private data
works.

Drivers no longer need to allocate a Pin<KBox<_>> themselves; they just
return an initializer and the subsystem takes care of the allocation.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/drm/nova/file.rs | 4 ++--
 drivers/gpu/drm/tyr/file.rs  | 4 ++--
 rust/kernel/drm/file.rs      | 7 ++++---
 rust/kernel/drm/gem/shmem.rs | 4 ++--
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 1f94201af92b..30bbabe6ee78 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -20,8 +20,8 @@
 impl drm::file::DriverFile<'_> for File {
     type Driver = NovaDriver;
 
-    fn open(_device: &NovaDevice<Registered>, _reg_data: &()) -> Result<Pin<KBox<Self>>> {
-        Ok(KBox::new(Self, GFP_KERNEL)?.into())
+    fn open(_device: &NovaDevice<Registered>, _reg_data: &()) -> impl PinInit<Self, Error> {
+        Ok(Self)
     }
 }
 
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 0e0878090de6..933a365cb016 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -28,8 +28,8 @@ impl drm::file::DriverFile<'_> for TyrDrmFileData {
     fn open(
         _device: &TyrDrmDevice<Registered>,
         _reg_data: &TyrDrmRegistrationData<'_>,
-    ) -> Result<Pin<KBox<Self>>> {
-        KBox::try_pin_init(try_pin_init!(Self {}), GFP_KERNEL)
+    ) -> impl PinInit<Self, Error> {
+        Ok(Self {})
     }
 }
 
diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
index 6491ec5707a0..31fc318eb535 100644
--- a/rust/kernel/drm/file.rs
+++ b/rust/kernel/drm/file.rs
@@ -30,12 +30,12 @@ pub trait DriverFile<'a>: Sized {
 
     /// Open a new DRM file, creating the per-file driver data.
     ///
-    /// Called when a client opens the DRM device. The returned file data may borrow from
+    /// Called when a client opens the DRM device. The returned initializer may borrow from
     /// `reg_data` with lifetime `'a`.
     fn open(
         device: &drm::Device<Self::Driver, drm::Registered>,
         reg_data: &'a <Self::Driver as drm::Driver>::RegistrationData<'a>,
-    ) -> Result<Pin<KBox<Self>>>;
+    ) -> impl PinInit<Self, Error>;
 }
 
 /// An open DRM File.
@@ -107,7 +107,8 @@ pub(crate) extern "C" fn open_callback(
             unsafe { drm::device::Device::from_raw(raw_dev) };
 
         dev.registration_data_with(|reg_data| {
-            let inner = match <<D::File as ForLt>::Of<'_> as DriverFile<'_>>::open(dev, reg_data) {
+            let init = <<D::File as ForLt>::Of<'_> as DriverFile<'_>>::open(dev, reg_data);
+            let inner = match KBox::try_pin_init(init, GFP_KERNEL) {
                 Err(e) => return e.to_errno(),
                 Ok(i) => i,
             };
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index 793b4883de3a..d882b505fe0f 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -611,8 +611,8 @@ impl drm::file::DriverFile<'_> for KunitFile {
         fn open(
             _device: &drm::Device<KunitDriver, drm::Registered>,
             _reg_data: &(),
-        ) -> Result<Pin<KBox<Self>>> {
-            Ok(KBox::new(Self, GFP_KERNEL)?.into())
+        ) -> impl PinInit<Self, Error> {
+            Ok(Self)
         }
     }
 
-- 
2.55.0