mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] media: v4l2-mem2mem: wait for the job-finish scheduling tail
@ 2026-11-04  9:30 Yaohan Li
  2026-11-12  9:41 ` hverkuil+cisco
  0 siblings, 1 reply; 2+ messages in thread
From: Yaohan Li @ 2026-11-04  9:30 UTC (permalink / raw)
  To: mchehab
  Cc: hverkuil+cisco, Frank.Li, ming.qian, nicolas.dufresne, dmantipov,
	121811682, ezequiel, linux-media, linux-kernel

The job-finish path clears TRANS_RUNNING and TRANS_QUEUED before its
follow-up scheduling tail has finished using the mem2mem context.
If release races with that tail, v4l2_m2m_cancel_job() can observe no
running or queued job and allow the context to be freed while the tail
still accesses it.

Track the scheduling tail with a completion and wait for it in the
cancellation path. Start the completion in the done state so contexts
that never ran a job do not wait indefinitely, reinitialize it when a
scheduling tail begins, and complete it after the tail finishes.

The original race was reproduced as a KASAN use-after-free in the
common mem2mem code using the in-tree vicodec test driver and targeted
timing injection.

Fixes: cbd9463da1b12 ("media: v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish")
Assisted-by: ZCode:GLM-4
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 27 ++++++++++++++++++++++++++
 include/media/v4l2-mem2mem.h           |  3 ++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index a65cbb124cfe0..7637a876bb8c3 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -439,6 +439,13 @@ static void v4l2_m2m_cancel_job(struct v4l2_m2m_ctx *m2m_ctx)
 		/* Do nothing, was not on queue/running */
 		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
 	}
+
+	/*
+	 * A job-finish tail may still walk this instance after
+	 * TRANS_RUNNING was cleared; wait for it to finish so the caller
+	 * can safely release the instance.
+	 */
+	wait_for_completion(&m2m_ctx->scheduling_done);
 }
 
 /*
@@ -460,6 +467,14 @@ static void v4l2_m2m_schedule_next_job(struct v4l2_m2m_dev *m2m_dev,
 	 * but the job must be run in non-atomic context.
 	 */
 	schedule_work(&m2m_dev->job_work);
+
+	/*
+	 * Signal that the scheduling tail is done. complete_all is
+	 * atomic: once called, all current and future waiters are
+	 * released, with no lock-free condition window as with
+	 * wait_event() + a flag bit.
+	 */
+	complete_all(&m2m_ctx->scheduling_done);
 }
 
 /*
@@ -476,7 +491,14 @@ static bool _v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
 
 	list_del(&m2m_dev->curr_ctx->queue);
 	m2m_dev->curr_ctx->job_flags &= ~(TRANS_QUEUED | TRANS_RUNNING);
+	/*
+	 * The follow-up scheduling of the same instance still walks it
+	 * after the lock is dropped; re-arm the completion so that
+	 * v4l2_m2m_cancel_job() waits for the tail to finish before
+	 * releasing the instance.
+	 */
 	wake_up(&m2m_dev->curr_ctx->finished);
+	reinit_completion(&m2m_dev->curr_ctx->scheduling_done);
 	m2m_dev->curr_ctx = NULL;
 	return true;
 }
@@ -1245,6 +1267,11 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
 	m2m_ctx->priv = drv_priv;
 	m2m_ctx->m2m_dev = m2m_dev;
 	init_waitqueue_head(&m2m_ctx->finished);
+	init_completion(&m2m_ctx->scheduling_done);
+	/* Start as done: a context that never runs a job must not block
+	 * in v4l2_m2m_cancel_job(). reinit_completion() below re-arms it
+	 * each time a scheduling tail actually starts. */
+	complete_all(&m2m_ctx->scheduling_done);
 
 	out_q_ctx = &m2m_ctx->out_q_ctx;
 	cap_q_ctx = &m2m_ctx->cap_q_ctx;
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 31de25d792b98..89c8f10de67c7 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -95,7 +95,7 @@ struct v4l2_m2m_queue_ctx {
  * @out_q_ctx: Output (input from memory) queue context
  * @queue: List of memory to memory contexts
  * @job_flags: Job queue flags, used internally by v4l2-mem2mem.c:
- *		%TRANS_QUEUED, %TRANS_RUNNING and %TRANS_ABORT.
+ *		%TRANS_QUEUED, %TRANS_RUNNING, %TRANS_ABORT and %TRANS_SCHEDULING.
  * @finished: Wait queue used to signalize when a job queue finished.
  * @priv: Instance private data
  *
@@ -125,6 +125,7 @@ struct v4l2_m2m_ctx {
 	struct list_head		queue;
 	unsigned long			job_flags;
 	wait_queue_head_t		finished;
+	struct completion		scheduling_done;
 
 	void				*priv;
 };
-- 
2.34.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] media: v4l2-mem2mem: wait for the job-finish scheduling tail
  2026-11-04  9:30 [PATCH] media: v4l2-mem2mem: wait for the job-finish scheduling tail Yaohan Li
@ 2026-11-12  9:41 ` hverkuil+cisco
  0 siblings, 0 replies; 2+ messages in thread
From: hverkuil+cisco @ 2026-11-12  9:41 UTC (permalink / raw)
  To: Yaohan Li, mchehab
  Cc: Frank.Li, ming.qian, nicolas.dufresne, dmantipov, 121811682,
	ezequiel, linux-media, linux-kernel

On 04/11/2026 10:30, Yaohan Li wrote:
> The job-finish path clears TRANS_RUNNING and TRANS_QUEUED before its
> follow-up scheduling tail has finished using the mem2mem context.
> If release races with that tail, v4l2_m2m_cancel_job() can observe no
> running or queued job and allow the context to be freed while the tail
> still accesses it.
> 
> Track the scheduling tail with a completion and wait for it in the
> cancellation path. Start the completion in the done state so contexts
> that never ran a job do not wait indefinitely, reinitialize it when a
> scheduling tail begins, and complete it after the tail finishes.
> 
> The original race was reproduced as a KASAN use-after-free in the
> common mem2mem code using the in-tree vicodec test driver and targeted
> timing injection.
> 
> Fixes: cbd9463da1b12 ("media: v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish")
> Assisted-by: ZCode:GLM-4

Clearly AI prose. I can't make head nor tail (pun intended) of it.

What was the vicodec race? How was it patched?

Please rewrite the commit log and comments in words that a human would use.

Regards,

	Hans

> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 27 ++++++++++++++++++++++++++
>  include/media/v4l2-mem2mem.h           |  3 ++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index a65cbb124cfe0..7637a876bb8c3 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -439,6 +439,13 @@ static void v4l2_m2m_cancel_job(struct v4l2_m2m_ctx *m2m_ctx)
>  		/* Do nothing, was not on queue/running */
>  		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
>  	}
> +
> +	/*
> +	 * A job-finish tail may still walk this instance after
> +	 * TRANS_RUNNING was cleared; wait for it to finish so the caller
> +	 * can safely release the instance.
> +	 */
> +	wait_for_completion(&m2m_ctx->scheduling_done);
>  }
>  
>  /*
> @@ -460,6 +467,14 @@ static void v4l2_m2m_schedule_next_job(struct v4l2_m2m_dev *m2m_dev,
>  	 * but the job must be run in non-atomic context.
>  	 */
>  	schedule_work(&m2m_dev->job_work);
> +
> +	/*
> +	 * Signal that the scheduling tail is done. complete_all is
> +	 * atomic: once called, all current and future waiters are
> +	 * released, with no lock-free condition window as with
> +	 * wait_event() + a flag bit.
> +	 */
> +	complete_all(&m2m_ctx->scheduling_done);
>  }
>  
>  /*
> @@ -476,7 +491,14 @@ static bool _v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
>  
>  	list_del(&m2m_dev->curr_ctx->queue);
>  	m2m_dev->curr_ctx->job_flags &= ~(TRANS_QUEUED | TRANS_RUNNING);
> +	/*
> +	 * The follow-up scheduling of the same instance still walks it
> +	 * after the lock is dropped; re-arm the completion so that
> +	 * v4l2_m2m_cancel_job() waits for the tail to finish before
> +	 * releasing the instance.
> +	 */
>  	wake_up(&m2m_dev->curr_ctx->finished);
> +	reinit_completion(&m2m_dev->curr_ctx->scheduling_done);
>  	m2m_dev->curr_ctx = NULL;
>  	return true;
>  }
> @@ -1245,6 +1267,11 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>  	m2m_ctx->priv = drv_priv;
>  	m2m_ctx->m2m_dev = m2m_dev;
>  	init_waitqueue_head(&m2m_ctx->finished);
> +	init_completion(&m2m_ctx->scheduling_done);
> +	/* Start as done: a context that never runs a job must not block
> +	 * in v4l2_m2m_cancel_job(). reinit_completion() below re-arms it
> +	 * each time a scheduling tail actually starts. */
> +	complete_all(&m2m_ctx->scheduling_done);
>  
>  	out_q_ctx = &m2m_ctx->out_q_ctx;
>  	cap_q_ctx = &m2m_ctx->cap_q_ctx;
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 31de25d792b98..89c8f10de67c7 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -95,7 +95,7 @@ struct v4l2_m2m_queue_ctx {
>   * @out_q_ctx: Output (input from memory) queue context
>   * @queue: List of memory to memory contexts
>   * @job_flags: Job queue flags, used internally by v4l2-mem2mem.c:
> - *		%TRANS_QUEUED, %TRANS_RUNNING and %TRANS_ABORT.
> + *		%TRANS_QUEUED, %TRANS_RUNNING, %TRANS_ABORT and %TRANS_SCHEDULING.
>   * @finished: Wait queue used to signalize when a job queue finished.
>   * @priv: Instance private data
>   *
> @@ -125,6 +125,7 @@ struct v4l2_m2m_ctx {
>  	struct list_head		queue;
>  	unsigned long			job_flags;
>  	wait_queue_head_t		finished;
> +	struct completion		scheduling_done;
>  
>  	void				*priv;
>  };


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-12  9:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-04  9:30 [PATCH] media: v4l2-mem2mem: wait for the job-finish scheduling tail Yaohan Li
2026-11-12  9:41 ` hverkuil+cisco

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®