Re: [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

Le jeudi 23 juin 2022 à 15:42 -0400, Justin Green a écrit :
> On Thu, Jun 23, 2022 at 3:31 PM Nicolas Dufresne
> <nicolas.dufresne@collabora.com> wrote:
> > 
> > Le jeudi 23 juin 2022 à 15:14 -0400, Justin Green a écrit :
> > > Validate V4L2 plane data_offset values. We need to make sure the size of
> > > the image we're encoding does not exceed the size of the buffer minus
> > > its offset.
> > > 
> > > Signed-off-by: Justin Green <greenjustin@google.com>
> > > ---
> > >  drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > > index bc5b0a0168ec..8f5c1b9937bc 100644
> > > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > > @@ -687,6 +687,10 @@ static int mtk_jpeg_buf_prepare(struct vb2_buffer *vb)
> > > 
> > >       for (i = 0; i < q_data->fmt->colplanes; i++) {
> > >               plane_fmt = q_data->pix_mp.plane_fmt[i];
> > > +                if (vb->planes[i].data_offset > vb2_plane_size(vb, i) ||
> > > +                    vb2_plane_size(vb, i) - vb->planes[i].data_offset
> > > +                    < plane_fmt.sizeimage)
> > > +                    return -EINVAL;
> > 
> > Just double checking, but has data_offset been verified already to prevent the
> > underflow ?
> 
> I believe the "vb->planes[i].data_offset > vb2_plane_size(vb, i)"
> check should do that, right?

Perfect, with that said:

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

If no new version required, would it be possible to add:

Fixes: 45f13a57d8134 ("media: platform: Add jpeg enc feature")

regards,
Nicolas
> 
> > 
> > >               if (ctx->enable_exif &&
> > >                   q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG)
> > >                       vb2_set_plane_payload(vb, i, plane_fmt.sizeimage +
> >