Re: [PATCH v16 07/12] media: mediatek: jpeg: fix resolution change event handling in decoder

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Data race on V4L2 queue format structures due to unlocked modification from the asynchronous worker thread.

Pre-existing issues:
- [High] Infinite workqueue loop and STREAMOFF deadlock due to missing .job_ready callback in multicore decoder.
--

--- Patch [7]: [PATCH v16 07/12] media: mediatek: jpeg: fix resolution change event handling in decoder ---
commit e12d8cd3e85b476d842a0ea7879755769138198f
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix resolution change event handling in decoder

This patch refines the handling of resolution change events in the JPEG
decoder worker. It updates queue data before signaling the source change and
adds an early exit condition if already in the source change state.

> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1738,11 +1738,15 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  
>  	if (mtk_jpeg_check_resolution_change(ctx,
>  					     &jpeg_src_buf->dec_param)) {
> -		mtk_jpeg_queue_src_chg_event(ctx);
> +		mtk_jpeg_set_queue_data(ctx, &jpeg_src_buf->dec_param);

[Severity: High]
Does calling mtk_jpeg_set_queue_data() from the asynchronous worker thread
create a data race?

This function writes directly to the format structures like ctx->out_q and
ctx->cap_q. Standard V4L2 ioctls like VIDIOC_G_FMT and VIDIOC_TRY_FMT read
these exact structures and rely on the video device lock (vdev->lock) for
serialization.

Since the worker executes without acquiring the device lock, could this
cause userspace to read torn or mismatched dimension/size values if
VIDIOC_G_FMT is called concurrently?

>  		ctx->state = MTK_JPEG_SOURCE_CHANGE;
> +		mtk_jpeg_queue_src_chg_event(ctx);
>  		goto getbuf_fail;
>  	}
>  
> +	if (ctx->state == MTK_JPEG_SOURCE_CHANGE)
> +		goto getbuf_fail;

[Severity: High]
This is a pre-existing issue, but does jumping to getbuf_fail while
leaving buffers in the queue cause an infinite workqueue loop and
deadlock during VIDIOC_STREAMOFF?

In getbuf_fail, v4l2_m2m_job_finish() is called without removing the
source or destination buffers from the queue.

Looking at mtk_jpeg_multicore_dec_m2m_ops, it lacks a .job_ready callback:

static const struct v4l2_m2m_ops mtk_jpeg_multicore_dec_m2m_ops = {
	.device_run = mtk_jpeg_multicore_dec_device_run,
};

Because there is no .job_ready callback, the V4L2 M2M core assumes the
context is ready and immediately requeues the job. The worker will instantly
run again, hit this same early exit, and loop infinitely.

Additionally, since the buffers are never marked done or removed, could any
subsequent attempt by userspace to stop streaming deadlock permanently in
vb2_wait_for_all_buffers()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=7