Re: [PATCH v9 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect computation of the 34-bit extension for the encoder stall address corrupts hardware registers.
- [High] The asynchronous `job_timeout_work` is not canceled on driver removal, leading to a Use-After-Free.
--

--- Patch [3]: [PATCH v9 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC ---
commit 29466227a18e4e4c49341e3e3e858dbacaf50d59
Author: Jianhua Lin <jianhua.lin@mediatek.com>

media: mediatek: jpeg: add compatible for MT8189 SoC

This commit adds support for the MT8189 SoC's JPEG encoder and decoder by
providing new compatible strings ("mediatek,mt8189-jpgenc" and
"mediatek,mt8189-jpgdec") and their associated driver data. It includes
enabling 34-bit IOVA address space support for this SoC.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index d147ec4830811..14f2991a40535 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
[ ... ]
> @@ -1899,6 +1903,38 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
>  	.multi_core = false,
>  };
>  
> +static const struct mtk_jpeg_variant mtk8189_jpegenc_drvdata = {
> +	.clks = mtk_jpeg_clocks,
> +	.num_clks = ARRAY_SIZE(mtk_jpeg_clocks),
> +	.formats = mtk_jpeg_enc_formats,
> +	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
> +	.qops = &mtk_jpeg_enc_qops,
> +	.irq_handler = mtk_jpeg_enc_irq,
> +	.hw_reset = mtk_jpeg_enc_reset,
> +	.m2m_ops = &mtk_jpeg_enc_m2m_ops,
> +	.dev_name = "mtk-jpeg-enc",
> +	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
> +	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
> +	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> +	.support_34bit = true,
       ^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but enabling 34-bit support exposes
a pre-existing mathematical error in the hardware register configuration.

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c:mtk_jpeg_set_enc_dst(),
the 34-bit extension for the encoder stall address is incorrectly calculated by
adding the size directly to the extracted upper 32 bits:

    if (support_34bit) {
        addr_ext = FIELD_PREP(MTK_JPEG_ADDR_MASK, upper_32_bits(dma_addr));
        writel(addr_ext, base + JPEG_ENC_DEST_ADDR0_EXT);
        writel(addr_ext + size, base + JPEG_ENC_STALL_ADDR0_EXT);
    }

Does adding the size to addr_ext result in a corrupted garbage value being
written to the JPEG_ENC_STALL_ADDR0_EXT register, since it adds the buffer
byte size to the 2-bit extension mask rather than extracting the upper 2 bits
of the combined dma_addr + size?

Can this cause the hardware to fault when the physical address crosses the 4GB
boundary?

> +};
> +
> +static const struct mtk_jpeg_variant mtk8189_jpegdec_drvdata = {
[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
during driver teardown that affects all variants.

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpeg_single_core_init(),
a delayed work is initialized for job timeouts:

    INIT_DELAYED_WORK(&jpeg->job_timeout_work,
                      mtk_jpeg_job_timeout_work);

However, in mtk_jpeg_remove(), this delayed work is never explicitly canceled:

static void mtk_jpeg_remove(struct platform_device *pdev)
{
    struct mtk_jpeg_dev *jpeg = platform_get_drvdata(pdev);

    pm_runtime_disable(&pdev->dev);
    video_unregister_device(jpeg->vdev);
    v4l2_m2m_release(jpeg->m2m_dev);
    v4l2_device_unregister(&jpeg->v4l2_dev);
}

If a job timeout is pending when the device is unbound via sysfs or module
removal, could the timer subsequently fire and execute
mtk_jpeg_job_timeout_work()?

Would this dereference the already freed jpeg->m2m_dev and jpeg structures,
leading to a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709032657.25730-1-jianhua.lin@mediatek.com?part=3