[PATCH v7 08/19] media: meson: vdec: Fix concurrent execution races and unsafe teardown
Anand Moon
linux.amoon at gmail.com
Mon Jul 13 05:07:03 PDT 2026
Address data races involving 'should_stop' and prevent multi-session
hardware clobbering by enforcing atomic tracking and strict owner
validation during device teardown.
The esparser work queue reads 'sess->should_stop' outside of critical
regions without serialization primitives, risking data races or visibility
delays. Furthermore, vdec_close() and vdec_stop_streaming() blindly
shut down hardware components (via poweroff and canvas frees) and nullify
'core->cur_sess' without confirming that the executing session actually
owns the active hardware context. In multi-session scenarios, this allows
a closing inactive session to inadvertently break a running session.
To fix these synchronization and lifecycle issues with the following
changes use thread-safe flagging: Wrap reads and writes of
'sess->should_stop' in READ_ONCE() and WRITE_ONCE() to prevent compiler
optimizations from caching the condition variables across scheduling
boundaries and optimizations across workqueue execution threads.
Also safe context releasing: transition 'core->cur_sess' pointer
clearings to use smp_store_release(). This ensures all prior internal
memory structures are entirely flushed and visible to other execution
cores.
Cc: Nicolas Dufresne <nicolas at ndufresne.ca>
Signed-off-by: Anand Moon <linux.amoon at gmail.com>
---
drivers/staging/media/meson/vdec/esparser.c | 2 +-
drivers/staging/media/meson/vdec/vdec.c | 88 ++++++++++++++-------
2 files changed, 59 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/media/meson/vdec/esparser.c b/drivers/staging/media/meson/vdec/esparser.c
index e5d3d817b9b2b..959673742e699 100644
--- a/drivers/staging/media/meson/vdec/esparser.c
+++ b/drivers/staging/media/meson/vdec/esparser.c
@@ -379,7 +379,7 @@ void esparser_queue_all_src(struct work_struct *work)
scoped_guard(mutex, &sess->lock) {
/* Safe atomic tracking check: exit loop if session is shutting down */
- if (sess->should_stop)
+ if (READ_ONCE(sess->should_stop))
return;
/* Queue completely empty: exit work loop cleanly */
diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 6fe9722577179..83a9b1238972a 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -287,9 +287,13 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
struct amvdec_session *sess = vb2_get_drv_priv(q);
struct amvdec_codec_ops *codec_ops = sess->fmt_out->codec_ops;
struct amvdec_core *core = sess->core;
+ struct device *dev = core->dev_dec;
struct vb2_v4l2_buffer *buf;
int ret;
+ /* Reset workqueue loop shutdown signal to allow streaming */
+ WRITE_ONCE(sess->should_stop, 0);
+
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
sess->streamon_out = 1;
else
@@ -336,7 +340,7 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
dma_alloc_coherent(sess->core->dev, sess->vififo_size,
&sess->vififo_paddr, GFP_KERNEL);
if (!sess->vififo_vaddr) {
- dev_err(sess->core->dev, "Failed to request VIFIFO buffer\n");
+ dev_err(dev, "Failed to request VIFIFO buffer\n");
ret = -ENOMEM;
goto err_cleanup_session;
}
@@ -388,10 +392,12 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
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;
+ if (core->cur_sess == sess) {
+ /* Safely clear hardware ownership since we were confirmed as the owner */
+ smp_store_release(&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)))
@@ -441,6 +447,9 @@ static void vdec_stop_streaming(struct vb2_queue *q)
enum amvdec_status old_status;
bool full_cleanup = false;
+ /* Signal workqueue loop to abort instantly */
+ WRITE_ONCE(sess->should_stop, 1);
+
/* 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)))
@@ -493,26 +502,33 @@ static void vdec_stop_streaming(struct vb2_queue *q)
cancel_work_sync(&sess->esparser_queue_work);
mutex_lock(&core->lock);
- vdec_poweroff(sess);
- vdec_free_canvas(sess);
+ if (core->cur_sess == sess) {
+ 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;
+ }
- 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);
- vdec_reset_timestamps(sess);
- vdec_reset_bufs_recycle(sess);
- core->cur_sess = NULL;
+ kfree(sess->priv);
+ sess->priv = NULL;
- kfree(sess->priv);
- sess->priv = NULL;
+ /* Safely clear hardware ownership since we were confirmed as the owner */
+ smp_store_release(&core->cur_sess, NULL);
+ }
} else {
if (sess->status == STATUS_NEEDS_RESUME)
sess->changed_format = 0;
}
+
mutex_unlock(&core->lock);
}
@@ -802,7 +818,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
if (cmd->cmd == V4L2_DEC_CMD_START) {
v4l2_m2m_clear_state(sess->m2m_ctx);
- sess->should_stop = 0;
+ WRITE_ONCE(sess->should_stop, 0);
return 0;
}
@@ -812,7 +828,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
dev_dbg(dev, "Received V4L2_DEC_CMD_STOP\n");
- sess->should_stop = 1;
+ WRITE_ONCE(sess->should_stop, 1);
v4l2_m2m_mark_stopped(sess->m2m_ctx);
@@ -998,6 +1014,9 @@ static int vdec_close(struct file *file)
struct amvdec_session *sess = file_to_amvdec_session(file);
struct amvdec_core *core = sess->core;
+ /* Signal workqueue loop to abort instantly */
+ WRITE_ONCE(sess->should_stop, 1);
+
/* Synchronize and flush pending hardware interrupt service routines */
synchronize_irq(core->vdec_irq);
/* Ensure esparser ISR finishes executing */
@@ -1012,21 +1031,30 @@ static int vdec_close(struct file *file)
mutex_lock(&core->lock);
- vdec_poweroff(sess);
- vdec_free_canvas(sess);
- core->cur_sess = NULL;
+ if (core->cur_sess == sess) {
+ vdec_poweroff(sess);
+ vdec_free_canvas(sess);
- if (sess->vififo_vaddr) {
- dma_free_coherent(core->dev, sess->vififo_size,
- sess->vififo_vaddr, sess->vififo_paddr);
- sess->vififo_vaddr = NULL;
- sess->vififo_paddr = 0;
+ if (sess->vififo_vaddr) {
+ dma_free_coherent(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);
}
- vdec_reset_timestamps(sess);
- vdec_reset_bufs_recycle(sess);
+
kfree(sess->priv);
sess->priv = NULL;
+ /* Unconditionally set our local status to stopped */
+ sess->status = STATUS_STOPPED;
+ /* Safely clear hardware ownership since we were confirmed as the owner */
+ smp_store_release(&core->cur_sess, NULL);
+
mutex_unlock(&core->lock);
v4l2_m2m_ctx_release(sess->m2m_ctx);
--
2.50.1
More information about the linux-amlogic
mailing list