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;
MIME-Version: 1.0
References: <20230627081002.1768990-1-yqsun1997@gmail.com> <20230627104237.GA3601890@gnbcxd0016.gnb.st.com>
In-Reply-To: <20230627104237.GA3601890@gnbcxd0016.gnb.st.com>
From:   sun yq <yqsun1997@gmail.com>
Date:   Tue, 27 Jun 2023 20:28:11 +0800
Message-ID: <CAA0BgY_bKg4qXmtmWqHQW+EOvfXUG0FqRtAJ7A8C40uL1v8qVA@mail.gmail.com>
Subject: Re: [PATCH] OOB read and write in mtk multiple places
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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

When using V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, the number of planes is
controlled by the user. Only checking the oob at the function may miss
other functions, so it is appropriate to change the size of the macro.
You can refer to other functions, such as
mtk_dip_vb2_video_queue_setup, the max plane size of this module is 8


On Tue, Jun 27, 2023 at 6:42=E2=80=AFPM Alain Volmat <alain.volmat@foss.st.=
com> wrote:
>
> 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 =3D=3D 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) >=3D MTK_VDEC_LAT_SINGLE_=
CORE)
> > --
> > 2.39.2
> >