Re: [PATCH v4 15/16] rust: drm: Pass registration data to ioctl handlers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `declare_drm_ioctls!` macro fails to constrain the `Data` type parameter in `__call_ioctl`, allowing type inference to deduce an arbitrarily large struct size that leads to out-of-bounds memory accesses.
--

commit d383776ed6368e3f51c2779206ce9abab5ecfc2d
Author: Danilo Krummrich <dakr@kernel.org>

rust: drm: Pass registration data to ioctl handlers

This commit passes registration data to ioctl handlers via
drm::Device<Registered>::registration_data_with(). It modifies the ioctl
dispatching macro declare_drm_ioctls! and the internal __call_ioctl helper
to thread the registration data through to the ioctl callbacks.

> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index 6cefd26b31f91..75944e72d7af4 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs

[ ... ]

> @@ -202,25 +204,28 @@ macro_rules! declare_drm_ioctls {
>                                      unsafe { &*__ptr },
>                                      unreachable!(),
>                                      unreachable!(),
> +                                    unreachable!(),
>                                  )
>                              };
>  
>                              let Some(guard) = dev.registration_guard() else {
>                                  return $crate::error::code::ENODEV.to_errno();
>                              };
> -                            let __anchor = ();
>  
> -                            // 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. The argument is exclusively owned by this
> -                            //   handler, guaranteed by `drm_ioctl()` to remain valid for the
> -                            //   duration of the call.
> -                            // - `raw_file` is a valid `struct drm_file` pointer provided by the
> -                            //   DRM core.
> -                            match unsafe { $crate::drm::ioctl::internal::__call_ioctl(
> -                                &__anchor, &*guard, raw_data, raw_file, $func,
> -                            ) } {
> +                            match guard.registration_data_with(|reg_data| {
> +                                // 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. The argument is exclusively owned
> +                                //   by this handler, guaranteed by `drm_ioctl()` to remain valid
> +                                //   for the duration of the call.
> +                                // - `raw_file` is a valid `struct drm_file` pointer provided by
> +                                //   the DRM core.
> +                                unsafe { $crate::drm::ioctl::internal::__call_ioctl(
> +                                    reg_data, &*guard, raw_data, raw_file,
> +                                    |dev, data, file| $func(dev, reg_data, data, file),
> +                                ) }

[Severity: High]
In the declare_drm_ioctls! macro in rust/kernel/drm/ioctl.rs, does this
closure allow type inference to deduce an arbitrarily large struct size that
leads to out-of-bounds memory accesses?

Because Data is not explicitly constrained here, its type is inferred
backward from the user-provided $func. If an ioctl handler is written to
accept a larger mutable reference, it seems Data would be inferred as that
larger type.

The __call_ioctl helper will then cast the raw_data pointer to that larger
type and safely dereference it. Since the DRM core only allocates
_IOC_SIZE(cmd) bytes for the ioctl payload, couldn't this allow Safe Rust to
read and write past the allocated kernel buffer?

Should the closure explicitly annotate the type of data (e.g.,
|dev, data: &mut $crate::uapi::$struct, file|) to prevent type inference
from breaking the size constraint?

> +                            }) {
>                                  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/20260620184924.2247517-1-dakr@kernel.org?part=15