Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Wrapping the entire DRM ioctl dispatch in a RegistrationGuard holds the SRCU read lock (drm_dev_enter) for the entire duration of the ioctl handler, which will deadlock device hot-unplug. --
commit e380f2be4330be05befae4edf318decdc7aa75d1 Author: Danilo Krummrich <[email protected]> rust: drm: Wrap ioctl dispatch in RegistrationGuard This commit wraps the DRM ioctl dispatch in registration_guard() to ensure that handlers only receive a reference to a registered device. If the device has already been unplugged, it intercepts the call and returns -ENODEV. > diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs > index 6f5a9877bdae0..f24cf7b146a8e 100644 > --- a/rust/kernel/drm/ioctl.rs > +++ b/rust/kernel/drm/ioctl.rs [ ... ] > @@ -131,17 +143,44 @@ macro_rules! declare_drm_ioctls { [ ... ] > + let Some(guard) = dev.registration_guard() else { > + return > $crate::error::code::ENODEV.to_errno(); > + }; > + > // 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. > @@ -154,7 +193,7 @@ macro_rules! declare_drm_ioctls { > // SAFETY: This is just the DRM file structure > let file = unsafe { > $crate::drm::File::from_raw(raw_file) }; > > - match $func(dev, data, file) { > + match $func(&*guard, data, file) { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] Will this deadlock device hot-unplug if the ioctl handler performs an unbounded wait? The RegistrationGuard acquires the SRCU read lock via drm_dev_enter(). Because this guard is held across the entire execution of $func, any unbounded wait inside the ioctl (such as waiting for a hardware fence) will hold the SRCU read-side critical section indefinitely. If the device is hot-unplugged during this wait, the driver's remove callback calls drm_dev_unplug(). This blocks on synchronize_srcu() waiting for the ioctl to drop the lock. The ioctl will remain blocked forever because the stalled remove callback cannot force-signal the fences, leading to a permanent hang. > 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/[email protected]?part=13
