Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Decoupling the DMA fence lock from the queue lock breaks implicit synchronization, causing a Use-After-Free of `bin_job` in `v3d_overflow_mem_work()`. --
commit f6b0f3208abbe884d00ac4c3eab910833b936a7f Author: Maíra Canal <[email protected]> Subject: drm/v3d: Use inline lock for dma fence initialization This patch changes dma_fence_init() to accept a NULL external lock, falling back to an inline spinlock embedded in the fence itself. This decouples the lock and the fence lifetime, allowing queue_lock to use spin_(un)lock() instead of the IRQ variants in all its uses. > diff --git a/drivers/gpu/drm/v3d/v3d_fence.c b/drivers/gpu/drm/v3d/v3d_fence.c > index c500136d04557..9b1a882a4c159 100644 > --- a/drivers/gpu/drm/v3d/v3d_fence.c > +++ b/drivers/gpu/drm/v3d/v3d_fence.c > @@ -15,7 +15,7 @@ struct dma_fence *v3d_fence_create(struct v3d_dev *v3d, > enum v3d_queue q) > fence->dev = &v3d->drm; > fence->queue = q; > fence->seqno = ++queue->emit_seqno; > - dma_fence_init(&fence->base, &v3d_fence_ops, &queue->queue_lock, > + dma_fence_init(&fence->base, &v3d_fence_ops, NULL, [Severity: Critical] Does decoupling the DMA fence lock from the queue lock here break implicit synchronization and cause a use-after-free of bin_job in v3d_overflow_mem_work()? It looks like v3d_overflow_mem_work() runs in a workqueue and accesses bin_job = (struct v3d_bin_job *)queue->active_job under queue_lock: drivers/gpu/drm/v3d/v3d_irq.c:v3d_overflow_mem_work() { ... spin_lock_irqsave(&queue->queue_lock, irq_flags); ... list_add_tail(&bo->unref_head, &bin_job->render->unref_list); ... } Meanwhile, the interrupt handler v3d_irq_signal_fence() clears queue->active_job and signals the job's fence: drivers/gpu/drm/v3d/v3d_irq.c:v3d_irq_signal_fence() { ... queue->active_job = NULL; ... dma_fence_signal(&fence->base); ... } Before this patch, the fence's external lock was set to &queue->queue_lock, so dma_fence_signal() would block on queue_lock, preventing the DRM scheduler from waking up and freeing the job while v3d_overflow_mem_work() was executing. By changing the fence's external lock to NULL, dma_fence_signal() uses its own inline lock and returns immediately. Could this allow the DRM scheduler's wait queue to wake up and free bin_job concurrently, leading to a use-after-free when v3d_overflow_mem_work() dereferences bin_job->render? > queue->fence_context, fence->seqno); > > return &fence->base; -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
