Re: [PATCH v8 14/14] media: mediatek: decoder: support av1 extend vsi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] media: Out-of-bounds write to VPU firmware memory due to oversized memcpy
--

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

media: mediatek: decoder: support av1 extend vsi

The patch expands the AV1 VSI state structure to include tile group
information to support the extended architecture. It also updates the
initialization routine to allow smaller shared memory buffers when
running on non-extended firmware.

> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> index 14639098fd463..1ef561dae18c7 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> @@ -665,6 +670,8 @@ struct vdec_av1_slice_vsi {
>  	struct vdec_av1_slice_frame frame;
>  	struct vdec_av1_slice_state state;
>  	u32 cur_lst_tile_id;
> +	struct vdec_av1_slice_tile_group tile_group;
> +	unsigned int reserved[4];
>  };

[ ... ]

> @@ -1942,9 +1978,18 @@ static int vdec_av1_slice_init(struct mtk_vcodec_dec_ctx *ctx)
>  		goto error_vsi;
>  	}
>  
> -	if (vsi->vsi_size != sizeof(struct vdec_av1_slice_vsi))
> -		mtk_vdec_err(ctx, "remote vsi size 0x%x mismatch! expected: 0x%zx\n",
> -			     vsi->vsi_size, sizeof(struct vdec_av1_slice_vsi));
> +	if (IS_VDEC_SUPPORT_EXT(ctx->dev->dec_capability)) {
> +		vsi_size = sizeof(struct vdec_av1_slice_vsi);
> +		vsi->iq_table_size = AV1_IQ_TABLE_SIZE;
> +		vsi->cdf_table_size = AV1_CDF_SIZE;
> +	}
> +
> +	if (vsi->vsi_size != vsi_size) {
> +		mtk_vdec_err(ctx, "remote vsi size 0x%x mismatch! expected: 0x%x\n",
> +			     vsi->vsi_size, vsi_size);
> +		ret = -EINVAL;
> +		goto error_vsi;
> +	}

[Severity: Critical]
This correctly enforces that older firmware provides a 0xD50-byte buffer for
the non-extended architecture.

However, since struct vdec_av1_slice_vsi was expanded with tile_group,
does vdec_av1_slice_vsi_to_remote() now copy too much data?

vdec_av1_slice_vsi_to_remote() {
	memcpy(remote_vsi, vsi, sizeof(*vsi));
}

Since sizeof(*vsi) evaluates to the new, enlarged compile-time size of the
struct, will this unconditionally write ~7.5KB of data into the 0xD50
(3.4KB) firmware buffer when decoding on non-extended hardware?

Could this out-of-bounds write corrupt adjacent data in the VPU shared
memory or firmware SRAM?

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