Re: [PATCH v17 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 1 potential issue(s) to consider:
- [High] media: mediatek: jpeg: data race in mtk_jpegdec_worker() on format queues
--

commit 35ce87a0072442a39f5ea440c3ef1d674406f548
Author: Kyrie Wu <kyrie.wu@mediatek.com>

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

This commit refines the handling of resolution change events within the
JPEG decoder worker. It reorganizes operations to set up queue data before
signaling a source change and adds an early exit condition if the context is
already in the source change state.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index e152ebae0f219..edd9e2d0ab05f 100644
> --- 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 this introduce a data race with concurrent V4L2 ioctl handlers?

The lockless worker thread mtk_jpegdec_worker() appears to be updating the
V4L2 context format queues (ctx->cap_q and ctx->out_q) asynchronously through
mtk_jpeg_set_queue_data() without holding the video device lock.

Concurrent ioctls such as VIDIOC_G_FMT read these queue structures under
the video device lock. For example, in mtk_jpeg_g_fmt_vid_mplane():

    pix_mp->pixelformat = q_data->fmt->fourcc;
    pix_mp->num_planes = q_data->fmt->colplanes;

Could a local userspace application streaming a JPEG that changes
resolution trigger this race while concurrently invoking format-querying
ioctls, potentially exposing inconsistent format fields (like width, height,
and bytesperline) to userspace?

>  		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;
> +
>  	mtk_jpegdec_set_hw_param(ctx, hw_id, src_buf, dst_buf);
>  	ret = pm_runtime_resume_and_get(comp_jpeg[hw_id]->dev);
>  	if (ret < 0) {

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