Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Ignoring the return value of `qxl_release_list_add()` can lead to a use-after-free of a buffer object by the hardware. - [High] Sleeping in atomic context during plane updates. -- commit f18d8a877369f4db994d43ef4a35d9b13d174685 Author: Jiri Slaby (SUSE) <[email protected]> drm/qxl: fix use-after-free and NULL pointer deref 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. Closes: https://bugzilla.suse.com/show_bug.cgi?id=1271081 > diff --git a/drivers/gpu/drm/qxl/qxl_release.c > b/drivers/gpu/drm/qxl/qxl_release.c > index 06979d0e8a9f0..07dc6eafe6f7f 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; > > - 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); [Severity: High] This isn't a bug introduced by this patch, but does this code sleep in an atomic context? Looking at the plane update paths, qxl_primary_atomic_update() is an atomic commit callback where sleeping is forbidden. It appears to call qxl_release_alloc() through this chain: qxl_primary_atomic_update() qxl_draw_dirty_fb() alloc_drawable() qxl_alloc_release_reserved() qxl_release_alloc() Can kzalloc_obj() and idr_preload(GFP_KERNEL) sleep here? Additionally, qxl_alloc_release_reserved() acquires a mutex before calling this function, which could also cause deadlocks in an atomic context. [Severity: High] This is also a pre-existing issue, but can ignoring the return value of qxl_release_list_add() lead to a hardware use-after-free? In drivers/gpu/drm/qxl/qxl_release.c:qxl_alloc_surface_release_reserved(): qxl_release_list_add(*release, bo); If qxl_release_list_add() fails (e.g. returning -ENOMEM), the BO is not added to the list and its reference count is not incremented. However, the function ignores this failure and continues, returning success. When the hardware ring processes the command referencing this BO, could the BO memory be freed prematurely if the original reference is dropped? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
