From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Sami Tolvanen via B4 Relay"
<devnull+samitolvanen.google.com@kernel.org>
Cc: samitolvanen@google.com, "David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Danilo Krummrich" <dakr@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Tamir Duberstein" <tamird@kernel.org>,
"Onur Özkan" <work@onurozkan.dev>,
"Lyude Paul" <lyude@redhat.com>,
dri-devel@lists.freedesktop.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rust: drm: gem: shmem: do not leak pages when sg_table() fails
Date: Wed, 11 Nov 2026 10:11:39 +0900 [thread overview]
Message-ID: <DMRZCU0UJL9Q.251K7FN6X4GTW@nvidia.com> (raw)
In-Reply-To: <20261019-rust-shmem-fixes-v1-1-3f7f6de5b5e7@google.com>
On Tue Oct 20, 2026 at 5:21 AM JST, Sami Tolvanen via B4 Relay wrote:
> From: Sami Tolvanen <samitolvanen@google.com>
>
> `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@google.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
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`.
prev parent reply other threads:[~2026-11-11 1:11 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-19 20:21 Sami Tolvanen via B4 Relay
2026-10-20 12:23 ` Alice Ryhl
2026-10-20 12:31 ` Gary Guo
2026-10-20 20:16 ` Sami Tolvanen
2026-10-23 8:28 ` Alice Ryhl
2026-11-11 1:11 ` Alexandre Courbot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DMRZCU0UJL9Q.251K7FN6X4GTW@nvidia.com \
--to=acourbot@nvidia.com \
--cc=a.hindborg@kernel.org \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=devnull+samitolvanen.google.com@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=lyude@redhat.com \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=samitolvanen@google.com \
--cc=simona@ffwll.ch \
--cc=tamird@kernel.org \
--cc=tmgross@umich.edu \
--cc=work@onurozkan.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®