Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Type confusion leading to out-of-bounds memory access due to 
C-allocated GEM objects being unsafely cast to Rust `Object<T>`.
--

commit c0b1cc771ed0a5a11fb0a16d94befa79e5c52a6d
Author: Danilo Krummrich <[email protected]>

rust: drm: restrict AlwaysRefCounted to Normal GEM Object context

This commit restricts AlwaysRefCounted for gem::Object and
gem::shmem::Object to the Normal context. It updates Object::new methods
to Normal-only blocks and simplifies DeviceContext usage.

> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index 1023ddccd7857..d56cbe2663e29 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
[ ... ]
> @@ -198,8 +196,8 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> 
> Result<u32>
>      /// Looks up an object by its handle for a given File.
>      fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> 
> Result<ARef<Self>>
>      where
> -        Self: AllocImpl<Driver = D>,
> -        D: drm::Driver<Object<Normal> = Self, File = F>,
> +        Self: AllocImpl<Driver = D> + AlwaysRefCounted,
> +        D: drm::Driver<Object = Self, File = F>,
>          F: drm::file::DriverFile<Driver = D>,
>      {
>          // SAFETY: The arguments are all valid per the type invariants.

[Severity: High]
This is a pre-existing issue, but can this lead to an out-of-bounds memory
access during type casting?

In rust/kernel/drm/gem/shmem.rs, ALLOC_OPS leaves gem_create_object as None.
When userspace creates a dumb buffer or imports a PRIME buffer, the C helpers
drm_gem_shmem_dumb_create() and drm_gem_shmem_prime_import_sg_table() fall 
back to allocating a bare C struct drm_gem_shmem_object via kzalloc().

When the driver later calls lookup_handle() (defined here in 
rust/kernel/drm/gem/mod.rs), the underlying code unsafely casts the raw
C object pointer to the Rust wrapper:

    let obj = unsafe { Self::from_raw(ptr) };

Since the C-allocated object lacks the trailing fields of the Rust Object
wrapper (such as inner and sgt_lock), would accessing those fields read or
write out of bounds? Is there a need to provide a gem_create_object callback
so the full Rust wrapper is always allocated?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=7

Reply via email to