* [PATCH 1/2] drm/vc4: Drop BO usecnt before releasing the old FB
2026-11-12 12:56 [PATCH 0/2] drm/vc4: Fix async page flip completion Maíra Canal
@ 2026-11-12 12:56 ` Maíra Canal
2026-11-12 12:56 ` [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue Maíra Canal
1 sibling, 0 replies; 4+ messages in thread
From: Maíra Canal @ 2026-11-12 12:56 UTC (permalink / raw)
To: Iago Toral, Melissa Wen, Maxime Ripard, Dave Stevenson,
Raspberry Pi Kernel Maintenance, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: kernel-dev, dri-devel, stable, Maíra Canal
The async page flip completion drops the old FB reference before
decrementing its BO usecnt. If that was the last reference, the BO is
already freed or recycled into the BO cache when vc4_bo_dec_usecnt()
runs, leading to a use-after-free or a refcount underflow.
This could happen when userspace removes the old FB and closes its GEM
handle while the flip is still pending.
Decrement the BO usecnt before releasing the old FB reference. Do this
by unifying the two fence callbacks and only decrementing the BO usecnt
in VC4_GEN_4.
Cc: stable@vger.kernel.org
Fixes: 4d12c36fb73b ("drm/vc4: crtc: Move the BO handling out of common page-flip callback")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 62 +++++++++++++++---------------------------
1 file changed, 22 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f42cd5353230..c558b527c7e6 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -895,6 +895,7 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
{
struct drm_crtc *crtc = flip_state->crtc;
struct drm_device *dev = crtc->dev;
+ struct vc4_dev *vc4 = to_vc4_dev(dev);
struct drm_plane *plane = crtc->primary;
vc4_plane_async_set_fb(plane, flip_state->fb);
@@ -909,39 +910,27 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
drm_crtc_vblank_put(crtc);
drm_framebuffer_put(flip_state->fb);
- if (flip_state->old_fb)
- drm_framebuffer_put(flip_state->old_fb);
-
- kfree(flip_state);
-}
-
-static void vc4_async_page_flip_complete_with_cleanup(struct dma_fence *fence,
- struct dma_fence_cb *cb)
-{
- struct vc4_async_flip_state *flip_state =
- container_of(cb, struct vc4_async_flip_state, cb);
- struct vc4_bo *bo = NULL;
-
if (flip_state->old_fb) {
- struct drm_gem_dma_object *dma_bo =
- drm_fb_dma_get_gem_obj(flip_state->old_fb, 0);
- bo = to_vc4_bo(&dma_bo->base);
+ /*
+ * Decrement the BO usecnt in order to keep the inc/dec
+ * calls balanced when the planes are updated through
+ * the async update path.
+ *
+ * FIXME: we should move to generic async-page-flip when
+ * it's available, so that we can get rid of this
+ * hand-made cleanup_fb() logic.
+ */
+ if (vc4->gen == VC4_GEN_4) {
+ struct drm_gem_dma_object *dma_bo =
+ drm_fb_dma_get_gem_obj(flip_state->old_fb, 0);
+
+ vc4_bo_dec_usecnt(to_vc4_bo(&dma_bo->base));
+ }
+
+ drm_framebuffer_put(flip_state->old_fb);
}
- vc4_async_page_flip_complete(flip_state);
- dma_fence_put(fence);
-
- /*
- * Decrement the BO usecnt in order to keep the inc/dec
- * calls balanced when the planes are updated through
- * the async update path.
- *
- * FIXME: we should move to generic async-page-flip when
- * it's available, so that we can get rid of this
- * hand-made cleanup_fb() logic.
- */
- if (bo)
- vc4_bo_dec_usecnt(bo);
+ kfree(flip_state);
}
static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
@@ -959,30 +948,23 @@ static int vc4_async_set_fence_cb(struct drm_device *dev,
{
struct drm_framebuffer *fb = flip_state->fb;
struct drm_gem_dma_object *dma_bo = drm_fb_dma_get_gem_obj(fb, 0);
- dma_fence_func_t async_page_flip_complete_function;
- struct vc4_dev *vc4 = to_vc4_dev(dev);
struct dma_fence *fence;
int ret;
- if (vc4->gen == VC4_GEN_4)
- async_page_flip_complete_function = vc4_async_page_flip_complete_with_cleanup;
- else
- async_page_flip_complete_function = vc4_async_page_flip_fence_complete;
-
ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ, &fence);
if (ret)
return ret;
/* If there's no fence, complete the page flip immediately */
if (!fence) {
- async_page_flip_complete_function(fence, &flip_state->cb);
+ vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
return 0;
}
/* If the fence has already been completed, complete the page flip */
if (dma_fence_add_callback(fence, &flip_state->cb,
- async_page_flip_complete_function))
- async_page_flip_complete_function(fence, &flip_state->cb);
+ vc4_async_page_flip_fence_complete))
+ vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
return 0;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue
2026-11-12 12:56 [PATCH 0/2] drm/vc4: Fix async page flip completion Maíra Canal
2026-11-12 12:56 ` [PATCH 1/2] drm/vc4: Drop BO usecnt before releasing the old FB Maíra Canal
@ 2026-11-12 12:56 ` Maíra Canal
2026-11-12 13:17 ` sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Maíra Canal @ 2026-11-12 12:56 UTC (permalink / raw)
To: Iago Toral, Melissa Wen, Maxime Ripard, Dave Stevenson,
Raspberry Pi Kernel Maintenance, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: kernel-dev, dri-devel, stable, Maíra Canal
Async page flips complete from a dma_fence callback, which runs in the
fence signaling context: the VC4 render job IRQ handler on RPi 0-3 since
commit e8941ac97f28 ("drm/vc4: Remove BOs seqnos"), and the V3D IRQ
handler on RPi 4-5 since commit d19e00ee06a9 ("drm/vc4: crtc: Fix out of
order frames during asynchronous page flips").
Completing the flip may sleep: on RPi 0-3, vc4_bo_dec_usecnt() takes
bo->madv_lock, and on all platforms, dropping the last framebuffer
reference takes fb_lock and can free the BO. When running the kernel
with CONFIG_DEBUG_ATOMIC_SLEEP=y on RPi 3, this triggers the following
warning:
[ 2511.636319] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:623
[ 2511.636667] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 0, name: swapper/0
[ 2511.636681] preempt_count: 1000002, expected: 0
[ 2511.636689] RCU nest depth: 0, expected: 0
[ 2511.636698] locks held by swapper/0/0: 1, on CPU#0:
[ 2511.636711] #0: ffff000037183198 (&fence->inline_lock){-...}-{3:3}, at: dma_fence_signal+0x88/0xec
<snip>
[ 2511.636892] Hardware name: Raspberry Pi 3 Model B Plus Rev 1.3 (DT)
[ 2511.636899] Call trace:
<snip>
[ 2511.636991] mutex_lock_nested+0x24/0x30
[ 2511.637002] vc4_bo_dec_usecnt+0x40/0x124 [vc4]
[ 2511.637070] vc4_async_page_flip_complete_with_cleanup+0xa0/0xc4 [vc4]
[ 2511.637102] dma_fence_signal_timestamp_locked+0x118/0x290
[ 2511.637116] dma_fence_signal+0x9c/0xec
[ 2511.637126] vc4_irq_render_job_done+0x24/0x114 [vc4]
[ 2511.637158] vc4_irq+0x16c/0x274 [vc4]
Make the fence callback only queue a work item on a per-CRTC ordered
workqueue and complete the flip from it. Flips with no fence or an already
signaled one still complete inline, but only after flushing that workqueue,
so they can't complete before earlier pending flips on the same CRTC.
Cc: stable@vger.kernel.org
Fixes: d19e00ee06a9 ("drm/vc4: crtc: Fix out of order frames during asynchronous page flips")
Fixes: e8941ac97f28 ("drm/vc4: Remove BOs seqnos")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 52 +++++++++++++++++++++++++++---------------
drivers/gpu/drm/vc4/vc4_drv.h | 6 +++++
2 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index c558b527c7e6..623b636a0365 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -884,15 +884,18 @@ struct vc4_async_flip_state {
struct drm_framebuffer *fb;
struct drm_framebuffer *old_fb;
struct drm_pending_vblank_event *event;
+ struct dma_fence *fence;
struct dma_fence_cb cb;
+ struct work_struct work;
};
-/* Called when the V3D execution for the BO being flipped to is done, so that
- * we can actually update the plane's address to point to it.
+/* Executed when the V3D execution for the BO being flipped to is done, so
+ * that we can actually update the plane's address to point to it.
*/
-static void
-vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
+static void vc4_async_page_flip_work(struct work_struct *work)
{
+ struct vc4_async_flip_state *flip_state =
+ container_of(work, struct vc4_async_flip_state, work);
struct drm_crtc *crtc = flip_state->crtc;
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -930,17 +933,19 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
drm_framebuffer_put(flip_state->old_fb);
}
+ dma_fence_put(flip_state->fence);
kfree(flip_state);
}
-static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
- struct dma_fence_cb *cb)
+static void vc4_async_page_flip_fence_cb(struct dma_fence *fence,
+ struct dma_fence_cb *cb)
{
struct vc4_async_flip_state *flip_state =
container_of(cb, struct vc4_async_flip_state, cb);
+ struct vc4_crtc *vc4_crtc = to_vc4_crtc(flip_state->crtc);
- vc4_async_page_flip_complete(flip_state);
- dma_fence_put(fence);
+ /* Completing the flip may sleep, so defer it to a worker. */
+ queue_work(vc4_crtc->async_flip_wq, &flip_state->work);
}
static int vc4_async_set_fence_cb(struct drm_device *dev,
@@ -948,23 +953,26 @@ static int vc4_async_set_fence_cb(struct drm_device *dev,
{
struct drm_framebuffer *fb = flip_state->fb;
struct drm_gem_dma_object *dma_bo = drm_fb_dma_get_gem_obj(fb, 0);
- struct dma_fence *fence;
+ struct vc4_crtc *vc4_crtc = to_vc4_crtc(flip_state->crtc);
int ret;
- ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ, &fence);
+ INIT_WORK(&flip_state->work, vc4_async_page_flip_work);
+
+ ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ,
+ &flip_state->fence);
if (ret)
return ret;
- /* If there's no fence, complete the page flip immediately */
- if (!fence) {
- vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
+ if (flip_state->fence &&
+ !dma_fence_add_callback(flip_state->fence, &flip_state->cb,
+ vc4_async_page_flip_fence_cb))
return 0;
- }
- /* If the fence has already been completed, complete the page flip */
- if (dma_fence_add_callback(fence, &flip_state->cb,
- vc4_async_page_flip_fence_complete))
- vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
+ /* No fence or the fence has already signaled. Complete the flip
+ * immediately, after earlier queued flips.
+ */
+ flush_workqueue(vc4_crtc->async_flip_wq);
+ vc4_async_page_flip_work(&flip_state->work);
return 0;
}
@@ -990,7 +998,7 @@ vc4_async_page_flip_common(struct drm_crtc *crtc,
/* Save the current FB before it's replaced by the new one in
* drm_atomic_set_fb_for_plane(). We'll need the old FB in
- * vc4_async_page_flip_complete() to decrement the BO usecnt and keep
+ * vc4_async_page_flip_work() to decrement the BO usecnt and keep
* it consistent.
* FIXME: we should move to generic async-page-flip when it's
* available, so that we can get rid of this hand-made cleanup_fb()
@@ -1387,7 +1395,13 @@ int __vc4_crtc_init(struct drm_device *drm,
vc4_crtc->data = data;
vc4_crtc->pdev = pdev;
vc4_crtc->feeds_txp = feeds_txp;
+ vc4_crtc->async_flip_wq = drmm_alloc_ordered_workqueue(drm, "%s-async-flip",
+ 0, data->name);
+ if (IS_ERR(vc4_crtc->async_flip_wq))
+ return PTR_ERR(vc4_crtc->async_flip_wq);
+
spin_lock_init(&vc4_crtc->irq_lock);
+
ret = drmm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
crtc_funcs, data->name);
if (ret)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 09134809bf15..f68486be08a6 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -596,6 +596,12 @@ struct vc4_crtc {
* access to that value.
*/
unsigned int current_hvs_channel;
+
+ /**
+ * @async_flip_wq: Ordered workqueue completing the CRTC's async
+ * page flips.
+ */
+ struct workqueue_struct *async_flip_wq;
};
#define to_vc4_crtc(_crtc) \
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread