Re: [PATCH v3 13/13] drm: nova: Use drm::Device<Registered> to access the parent bus device

[sashiko-bot@kernel.org]

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 <dakr@kernel.org>

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/20260620005431.1562115-1-dakr@kernel.org?part=13