Re: [PATCH] media: mediatek: vcodec: fix use-after-free in decoder release path

[Nicolas Dufresne]

Le lundi 15 juin 2026 à 16:05 +0200, Doruk Tan Ozturk a écrit :
> fops_vcodec_release() frees the decoder context with kfree(ctx) but
> never cancels the per-context decode_work worker first. Although
> v4l2_m2m_ctx_release() waits for any in-flight m2m job to finish, the
> workqueue handler (mtk_vdec_worker) may still be running and accessing
> the context after v4l2_m2m_job_finish() returns. Once kfree(ctx) runs,
> that worker dereferences freed memory, resulting in a use-after-free.
> 
> Cancel the pending decode work with cancel_work_sync(&ctx->decode_work)
> after the controls and m2m context are torn down and before kfree(ctx),
> mirroring the fix already applied to the encoder release path in
> commit 76e35091ffc7 ("media: mediatek: vcodec: fix use-after-free in encoder release path").
> 
> decode_work is always initialised before release can run:
> fops_vcodec_open() calls mtk_vcodec_dec_set_default_params() (its only
> caller) unconditionally after a successful v4l2_m2m_ctx_init(), and that
> function runs INIT_WORK(&ctx->decode_work, ...). A context can only reach
> fops_vcodec_release() via an open() that returned 0, i.e. one that passed
> that INIT_WORK. cancel_work_sync() on a properly initialised work_struct
> is therefore always safe, even if the work was never queued. This is
> unlike the 2023 msg_queue->core_work regression, where the work item
> could be uninitialised at cancel time.
> 
> Fixes: 590577a4e525 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Doruk Tan Ozturk <doruk@xxxxxxx>
> ---
>  .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c         | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> index e936ed8dffbaf..30906b24c608a 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> @@ -313,6 +313,15 @@ static int fops_vcodec_release(struct file *file)
>  	v4l2_fh_exit(&ctx->fh);
>  	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
>  
> +	/*
> +	 * Cancel any pending decode work before freeing the context.
> +	 * Although v4l2_m2m_ctx_release() waits for m2m job completion,
> +	 * the workqueue handler (mtk_vdec_worker) may still be accessing
> +	 * the context after v4l2_m2m_job_finish() returns. Without this,
> +	 * a use-after-free occurs when the worker accesses ctx after kfree.
> +	 */
> +	cancel_work_sync(&ctx->decode_work);

The worker is active if there is a job active, but v4l2_m2m_ctx_release() will
exit this condition by synchronously waiting for jobs to finish. This is common
mistake in AI code reviews.

Since this is redundant, NAK

Nicolas

> +
>  	mtk_vcodec_dbgfs_remove(dev, ctx->id);
>  	spin_lock_irqsave(&dev->dev_ctx_lock, flags);
>  	list_del_init(&ctx->list);

Attachment: signature.asc
Description: This is a digitally signed message part