Re: [PATCH v16 06/12] media: mediatek: jpeg: fix buffer state update timing

[sashiko-bot@kernel.org]

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

New issues:
- [High] Moving destination buffer state initialization after scheduling the timeout worker introduces a race condition causing list corruption and a kernel oops.

Pre-existing issues:
- [High] The hardware timeout workers are scheduled before hardware start and completely lack lock synchronization, causing data races, hardware bus faults, and use-after-free vulnerabilities.
- [High] The encoder worker permanently leaks a Runtime PM usage reference when `pm_runtime_get_sync()` fails.
--

--- Patch [6]: [PATCH v16 06/12] media: mediatek: jpeg: fix buffer state update timing ---
commit adfbbc474f76db07fc27613935d0bb09341844c6
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix buffer state update timing

Update the destination buffer state only after the decoder has selected
a hardware core and resumed its runtime PM state successfully. This avoids
assigning frame tracking data to buffers that will be returned early, such
as when a source resolution change is detected.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index a5a1f6126af73..e152ebae0f219 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
[ ... ]
> @@ -1773,6 +1767,9 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
>  
>  	spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags);
> +	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
> +	jpeg_dst_buf->curr_ctx = ctx;
> +	jpeg_dst_buf->frame_num = ctx->total_frame_num;

[Severity: High]
Does moving this initialization after schedule_delayed_work() create a race
condition that can corrupt the destination done queue?

If the worker thread is preempted right after calling schedule_delayed_work(),
the timeout worker (mtk_jpegdec_timeout_work) can fire early before
jpeg_dst_buf->frame_num is initialized.

The timeout worker will pass this uninitialized buffer to mtk_jpegdec_put_buf(),
which adds it to ctx->dst_done_queue. Because frame_num is uninitialized,
it won't match ctx->last_done_frame_num and the buffer gets stranded in the
queue.

When the worker thread resumes, it initializes frame_num and starts the
hardware. Upon hardware completion, the IRQ handler will call
mtk_jpegdec_put_buf() again and execute list_add_tail() on the exact same
buffer, corrupting the linked list.

[Severity: High]
This isn't a bug introduced by this patch, but could this delayed work
scheduling cause hardware bus faults or memory corruption?

The timeout worker is armed before the hardware lock is acquired and the
hardware is started. Since the timeout workers (both mtk_jpegdec_timeout_work
and mtk_jpegenc_timeout_work) do not acquire hw_lock, if the main worker is
preempted right after scheduling the work, the timeout worker can fire
concurrently.

It will reset the device, complete the buffers to userspace, and call
pm_runtime_put() to potentially power off the device. When the main worker
resumes, it will write to powered-off registers (causing a bus fault), and
instruct the hardware to DMA into buffers that userspace may have already
freed or reused.

[Severity: High]
This is a pre-existing issue, but is there a Runtime PM reference leak in the
encoder worker's failure path?

In mtk_jpegenc_worker() (not modified in this diff), pm_runtime_get_sync()
increments the device's PM usage counter even if it returns an error. If the
resume operation fails, the code branches to the enc_end error path and
returns without calling pm_runtime_put_noidle(). This leaves the usage
counter elevated and permanently prevents the device from suspending again.

>  	ctx->total_frame_num++;
>  	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
>  	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,

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