linux-kernel - Re: [PATCH] OOB read and write in mtk multiple places

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230627134314.GA3605030@gnbcxd0016.gnb.st.com>
Date:   Tue, 27 Jun 2023 15:43:51 +0200
From:   Alain Volmat <alain.volmat@...s.st.com>
To:     sun yq <yqsun1997@...il.com>
CC:     <tiffany.lin@...iatek.com>, <andrew-ct.chen@...iatek.com>,
        <yunfei.dong@...iatek.com>, <matthias.bgg@...il.com>,
        <angelogioacchino.delregno@...labora.com>,
        <linux-media@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>, <499671216@...com>
Subject: Re: [PATCH] OOB read and write in mtk multiple places

Hi,

On Tue, Jun 27, 2023 at 08:28:11PM +0800, sun yq wrote:
> 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
o> mtk_dip_vb2_video_queue_setup, the max plane size of this module is 8

To my understanding, the vcodec driver only has formats requiring up to
3 planes.  Could you explain why you need to change this macro to 8 ?
Could you please describe the issue you had which led to this change ?

> 
> 
> On Tue, Jun 27, 2023 at 6:42 PM Alain Volmat <alain.volmat@...s.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@...il.com wrote:
> > > From: yqsun1997 <yqsun1997@...il.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@...il.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
> > >