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) <[email protected]>
Assisted-by: Gemini <[email protected]> # 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 <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Dave Airlie <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Simona Vetter <[email protected]>
Cc: [email protected]
---
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

[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

Reply via email to