[PATCH v2] drm/qxl: fix use-after-free and NULL pointer deref

[PATCH v2] drm/qxl: fix use-after-free and NULL pointer deref

[Jiri Slaby (SUSE)]

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@kernel.org>
Assisted-by: Gemini <gemini@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@amd.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: stable@vger.kernel.org
---
Cc: virtualization@lists.linux.dev
Cc: spice-devel@lists.freedesktop.org
Cc: dri-devel@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);
-- 
2.55.0

Re: [PATCH v2] drm/qxl: fix use-after-free and NULL pointer deref

[Tvrtko Ursulin]

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@kernel.org>
> Assisted-by: Gemini <gemini@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@amd.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: stable@vger.kernel.org
> ---
> Cc: virtualization@lists.linux.dev
> Cc: spice-devel@lists.freedesktop.org
> Cc: dri-devel@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?

Regards,

Tvrtko

Re: [PATCH v2] drm/qxl: fix use-after-free and NULL pointer deref

[Jiri Slaby]

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@kernel.org>
>> Assisted-by: Gemini <gemini@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@amd.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Simona Vetter <simona@ffwll.ch>
>> Cc: stable@vger.kernel.org
>> ---
>> Cc: virtualization@lists.linux.dev
>> Cc: spice-devel@lists.freedesktop.org
>> Cc: dri-devel@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

Re: [PATCH v2] drm/qxl: fix use-after-free and NULL pointer deref

[Tvrtko Ursulin]

On 02/09/2026 11:45, Jiri Slaby wrote:
> 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@kernel.org>
>>> Assisted-by: Gemini <gemini@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@amd.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Maxime Ripard <mripard@kernel.org>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: David Airlie <airlied@gmail.com>
>>> Cc: Simona Vetter <simona@ffwll.ch>
>>> Cc: stable@vger.kernel.org
>>> ---
>>> Cc: virtualization@lists.linux.dev
>>> Cc: spice-devel@lists.freedesktop.org
>>> Cc: dri-devel@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?

Right, but on a superficial look what would be lost if that wasn't done, ie:

diff --git a/drivers/gpu/drm/qxl/qxl_release.c 
b/drivers/gpu/drm/qxl/qxl_release.c
index 06979d0e8a9f..ea1e0b4f6e5b 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -147,16 +147,11 @@ qxl_release_free(struct qxl_device *qdev,
         idr_remove(&qdev->release_idr, release->id);
         spin_unlock(&qdev->release_idr_lock);

-       if (dma_fence_was_initialized(&release->base)) {
-               WARN_ON(list_empty(&release->bos));
-               qxl_release_free_list(release);
+       qxl_release_free_list(release);
+
+       dma_fence_signal(&release->base);
+       dma_fence_put(&release->base);

-               dma_fence_signal(&release->base);
-               dma_fence_put(&release->base);
-       } else {
-               qxl_release_free_list(release);
-               kfree(release);
-       }
         atomic_dec(&qdev->release_count);
  }


WARN_ON is lost but on balance how much does that matter? Or could it be 
moved somewhere else?

I don't know this driver to be clear but was just curious to understand 
if there is an alternative. As said, the patch as is looks okay to me 
looking from the outside.

Regards,

Tvrtko