Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Unsound Safety Contract in IntoGEMObject::from_raw
--

--- Patch [1]: [PATCH] rust: drm: fix GEM object pointer safety docs ---
commit 83b1634421cc950ce3b8ca23f3c9ab5c476d4697
Author: Yilin Chen <[email protected]>

rust: drm: fix GEM object pointer safety docs

This commit updates the Safety documentation for IntoGEMObject::from_raw()
to state that the argument must be a valid pointer to struct drm_gem_object
rather than a pointer to Self.

> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index c8b66d8168719..90b256093e4cb 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -118,7 +118,7 @@ pub trait IntoGEMObject: Sized + super::private::Sealed + 
> AlwaysRefCounted {
>      ///
>      /// # Safety
>      ///
> -    /// - `self_ptr` must be a valid pointer to `Self`.
> +    /// - `self_ptr` must be a valid pointer to `struct drm_gem_object`.

[Severity: High]
Does this change make the safety contract unsound?

By removing the requirement that the pointer originates from an instance of
Self, the safety contract no longer protects against passing an arbitrary
valid drm_gem_object pointer.

If a caller satisfies this new documentation by passing a valid
drm_gem_object pointer that is not embedded in an instance of Self, won't
the container_of macro in the trait implementation yield an invalid pointer
and cause out-of-bounds memory access when fields of Self are accessed?

>      /// - The caller promises that holding the immutable reference returned 
> by this function does
>      ///   not violate rust's data aliasing rules and remains valid 
> throughout the lifetime of `'a`.
>      unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a 
> Self;

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

Reply via email to