On 02. 09. 26, 11:53, Tvrtko Ursulin wrote:
On 28/08/2026 09:51, Jiri Slaby (SUSE) wrote:
> When allocating a `qxl_release` structure with `kmalloc()`, the
> underlying
> memory contained uninitialized garbage. Specifically, `release-
> >base.flags`
> (part of the embedded `dma_fence`) was not cleared.
>
> This garbage in `base.flags` caused helper functions such as
> `dma_fence_was_initialized()` to return true even for releases where the
> fence was never actually initialized (e.g. via `dma_fence_init()`).
>
> Consequently, during release cleanup in `qxl_release_free()`, the driver
> attempted to put/free an uninitialized `dma_fence`, leading to refcount
> underflows (`refcount_t: underflow; use-after-free`) and subsequent NULL
> pointer dereferences in `dma_fence_signal_timestamp_locked()`.
>
> Fix this by switching from `kmalloc()` to `kzalloc_obj()` in
> `qxl_release_alloc()`, ensuring all fields (including embedded fence
> flags) are properly zero-initialized upon allocation, and remove
> redundant explicit zero-initializations.
>
> The dumps in question:
> refcount_t: underflow; use-after-free.
> WARNING: lib/refcount.c:28 at refcount_warn_saturate+0x59/0x90,
> CPU#0: kworker/0:0/1534
> Modules linked in: af_packet nft_fib_inet ...
> CPU: 0 UID: 0 PID: 1534 Comm: kworker/0:0 Not tainted 7.1.3-1-
> default #1 PREEMPT(full) openSUSE Tumbleweed
> b041a6527f6e58424f4cd3de0fade8d408b378fd
> ...
> RIP: 0010:refcount_warn_saturate+0x59/0x90
> ...
> Call Trace:
> <TASK>
> qxl_release_free+0xee/0xf0 [qxl
> d93e9381353e619799d56790f5f8dda6cce491f6]
> qxl_garbage_collect+0xd1/0x1b0 [qxl
> d93e9381353e619799d56790f5f8dda6cce491f6]
> process_one_work+0x19e/0x3a0
> ...
>
> And then of course:
> BUG: kernel NULL pointer dereference, address: 0000000000000028
> ...
> RIP: 0010:dma_fence_signal_timestamp_locked+0x32/0x120
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby(a)kernel.org>
> Assisted-by: Gemini <gemini(a)google.com> # only commit log
> Fixes: 2bcbc706dfa0 ("dma-buf: add dma_fence_was_initialized function
> v2")
> Closes:
https://bugzilla.suse.com/show_bug.cgi?id=1271081
> Cc: Christian König <christian.koenig(a)amd.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com>
> Cc: Dave Airlie <airlied(a)redhat.com>
> Cc: Gerd Hoffmann <kraxel(a)redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com>
> Cc: Maxime Ripard <mripard(a)kernel.org>
> Cc: Thomas Zimmermann <tzimmermann(a)suse.de>
> Cc: David Airlie <airlied(a)gmail.com>
> Cc: Simona Vetter <simona(a)ffwll.ch>
> Cc: stable(a)vger.kernel.org
> ---
> Cc: virtualization(a)lists.linux.dev
> Cc: spice-devel(a)lists.freedesktop.org
> Cc: dri-devel(a)lists.freedesktop.org
>
> [v2] use kzalloc_obj() instead of bare kzalloc()
> ---
> drivers/gpu/drm/qxl/qxl_release.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/
> qxl_release.c
> index 06979d0e8a9f..07dc6eafe6f7 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -89,17 +89,13 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
> {
> struct qxl_release *release;
> int handle;
> - size_t size = sizeof(*release);
> - release = kmalloc(size, GFP_KERNEL);
> + release = kzalloc_obj(*release);
> if (!release) {
> DRM_ERROR("Out of memory\n");
> return -ENOMEM;
> }
> - release->base.ops = NULL;
> release->type = type;
> - release->release_offset = 0;
> - release->surface_release_id = 0;
> INIT_LIST_HEAD(&release->bos);
> idr_preload(GFP_KERNEL);
Looks plausible on a superficial look, albeit fragile. I am not sure why
qxl_release_alloc wasn't calling dma_fence_init in the first place?
If you did, you could not test the ops (previously) or
dma_fence_was_initialized() now, right?
thanks,
--
js
suse labs