Re: [PATCH v18 11/13] media: mediatek: jpeg: add jpeg compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: mediatek: jpeg: incorrect STALL address extension calculation in 34-bit mode
--

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

media: mediatek: jpeg: add jpeg compatible
    
Add jpeg dec and enc compatible for mt8196

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 0907960db1436..8e96501a4f9fa 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1919,6 +1919,20 @@ static struct mtk_jpeg_variant mtk8195_jpegenc_drvdata = {
>  	.jpeg_worker = mtk_jpegenc_worker,
>  };
>  
> +static struct mtk_jpeg_variant mtk8196_jpegenc_drvdata = {
> +	.formats = mtk_jpeg_enc_formats,
> +	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
> +	.qops = &mtk_jpeg_enc_qops,
> +	.m2m_ops = &mtk_jpeg_multicore_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,
> +	.multi_core = true,
> +	.jpeg_worker = mtk_jpegenc_worker,
> +	.support_34bit = true,

[Severity: High]
This problem wasn't introduced by this patch, but setting support_34bit to true
makes a broken 34-bit code path reachable for the first time.

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c:mtk_jpeg_set_enc_dst(),
the STALL address extension is calculated by adding the buffer size directly to
the 2-bit extension address:

	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);
	}

Since size is in bytes, won't adding it to the 2-bit addr_ext write a massively
out-of-bounds value to the hardware extension register, rather than correctly
extracting the upper 32 bits of dma_addr + size?

Could this corrupt the DMA engine's STALL boundary setup and risk memory
corruption or IOMMU faults?

> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791614327.git.kyrie.wu@mediatek.com?part=11