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 <[email protected]> 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/[email protected]?part=15
