Re: [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing check on `mtk_vcodec_fw_get_ipi()` return value leads to out-of-bounds memory write during VPU IPI registration
--

--- Patch [4]: [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id ---
commit 74021a635ff4b0a5bb38e1b68df4fa4d276cadb5
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: vcodec: get different firmware ipi id

This commit introduces a helper function to get the IPI ID based on
firmware type and hardware index for different architectures. It replaces
hardcoded SCP IPI IDs with this helper in various decoder interfaces.

> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> index 552d0d8a84444..8630e871d78e2 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> @@ -3,6 +3,20 @@
>  #include "../decoder/mtk_vcodec_dec_drv.h"
>  #include "../encoder/mtk_vcodec_enc_drv.h"
>  #include "mtk_vcodec_fw_priv.h"
> +#include "mtk_vcodec_fw_vcp.h"
> +
> +int mtk_vcodec_fw_get_ipi(enum mtk_vcodec_fw_type type, int hw_id)
> +{
> +	switch (type) {
> +	case SCP:
> +		return hw_id == MTK_VDEC_LAT0 ? SCP_IPI_VDEC_LAT : SCP_IPI_VDEC_CORE;
> +	case VCP:
> +		return hw_id == MTK_VDEC_LAT0 ? VCP_IPI_LAT_DECODER : VCP_IPI_CORE_DECODER;
> +	default:
> +		return -EINVAL;
> +	}
> +}

[ ... ]

> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> index 10359ce9b934e..69d60717181a6 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> @@ -1214,8 +1215,8 @@ static int vdec_h264_slice_init(struct mtk_vcodec_dec_ctx *ctx)
>  
>  	inst->ctx = ctx;
>  
> -	inst->vpu.id = SCP_IPI_VDEC_LAT;
> -	inst->vpu.core_id = SCP_IPI_VDEC_CORE;
> +	inst->vpu.id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_LAT0);
> +	inst->vpu.core_id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_CORE);

[Severity: High]
Does this code need to check the return value of mtk_vcodec_fw_get_ipi()
before assigning it to inst->vpu.id?

If the firmware type is not SCP or VCP, mtk_vcodec_fw_get_ipi() returns
-EINVAL. This assigns -22 to inst->vpu.id, which then gets passed down
through vpu_dec_init() into vpu_ipi_register():

drivers/media/platform/mediatek/vpu/mtk_vpu.c:vpu_ipi_register() {
    ...
    if (id < IPI_MAX && handler) {
        ipi_desc = vpu->ipi_desc;
        ipi_desc[id].name = name;
        ipi_desc[id].handler = handler;
        ipi_desc[id].priv = priv;
        return 0;
    }
    ...
}

Since the bounds check uses a signed comparison, -22 < 7 evaluates to true,
which could lead to an out-of-bounds write to ipi_desc[-22] and corrupt
memory. This same pattern also appears in the slice initialization routines
for AV1, HEVC, VP8, and VP9.

Could this negative ID assignment be prevented by validating the return
value of mtk_vcodec_fw_get_ipi()?

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