Re: [PATCH v17 12/12] media: mediatek: jpeg: add jpeg smmu sid setting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] media: mediatek: jpeg: Probe failure UAF due to uncleared array entries in smmu init
- [Low] media: mediatek: jpeg: Missing const on mtk8196_jpegenc_drvdata
--

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

media: mediatek: jpeg: add jpeg smmu sid setting
    
Add a configuration to set jpeg dec & enc smmu sid

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 8e96501a4f9fa..bb9bdd9913d94 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
[ ... ]
> @@ -1931,6 +1958,7 @@ static struct mtk_jpeg_variant mtk8196_jpegenc_drvdata = {
>  	.multi_core = true,
>  	.jpeg_worker = mtk_jpegenc_worker,
>  	.support_34bit = true,
> +	.support_smmu = true,
>  };

[Severity: Low]
Is this struct missing the const qualifier?  Without it,
mtk8196_jpegenc_drvdata will be mapped to writable memory instead of
read-only data.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> index 07452249bcb67..3058fdf4f6054 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
[ ... ]
> @@ -665,6 +684,10 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
>  	master_dev->reg_decbase[i] = dev->reg_base;
>  	dev->master_dev = master_dev;
>  
> +	ret = mtk_jpegdec_smmu_init(dev);
> +	if (ret)
> +		return ret;
> +
>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
>  

[Severity: High]
Does this code leave a dangling pointer in master_dev->dec_hw_dev[i] when
mtk_jpegdec_smmu_init() fails?

In mtk_jpegdec_hw_probe(), if mtk_jpegdec_smmu_init() fails during child
probe, it returns an error and devm_kzalloc() will free the dev structure.
However, dev is already stored in the parent's array
(master_dev->dec_hw_dev[i] = dev) and master_dev->hw_index was incremented
just above this snippet.

Can a subsequent job queued to the V4L2 device trigger mtk_jpegdec_worker()
to scan the hardware array and dereference the dangling pointer, causing
a use-after-free?

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> index c7954af0eb65a..506a49d1ac04d 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
[ ... ]
> @@ -388,6 +407,10 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
>  	master_dev->reg_encbase[i] = dev->reg_base;
>  	dev->master_dev = master_dev;
>  
> +	ret = mtk_jpegenc_smmu_init(dev);
> +	if (ret)
> +		return ret;
> +
>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
>  

[Severity: High]
Does this code leave a dangling pointer in master_dev->enc_hw_dev[i] when
mtk_jpegenc_smmu_init() fails?

In mtk_jpegenc_hw_probe(), similar to the decode probe above, if
mtk_jpegenc_smmu_init() fails, dev is freed but remains stored in the
parent's array (master_dev->enc_hw_dev[i] = dev) with master_dev->hw_index
already incremented.

Can a subsequent job queued to the V4L2 device trigger mtk_jpegenc_worker()
to dereference this freed pointer?

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