Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Missing `gem_create_object` callback in `shmem::Object` causes the DRM core to allocate bare C structs, leading to out-of-bounds reads/writes when Rust drivers access them via `lookup_handle`. -- commit 21c57b4cda527522a0ac16331bcf2d0d3de21563 Author: Danilo Krummrich <[email protected]> Subject: rust: drm: restrict AlwaysRefCounted to Normal GEM Object context Restrict AlwaysRefCounted for gem::Object and gem::shmem::Object to the Normal context. This simplifies generic parameters, removes AlwaysRefCounted from IntoGEMObject's supertraits, and adds it as an explicit bound on lookup_handle(). > 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>, > { [Severity: Critical] This is a pre-existing issue, but since we are modifying the bounds on lookup_handle(), could the current implementation of shmem::Object lead to out-of-bounds heap accesses? In rust/kernel/drm/gem/shmem.rs, ALLOC_OPS sets gem_create_object to None while enabling dumb_create and gem_prime_import_sg_table: rust/kernel/drm/gem/shmem.rs:ALLOC_OPS { gem_create_object: None, ... gem_prime_import_sg_table: Some(bindings::drm_gem_shmem_prime_import_sg_table), dumb_create: Some(bindings::drm_gem_shmem_dumb_create), } When userspace creates a dumb buffer, __drm_gem_shmem_create() sees a NULL gem_create_object and falls back to a pure C allocation using kzalloc(sizeof(*shmem), GFP_KERNEL). This allocation lacks the trailing Rust wrapper fields (inner and parent_resv_obj). If a Rust driver later retrieves this handle via Object::lookup_handle(), it casts the C-allocated struct back to the larger Rust wrapper: rust/kernel/drm/gem/mod.rs:lookup_handle() { let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) }; if ptr.is_null() { return Err(ENOENT); } let obj = unsafe { Self::from_raw(ptr) }; ... } Does this cause any subsequent access to obj.inner or obj.parent_resv_obj by the Rust driver to read or write out of bounds on the kernel heap? Should shmem::Object provide a gem_create_object callback to ensure the full Rust struct is always allocated? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
