On Mon Oct 19, 2026 at 9:21 PM BST, Sami Tolvanen via B4 Relay wrote:
> From: Sami Tolvanen <[email protected]>
>
> `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 <[email protected]>
> ---
> 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
