Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Changing the IOCTL handler signature to accept `&Device<Registered>` forces the `declare_drm_ioctls!` macro to unsafely assert registration invariants before verifying them. --
commit ae89d443b44234dbccc8b42ac767d5eb9bdfb248 Author: Danilo Krummrich <[email protected]> drm: nova: Use drm::Device<Registered> to access the parent bus device This commit modifies the get_param ioctl handler to accept a &NovaDevice<Registered> instead of &NovaDevice, avoiding the need for storing device data. > diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs > index 4c9fa409a8be8..e0e79b5719836 100644 > --- a/drivers/gpu/drm/nova/file.rs > +++ b/drivers/gpu/drm/nova/file.rs [ ... ] > @@ -23,14 +29,13 @@ 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>, [Severity: High] Does changing the ioctl handler signature to accept &Device<Registered> cause the declare_drm_ioctls! macro to unsafely assert registration invariants before verifying them? The declare_drm_ioctls! macro instantiates a reference to the DRM device using Device::from_raw(raw_dev). Because of the type-inference anchor let _ = || { $func(dev, ...) }, changing the first parameter to &NovaDevice<Registered> causes from_raw to be invoked with the Registered context type. The safety contract of from_raw requires callers to promise that any type invariants of the generic context C are upheld. The Registered context guarantees that the DRM device is currently registered and its parent bus device is bound. However, the macro calls from_raw before acquiring the RegistrationGuard, which verifies via drm_dev_enter that the device hasn't been unplugged: rust/kernel/drm/ioctl.rs:declare_drm_ioctls!() { ... let dev = $crate::drm::device::Device::from_raw(raw_dev); ... } If the device is concurrently unplugged between drm_ioctl's initial check and from_raw, the invariant is violated, which breaks the from_raw safety contract and could cause undefined behavior. Would it be safer for the handler to continue accepting &NovaDevice (the Normal context) and call dev.registration_guard() internally to obtain a Registered reference? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
