Re: [PATCH v16 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:

New issues:
- [Critical] Probe failure in SMMU initialization leaves dangling pointers in the global master device arrays and permanently increments hw_index, causing UAF and array OOB on deferred probes.

Pre-existing issues:
- [High] Blind dereference of HW arrays when child probes fail early.
--

--- Patch [12]: [PATCH v16 12/12] media: mediatek: jpeg: add jpeg smmu sid setting ---
commit 73b8529184733966196568926fff60cf629320ae
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
[ ... ]
> @@ -1663,6 +1680,11 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  	jpeg_dst_buf->frame_num = ctx->total_frame_num;
>  	ctx->total_frame_num++;
>  	mtk_jpeg_enc_reset(comp_jpeg[hw_id]->reg_base);
> +
> +	if (jpeg->variant->support_smmu && comp_jpeg[hw_id]->smmu_regmap)
> +		mtk_jpeg_set_smmu_sid(comp_jpeg[hw_id]->smmu_regmap,
> +				      JPG_REG_GUSER_ID_ENC_SID);
> +

[Severity: High]
This isn't a bug introduced by this patch, but I have a question about how
unpopulated hardware array entries are handled when a child probe fails.

In mtk_jpegenc_worker(), it calls mtk_jpegenc_get_hw() to select the hw_id.
If an earlier probe failure left the array entry in enc_hw_dev as NULL, could
this result in a NULL pointer dereference?

mtk_jpegenc_get_hw() iterates based on max_hw_count (which is set from the
Device Tree child nodes) and accesses the state directly:

drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpegenc_get_hw() {
    ...
    for (i = 0; i < jpeg->max_hw_count; i++) {
        comp_jpeg = jpeg->enc_hw_dev[i];
        if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
    ...
}

Could this blindly dereference comp_jpeg if the child probe never completed
and left it NULL? The exact same pattern seems to exist in mtk_jpegdec_get_hw().

>  	mtk_jpeg_set_enc_dst(ctx,
>  			     comp_jpeg[hw_id]->reg_base,
>  			     &dst_buf->vb2_buf);
[ ... ]
> 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;
> +

[Severity: Critical]
If mtk_jpegdec_smmu_init() fails and returns an error like -EPROBE_DEFER,
will this leave a dangling pointer in the master_dev->dec_hw_dev array?

Looking earlier in mtk_jpegdec_hw_probe(), dev is allocated with devm_kzalloc
and added to the master_dev array, and hw_index is incremented:

    i = atomic_add_return(1, &master_dev->hw_index) - 1;
    master_dev->dec_hw_dev[i] = dev;
    master_dev->reg_decbase[i] = dev->reg_base;
    dev->master_dev = master_dev;

    ret = mtk_jpegdec_smmu_init(dev);
    if (ret)
        return ret;

When returning ret, the devm-managed dev will be freed, but the pointer
remains in master_dev->dec_hw_dev[i] and hw_index is never decremented.
Could this lead to an out-of-bounds write on deferred probe retries and a
use-after-free when the worker thread accesses the array?

This same issue seems to be present in mtk_jpegenc_hw_probe() in
mtk_jpeg_enc_hw.c as well.

>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
>

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