Re: [PATCH v4 08/16] rust: drm: pin ioctl Device reference to Normal context

[sashiko-bot@kernel.org]

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