* [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes
@ 2022-06-23 19:14 Justin Green
2022-06-23 19:31 ` Nicolas Dufresne
2022-06-29 9:56 ` Hans Verkuil
0 siblings, 2 replies; 6+ messages in thread
From: Justin Green @ 2022-06-23 19:14 UTC (permalink / raw)
To: linux-media
Cc: tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg,
nicolas.dufresne, andrescj, yunfei.dong, Justin Green,
Justin Green
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;
if (ctx->enable_exif &&
q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG)
vb2_set_plane_payload(vb, i, plane_fmt.sizeimage +
--
2.37.0.rc0.104.g0611611a94-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes
2022-06-23 19:14 [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes Justin Green
@ 2022-06-23 19:31 ` Nicolas Dufresne
2022-06-23 19:42 ` Justin Green
2022-06-29 9:56 ` Hans Verkuil
1 sibling, 1 reply; 6+ messages in thread
From: Nicolas Dufresne @ 2022-06-23 19:31 UTC (permalink / raw)
To: Justin Green, linux-media
Cc: tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg, andrescj,
yunfei.dong, Justin Green
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 ?
> if (ctx->enable_exif &&
> q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG)
> vb2_set_plane_payload(vb, i, plane_fmt.sizeimage +
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes
2022-06-23 19:31 ` Nicolas Dufresne
@ 2022-06-23 19:42 ` Justin Green
2022-06-23 19:48 ` Nicolas Dufresne
0 siblings, 1 reply; 6+ messages in thread
From: Justin Green @ 2022-06-23 19:42 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: linux-media, tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg,
andrescj, yunfei.dong, Justin Green
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?
>
> > if (ctx->enable_exif &&
> > q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG)
> > vb2_set_plane_payload(vb, i, plane_fmt.sizeimage +
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes
2022-06-23 19:42 ` Justin Green
@ 2022-06-23 19:48 ` Nicolas Dufresne
2022-06-23 20:27 ` Justin Green
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Dufresne @ 2022-06-23 19:48 UTC (permalink / raw)
To: Justin Green
Cc: linux-media, tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg,
andrescj, yunfei.dong, Justin Green
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 +
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes
2022-06-23 19:48 ` Nicolas Dufresne
@ 2022-06-23 20:27 ` Justin Green
0 siblings, 0 replies; 6+ messages in thread
From: Justin Green @ 2022-06-23 20:27 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: linux-media, tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg,
andrescj, yunfei.dong, Justin Green
On Thu, Jun 23, 2022 at 3:48 PM Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> 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 +
> > >
>
Sure thing!
Fixes: 45f13a57d8134 ("media: platform: Add jpeg enc feature")
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes
2022-06-23 19:14 [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes Justin Green
2022-06-23 19:31 ` Nicolas Dufresne
@ 2022-06-29 9:56 ` Hans Verkuil
1 sibling, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2022-06-29 9:56 UTC (permalink / raw)
To: Justin Green, linux-media
Cc: tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg,
nicolas.dufresne, andrescj, yunfei.dong, Justin Green
On 23/06/2022 21:14, Justin Green wrote:
> 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)
Is this correct? AFAICS this function is used for both buffers containing the
raw image (and it is correct in that case) and for buffers containing the
compressed JPEG data. But in the latter case sizeimage is the worst-case
image size, the actual data can be (and almost certainly is) less than that
and this function returns an error when it shouldn't.
Or did I miss something?
In any case: this needs to be tested on actual hardware for both encoder
and decoder. A Tested-by tag would be very welcome.
Regards,
Hans
> + return -EINVAL;
> if (ctx->enable_exif &&
> q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG)
> vb2_set_plane_payload(vb, i, plane_fmt.sizeimage +
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-29 9:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-23 19:14 [PATCH] mediatek/jpeg: validate data_offsets for v4l2 planes Justin Green
2022-06-23 19:31 ` Nicolas Dufresne
2022-06-23 19:42 ` Justin Green
2022-06-23 19:48 ` Nicolas Dufresne
2022-06-23 20:27 ` Justin Green
2022-06-29 9:56 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.