Hello,
I would like to report a reproducible kernel NULL pointer dereference in the
QXL DRM driver's release/fence cleanup path. I reproduced it on a Linux
7.1.5-based x86_64 kernel in QEMU.
Summary
=======
A userspace DRM_IOCTL_QXL_EXECBUFFER request can make
qxl_process_single_command()
allocate a qxl_release and then fail while looking up a relocation's GEM handle.
The error path calls qxl_release_free() before dma_fence_init() has run. In the
affected code, qxl_release_alloc() allocates the object with kmalloc() and only
initializes selected fields. qxl_release_free() then calls
dma_fence_was_initialized(), which reads the uninitialized fence flags. When
the stale INITIALIZED bit is set, cleanup treats the fence as initialized and
calls dma_fence_signal(). The fence lock selection can then return the
uninitialized NULL extern_lock, leading to a NULL dereference in
_raw_spin_lock_irqsave().
Trigger and call path
=====================
The reproducing userspace sequence opens /dev/dri/card0, issues
DRM_IOCTL_QXL_ALLOC, then issues DRM_IOCTL_QXL_EXECBUFFER with a QXL_CMD_DRAW
command and a BO relocation whose src_handle does not exist (the test used
0xdeadbeef). The command and relocation lengths/types are otherwise valid.
The invalid handle makes the GEM lookup return -EINVAL and takes the normal
error-cleanup path:
ioctl(DRM_IOCTL_QXL_EXECBUFFER)
-> qxl_execbuffer_ioctl()
-> qxl_process_single_command()
-> qxl_alloc_release_reserved()
-> qxlhw_handle_to_bo() [invalid GEM handle, returns -EINVAL]
-> qxl_release_free() [before dma_fence_init()]
-> dma_fence_was_initialized() [reads uninitialized flags]
-> dma_fence_signal()
-> dma_fence_spinlock() [selects NULL extern_lock]
-> _raw_spin_lock_irqsave(NULL)
Relevant source code
====================
qxl_release_alloc() allocates the release with kmalloc() and initializes only
selected fields; the embedded dma_fence state is not zeroed:
release = kmalloc(size, GFP_KERNEL);
if (!release)
return -ENOMEM;
release->base.ops = NULL;
release->type = type;
release->release_offset = 0;
release->surface_release_id = 0;
INIT_LIST_HEAD(&release->bos);
In qxl_process_single_command(), a missing GEM handle makes
qxlhw_handle_to_bo() return -EINVAL. The caller jumps to cleanup before reaching
qxl_release_fence_buffer_objects(), the later code path that can initialize the
fence with dma_fence_init():
ret = qxlhw_handle_to_bo(file_priv, reloc.src_handle, release,
&reloc_info[i].src_bo);
if (ret)
goto out_free_bos;
...
qxl_release_fence_buffer_objects(release);
out_free_bos:
out_free_release:
if (ret)
qxl_release_free(qdev, release);
The lookup returns -EINVAL when drm_gem_object_lookup() cannot find the handle:
gobj = drm_gem_object_lookup(file_priv, handle);
if (!gobj)
return -EINVAL;
The cleanup decision is the critical bug. dma_fence_was_initialized() tests
only a bit in fence->flags, and qxl_release_free() uses that result to decide
whether to signal the fence:
if (dma_fence_was_initialized(&release->base)) {
qxl_release_free_list(release);
dma_fence_signal(&release->base);
dma_fence_put(&release->base);
} else {
qxl_release_free_list(release);
kfree(release);
}
The helper itself is only a bit test; its kernel comment says this is valid
only when the fence backing memory was zero-initialized:
return fence &&
test_bit(DMA_FENCE_FLAG_INITIALIZED_BIT, &fence->flags);
Once dma_fence_signal() runs, it obtains the lock from the fence fields:
return test_bit(DMA_FENCE_FLAG_INLINE_LOCK_BIT, &fence->flags) ?
&fence->inline_lock : fence->extern_lock;
In the reproducing run, stale flags made the first branch run although the
fence had never been initialized. Its extern_lock was NULL, so
dma_fence_signal() reached the NULL lock operation in the crash trace.
The relevant userspace operations were:
openat(AT_FDCWD, "/dev/dri/card0", O_RDWR)
ioctl(fd, DRM_IOCTL_QXL_ALLOC, ...)
ioctl(fd, DRM_IOCTL_QXL_EXECBUFFER, ...)
Crash evidence
==============
Immediately before the failure, the diagnostic values were:
release=ff11000100a3fb40 base=ff11000100a3fb40
ops=0000000000000000 flags=1
extern_lock=0000000000000000 selected_lock=0000000000000000
qdev_lock=ff1100010155f2f0
The kernel reported:
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor write access in kernel mode
RIP: _raw_spin_lock_irqsave+0x73/0xc0
CR2: 0000000000000000
Call Trace:
dma_fence_signal
qxl_release_free
qxl_execbuffer_ioctl
drm_ioctl_kernel
drm_ioctl
__x64_sys_ioctl
This is a kernel Oops, not merely an ioctl returning an error. Replacing the
release allocation with kzalloc() in my QEMU test caused the same invalid
handle request to return -EINVAL without the NULL dereference. I have not
verified this reproducer on physical QXL hardware.
Proposed fix
============
The following change zero-initializes the release object so that the
pre-initialization cleanup path cannot mistake stale fence flags for an
initialized dma_fence:
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -91,8 +91,8 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
int handle;
size_t size = sizeof(*release);
- release = kmalloc(size, GFP_KERNEL);
+ release = kzalloc(size, GFP_KERNEL);
if (!release) {
DRM_ERROR("Out of memory\n");
return -ENOMEM;
GPT-6-Luna assisted with the initial analysis and drafting this proposed fix.
I am responsible for the report and its technical claims. I can provide the
complete reproducer and full kernel log directly if needed.
Regards,
Yuhang Pan
[email protected]
