[PATCH v7 02/19] media: meson: vdec: Fix STREAMON / STREAMOFF race conditions and session teardown
Anand Moon
linux.amoon at gmail.com
Mon Jul 13 05:06:57 PDT 2026
The vdec driver suffered from unsafe state transitions and race
conditions when handling concurrent STREAMON / STREAMOFF calls and
dynamic resolution change (DRC) events. Global context pointers
(e.g. core->cur_sess) and session status flags were updated outside
proper lock boundaries, allowing parallel threads to corrupt hardware
state under load.
Address these architectural stability flaws with the following changes:
1. In vdec_start_streaming(), safely encapsulate hardware occupancy
evaluations and target session context claims within a secure mutex
lock (core->lock). This blocks overlapping multi-threaded STREAMON
calls from creating concurrent assignment conflicts.
2. Restructure initialization error path handlers to avoid global memory
maps cross-contamination. Segregate buffer flushes cleanly based
strictly on the active vb2 queue type (OUTPUT vs CAPTURE) to avoid
incorrectly reclaiming undecoded ready blocks.
3. In vdec_stop_streaming(), introduce state-aware tracking to
accurately identify DRC conditions. If a resolution modification
forces a capture queue reset while the output stream is still live,
preserve the driver's inner runtime states and bypass premature
hardware power-off sweeps.
4. Enforce strict null-pointer safety limits inside DMA unmapping
sequences. Explicitly clear tracking metadata entries
(sess->vififo_vaddr = NULL) inside the VIFIFO freeing routines to
neutralize accidental double-free risks.
Together these changes harden the driver against concurrency bugs,
eliminate memory leaks, and ensure predictable session lifecycle
management under multi threaded workloads.
Cc: Nicolas Dufresne <nicolas at ndufresne.ca>
Reported-by: Sashiko <sashiko-bot at kernel.org>
Closes: https://lore.kernel.org/all/20260530100841.9CEBA1F00893@smtp.kernel.org/
Signed-off-by: Anand Moon <linux.amoon at gmail.com>
---
drivers/staging/media/meson/vdec/vdec.c | 136 +++++++++++++++++-------
1 file changed, 95 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 6ae3471155a87..d1f35fc893de1 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -286,11 +286,6 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
struct vb2_v4l2_buffer *buf;
int ret;
- if (core->cur_sess && core->cur_sess != sess) {
- ret = -EBUSY;
- goto bufs_done;
- }
-
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
sess->streamon_out = 1;
else
@@ -308,9 +303,29 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
}
if (sess->status == STATUS_RUNNING ||
- sess->status == STATUS_NEEDS_RESUME ||
- sess->status == STATUS_INIT)
+ sess->status == STATUS_NEEDS_RESUME)
+ return 0;
+
+ /*
+ * Secure the core hardware lock before checking availability
+ * and updating session states to prevent STREAMON race conditions.
+ */
+ mutex_lock(&core->lock);
+ if (core->cur_sess && core->cur_sess != sess) {
+ ret = -EBUSY;
+ mutex_unlock(&core->lock);
+ goto err_unlock_no_hw;
+ }
+
+ /* If already half-initialized, do not re-initialize */
+ if (sess->status == STATUS_INIT) {
+ mutex_unlock(&core->lock);
return 0;
+ }
+
+ sess->status = STATUS_INIT;
+ core->cur_sess = sess;
+ mutex_unlock(&core->lock);
sess->vififo_size = SIZE_VIFIFO;
sess->vififo_vaddr =
@@ -319,7 +334,7 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
if (!sess->vififo_vaddr) {
dev_err(sess->core->dev, "Failed to request VIFIFO buffer\n");
ret = -ENOMEM;
- goto bufs_done;
+ goto err_cleanup_session;
}
sess->should_stop = 0;
@@ -333,33 +348,43 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
ret = vdec_poweron(sess);
if (ret)
- goto vififo_free;
+ goto err_free_vififo;
sess->sequence_cap = 0;
sess->sequence_out = 0;
+
if (vdec_codec_needs_recycle(sess))
sess->recycle_thread = kthread_run(vdec_recycle_thread, sess,
"vdec_recycle");
- sess->status = STATUS_INIT;
- core->cur_sess = sess;
schedule_work(&sess->esparser_queue_work);
return 0;
-vififo_free:
- dma_free_coherent(sess->core->dev, sess->vififo_size,
- sess->vififo_vaddr, sess->vififo_paddr);
-bufs_done:
- while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
- v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
- while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
- v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
-
+err_free_vififo:
+ if (sess->vififo_vaddr) {
+ dma_free_coherent(sess->core->dev, sess->vififo_size,
+ sess->vififo_vaddr, sess->vififo_paddr);
+ sess->vififo_vaddr = NULL;
+ sess->vififo_paddr = 0;
+ }
+err_cleanup_session:
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
sess->streamon_out = 0;
else
sess->streamon_cap = 0;
+ mutex_lock(&core->lock);
+ if (core->cur_sess == sess)
+ core->cur_sess = NULL;
+ if (sess->status != STATUS_NEEDS_RESUME)
+ sess->status = STATUS_STOPPED;
+ mutex_unlock(&core->lock);
+err_unlock_no_hw:
+ while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
+ v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
+ while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
+ v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
+
return ret;
}
@@ -399,30 +424,13 @@ static void vdec_stop_streaming(struct vb2_queue *q)
struct amvdec_codec_ops *codec_ops = sess->fmt_out->codec_ops;
struct amvdec_core *core = sess->core;
struct vb2_v4l2_buffer *buf;
+ enum amvdec_status old_status;
+ bool full_cleanup = false;
- if (sess->status == STATUS_RUNNING ||
- sess->status == STATUS_INIT ||
- (sess->status == STATUS_NEEDS_RESUME &&
- (!sess->streamon_out || !sess->streamon_cap))) {
- if (vdec_codec_needs_recycle(sess))
- kthread_stop(sess->recycle_thread);
-
- vdec_poweroff(sess);
- vdec_free_canvas(sess);
- dma_free_coherent(sess->core->dev, sess->vififo_size,
- sess->vififo_vaddr, sess->vififo_paddr);
- vdec_reset_timestamps(sess);
- vdec_reset_bufs_recycle(sess);
- kfree(sess->priv);
- sess->priv = NULL;
- core->cur_sess = NULL;
- sess->status = STATUS_STOPPED;
- }
-
+ /* flush buffers to kill background workqueue thread */
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
v4l2_m2m_buf_done(buf, VB2_BUF_STATE_ERROR);
-
sess->streamon_out = 0;
} else {
/* Drain remaining refs if was still running */
@@ -431,9 +439,55 @@ static void vdec_stop_streaming(struct vb2_queue *q)
while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
v4l2_m2m_buf_done(buf, VB2_BUF_STATE_ERROR);
-
sess->streamon_cap = 0;
}
+
+ /* Hold core lock continuously for the state and resource processing */
+ mutex_lock(&core->lock);
+ old_status = sess->status;
+
+ if (old_status == STATUS_RUNNING || old_status == STATUS_INIT ||
+ (old_status == STATUS_NEEDS_RESUME && (!sess->streamon_out ||
+ !sess->streamon_cap))) {
+ /*
+ * If it's a DRC event (Capture queue streamoff only), preserve
+ * the status
+ */
+ if (old_status == STATUS_NEEDS_RESUME && sess->streamon_out) {
+ full_cleanup = false;
+ } else {
+ full_cleanup = true;
+ sess->status = STATUS_STOPPED;
+ }
+ }
+
+ if (full_cleanup) {
+ if ((q->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
+ !sess->streamon_out) && vdec_codec_needs_recycle(sess)) {
+ kthread_stop(sess->recycle_thread);
+ }
+
+ vdec_poweroff(sess);
+ vdec_free_canvas(sess);
+
+ if (sess->vififo_vaddr) {
+ dma_free_coherent(sess->core->dev, sess->vififo_size,
+ sess->vififo_vaddr, sess->vififo_paddr);
+ sess->vififo_vaddr = NULL;
+ sess->vififo_paddr = 0;
+ }
+
+ vdec_reset_timestamps(sess);
+ vdec_reset_bufs_recycle(sess);
+ core->cur_sess = NULL;
+
+ kfree(sess->priv);
+ sess->priv = NULL;
+ } else {
+ if (sess->status == STATUS_NEEDS_RESUME)
+ sess->changed_format = 0;
+ }
+ mutex_unlock(&core->lock);
}
static int vdec_vb2_buf_prepare(struct vb2_buffer *vb)
--
2.50.1
More information about the linux-amlogic
mailing list