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

From: Alexandre Courbot

Date: Tue Nov 10 2026 - 20:16:13 EST


On Tue Oct 20, 2026 at 5:21 AM JST, 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.
>
> 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>

Reviewed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>

As Alice mentioned we probably want a follow-up that makes the API more
symmetric (by calling `drm_gem_shmem_put_pages_locked()` in the
destructor?), as right now SGTableMap's Drop implementation doesn't undo
its constructor completely.

But this change is good to have regardless as we want to acquire the
pages after Devres' allocation succeeds, otherwise there is not point
returning an `impl Init`.