Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 27 Jun 2023 12:42:37 +0200
From:   Alain Volmat <alain.volmat@foss.st.com>
To:     <yqsun1997@gmail.com>
CC:     <tiffany.lin@mediatek.com>, <andrew-ct.chen@mediatek.com>,
        <yunfei.dong@mediatek.com>, <matthias.bgg@gmail.com>,
        <angelogioacchino.delregno@collabora.com>,
        <linux-media@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-mediatek@lists.infradead.org>, <499671216@qq.com>
Subject: Re: [PATCH] OOB read and write in mtk multiple places
Message-ID: <20230627104237.GA3601890@gnbcxd0016.gnb.st.com>
Mail-Followup-To: yqsun1997@gmail.com, tiffany.lin@mediatek.com,
        andrew-ct.chen@mediatek.com, yunfei.dong@mediatek.com,
        matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, 499671216@qq.com
References: <20230627081002.1768990-1-yqsun1997@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230627081002.1768990-1-yqsun1997@gmail.com>
Precedence: bulk

Hi,

I had a look at some places where this macro MTK_VCODEC_MAX_PLANES
is being used, such as q_data->bytesperline etc.
This patch seems to be increasing the table size from 3 to 8 but,
if my understanding is correct doesn't solve the issue that
(taking the example you give in vidioc_venc_g_fmt) the table
bytesperline is accessed taking into account a num_planes values which
is unchecked if appropriate for this driver.

What are the 8 planes you are referring to ?

While increasing the table to 8 might also be necessary, it seems to me
that the real OOB access issue should be solved by checking the num of
planes value.

Regards,
Alain

On Tue, Jun 27, 2023 at 04:10:02PM +0800, yqsun1997@gmail.com wrote:
> From: yqsun1997 <yqsun1997@gmail.com>
> 
> The num_planes max index is 8,
> but bytesperline and bytesperline in struct mtk_q_data,
> The max index is MTK_VCODEC_MAX_PLANES == 3,
> so will cause OOB read and write in multiple places.like vidioc_venc_g_fmt
> same as commit 8fbcf730
> 
> Signed-off-by: yqsun1997 <yqsun1997@gmail.com>
> ---
>  drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> index 9acab54fd..c2c157675 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> @@ -22,7 +22,7 @@
>  #define MTK_VCODEC_DEC_NAME	"mtk-vcodec-dec"
>  #define MTK_VCODEC_ENC_NAME	"mtk-vcodec-enc"
>  
> -#define MTK_VCODEC_MAX_PLANES	3
> +#define MTK_VCODEC_MAX_PLANES	8
>  #define MTK_V4L2_BENCHMARK	0
>  #define WAIT_INTR_TIMEOUT_MS	1000
>  #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= MTK_VDEC_LAT_SINGLE_CORE)
> -- 
> 2.39.2
>