2023-03-28 09:27:13

by Wei Chen

[permalink] [raw]
Subject: [PATCH] media: vcodec: Fix potential array out-of-bounds in vb2ops_venc_queue_setup

variable *nplanes is provided by user via system call argument. The
possible value of q_data->fmt->num_planes is 1-3, while the value
of *nplanes can be 1-8. The array access by index i can cause array
out-of-bounds.

Fix this bug by checking *nplanes against the array size.

Signed-off-by: Wei Chen <[email protected]>
---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
index d65800a3b89d..1ea02f9136f6 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
@@ -821,6 +821,8 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
return -EINVAL;

if (*nplanes) {
+ if (*nplanes != q_data->fmt->num_planes)
+ return -EINVAL;
for (i = 0; i < *nplanes; i++)
if (sizes[i] < q_data->sizeimage[i])
return -EINVAL;
--
2.25.1


2023-06-14 16:35:53

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH] media: vcodec: Fix potential array out-of-bounds in vb2ops_venc_queue_setup

On Tue, Mar 28, 2023 at 6:26 PM Wei Chen <[email protected]> wrote:
>
> variable *nplanes is provided by user via system call argument. The
> possible value of q_data->fmt->num_planes is 1-3, while the value
> of *nplanes can be 1-8. The array access by index i can cause array
> out-of-bounds.
>
> Fix this bug by checking *nplanes against the array size.
>
> Signed-off-by: Wei Chen <[email protected]>
> ---
> drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> index d65800a3b89d..1ea02f9136f6 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> @@ -821,6 +821,8 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
> return -EINVAL;
>
> if (*nplanes) {
> + if (*nplanes != q_data->fmt->num_planes)
> + return -EINVAL;
> for (i = 0; i < *nplanes; i++)
> if (sizes[i] < q_data->sizeimage[i])
> return -EINVAL;
> --
> 2.25.1
>

Thanks for the patch, it makes sense indeed.

Reviewed-by: Tomasz Figa <[email protected]>

Best regards,
Tomasz

2023-07-27 02:17:15

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH] media: vcodec: Fix potential array out-of-bounds in vb2ops_venc_queue_setup

Hi Mauro, Hans,

On Thu, Jun 15, 2023 at 1:23 AM Tomasz Figa <[email protected]> wrote:
>
> On Tue, Mar 28, 2023 at 6:26 PM Wei Chen <[email protected]> wrote:
> >
> > variable *nplanes is provided by user via system call argument. The
> > possible value of q_data->fmt->num_planes is 1-3, while the value
> > of *nplanes can be 1-8. The array access by index i can cause array
> > out-of-bounds.
> >
> > Fix this bug by checking *nplanes against the array size.
> >
> > Signed-off-by: Wei Chen <[email protected]>
> > ---
> > drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> > index d65800a3b89d..1ea02f9136f6 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> > @@ -821,6 +821,8 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
> > return -EINVAL;
> >
> > if (*nplanes) {
> > + if (*nplanes != q_data->fmt->num_planes)
> > + return -EINVAL;
> > for (i = 0; i < *nplanes; i++)
> > if (sizes[i] < q_data->sizeimage[i])
> > return -EINVAL;
> > --
> > 2.25.1
> >
>
> Thanks for the patch, it makes sense indeed.
>
> Reviewed-by: Tomasz Figa <[email protected]>

Do you plan to pick this patch? Thanks.

Best regards,
Tomasz

2023-08-09 07:05:06

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH] media: vcodec: Fix potential array out-of-bounds in vb2ops_venc_queue_setup

On Tue, Mar 28, 2023 at 09:26:08AM +0000, Wei Chen wrote:
> variable *nplanes is provided by user via system call argument. The
> possible value of q_data->fmt->num_planes is 1-3, while the value
> of *nplanes can be 1-8. The array access by index i can cause array
> out-of-bounds.
>
> Fix this bug by checking *nplanes against the array size.

This should have a fixes tag:

Fixes: 4e855a6efa54 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver")

And also CC the stable list:

Cc: [email protected]

> Signed-off-by: Wei Chen <[email protected]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

> ---
> drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> index d65800a3b89d..1ea02f9136f6 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> @@ -821,6 +821,8 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
> return -EINVAL;
>
> if (*nplanes) {
> + if (*nplanes != q_data->fmt->num_planes)
> + return -EINVAL;
> for (i = 0; i < *nplanes; i++)
> if (sizes[i] < q_data->sizeimage[i])
> return -EINVAL;
> --
> 2.25.1
>