Re: [PATCH] rust: drm: gem: shmem: do not leak pages when sg_table() fails

[Gary Guo]

On Mon Oct 19, 2026 at 9:21 PM BST, Sami Tolvanen via B4 Relay wrote:
> From: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
>
> `SGTableMap::new()` takes a pages reference and maps the table before
> `Devres::new()` allocates. If the allocation fails, dropping the map
> frees the table but keeps the reference, and the next `sg_table()`
> takes another one. `drm_gem_shmem_release()` puts only one, so the
> pages leak.

Sorry, could you explain it in more detail?

Creating a SGTableMap and dropping it doesn't restore the state? If so, it
sounds like an API design issue.

Best,
Gary

>
> Take the pages in the initializer, after the allocation.
>
> Fixes: 616c229ab010 ("rust: drm: gem: Introduce shmem::Object::sg_table()")
> Signed-off-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> ---
>  rust/kernel/drm/gem/shmem.rs | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
> index a687d46d170d..72e4af3b0503 100644
> --- a/rust/kernel/drm/gem/shmem.rs
> +++ b/rust/kernel/drm/gem/shmem.rs
> @@ -555,15 +555,18 @@ fn drop(&mut self) {
>  }
>  
>  impl<T: DriverObject> SGTableMap<T> {
> -    fn new(obj: &Object<T>) -> impl Init<Self, Error> {
> -        // INVARIANT:
> -        // - We call drm_gem_shmem_get_pages_sgt below and check whether or not it succeeds,
> -        //   fulfilling the invariant of SGTableMap that the object's `sgt` field is initialized.
> -        // SAFETY:
> -        // - `obj` is fully initialized, making this function safe to call.
> -        from_err_ptr(unsafe { bindings::drm_gem_shmem_get_pages_sgt(obj.as_raw_shmem()) })?;
> -
> -        Ok(Self { obj: obj.into() })
> +    fn new(obj: &Object<T>) -> impl Init<Self, Error> + '_ {
> +        pin_init::init_scope(move || {
> +            // SAFETY:
> +            // - `obj` is fully initialized, making this function safe to call.
> +            from_err_ptr(unsafe { bindings::drm_gem_shmem_get_pages_sgt(obj.as_raw_shmem()) })?;
> +
> +            // INVARIANT:
> +            // - We call drm_gem_shmem_get_pages_sgt above and check whether or not it succeeds,
> +            //   fulfilling the invariant of SGTableMap that the object's `sgt` field is
> +            //   initialized.
> +            Ok(try_init!(Self { obj: obj.into() }))
> +        })
>      }
>  }
>  
>
> ---
> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
> change-id: 20261019-rust-shmem-fixes-3a838ada39bf