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

[Jiri Slaby <jirislaby@kernel.org>]

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