Re: [PATCH v5 10/14] media: mediatek: vcodec: send share memory address to vcp

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The size of `struct vdec_ap_ipi_init` is increased from 16 to 24 bytes, and this larger size is unconditionally sent to all firmware architectures, risking backward compatibility breakage and firmware buffer overflows for older VPU/SCP implementations.
--

commit d463b7b0f56f0354049dbd8919b4208b451e2182
Author: Yunfei Dong <yunfei.dong@mediatek.com>

media: mediatek: vcodec: send share memory address to vcp

The share memory is allocated in kernel for vcp architecture, it's
different with vpu which share memors is reserved in vpu micro
processor. Need to send share memory address to vcp.

> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> @@ -67,11 +67,13 @@ struct vdec_vpu_ipi_ack {
>   * @msg_id	: AP_IPIMSG_DEC_INIT
>   * @codec_type	: codec fourcc
>   * @ap_inst_addr	: AP video decoder instance address
> + * @shared_iova	: reserved share memory address
>   */
>  struct vdec_ap_ipi_init {
>  	uint32_t msg_id;
>  	u32 codec_type;
>  	uint64_t ap_inst_addr;
> +	u64 shared_iova;
>  };

[ ... ]

> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> @@ -236,6 +236,8 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
>  	msg.msg_id = AP_IPIMSG_DEC_INIT;
>  	msg.ap_inst_addr = (unsigned long)vpu;
>  	msg.codec_type = vpu->codec_type;
> +	if (mtk_vcodec_fw_get_type(vpu->ctx->dev->fw_handler) == VCP)
> +		msg.shared_iova = vpu->ctx->dev->fw_handler->vcp->iova_addr;
>  
>  	mtk_vdec_debug(vpu->ctx, "vdec_inst=%p", vpu);

[Severity: High]
Could this structure size increase break compatibility with legacy VPU and SCP
firmwares?

Adding shared_iova to struct vdec_ap_ipi_init increases the message size
from 16 to 24 bytes. A few lines later in vpu_dec_init(), the message is
sent unconditionally using the new larger size:

    err = vcodec_vpu_send_msg(vpu, (void *)&msg, sizeof(msg));

If legacy firmware implementations expect exactly 16 bytes for
AP_IPIMSG_DEC_INIT, could sending a 24-byte payload cause initialization
failures or stack buffer overflows in the legacy firmware?

Perhaps the driver should only send the expanded length when communicating
with the VCP architecture, or use offsetof() to send the original 16-byte
size to older targets.

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