[v2] uvcvideo: Also validate buffers in BULK mode
Commit Message
Just like for ISOC, validate the decoded BULK buffer size when possible.
This avoids sending corrupted or partial buffers to userspace, which may
lead to application crash or run-time failure.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
drivers/media/usb/uvc/uvc_video.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
Comments
Le mardi 05 juin 2018 à 19:46 -0400, Nicolas Dufresne a écrit :
> Just like for ISOC, validate the decoded BULK buffer size when possible.
> This avoids sending corrupted or partial buffers to userspace, which may
> lead to application crash or run-time failure.
>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Ping. That was a year and a half ago and still applies.
> ---
> drivers/media/usb/uvc/uvc_video.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index aa0082fe5833..025ffac196f3 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1234,6 +1234,7 @@ static void uvc_video_next_buffers(struct uvc_streaming *stream,
> *meta_buf = uvc_queue_next_buffer(&stream->meta.queue,
> *meta_buf);
> }
> + uvc_video_validate_buffer(stream, *video_buf);
> *video_buf = uvc_queue_next_buffer(&stream->queue, *video_buf);
> }
>
> @@ -1258,10 +1259,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
> do {
> ret = uvc_video_decode_start(stream, buf, mem,
> urb->iso_frame_desc[i].actual_length);
> - if (ret == -EAGAIN) {
> - uvc_video_validate_buffer(stream, buf);
> + if (ret == -EAGAIN)
> uvc_video_next_buffers(stream, &buf, &meta_buf);
> - }
> } while (ret == -EAGAIN);
>
> if (ret < 0)
> @@ -1277,10 +1276,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
> uvc_video_decode_end(stream, buf, mem,
> urb->iso_frame_desc[i].actual_length);
>
> - if (buf->state == UVC_BUF_STATE_READY) {
> - uvc_video_validate_buffer(stream, buf);
> + if (buf->state == UVC_BUF_STATE_READY)
> uvc_video_next_buffers(stream, &buf, &meta_buf);
> - }
> }
> }
>
Le lundi 09 décembre 2019 à 21:27 -0500, Nicolas Dufresne a écrit :
> Le mardi 05 juin 2018 à 19:46 -0400, Nicolas Dufresne a écrit :
> > Just like for ISOC, validate the decoded BULK buffer size when possible.
> > This avoids sending corrupted or partial buffers to userspace, which may
> > lead to application crash or run-time failure.
> >
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> Ping. That was a year and a half ago and still applies.
Please ignore, I was looking into the wrong branch by accident. Please,
update patchwork, that might also help to avoid confusion.
>
> > ---
> > drivers/media/usb/uvc/uvc_video.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index aa0082fe5833..025ffac196f3 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -1234,6 +1234,7 @@ static void uvc_video_next_buffers(struct uvc_streaming *stream,
> > *meta_buf = uvc_queue_next_buffer(&stream->meta.queue,
> > *meta_buf);
> > }
> > + uvc_video_validate_buffer(stream, *video_buf);
> > *video_buf = uvc_queue_next_buffer(&stream->queue, *video_buf);
> > }
> >
> > @@ -1258,10 +1259,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
> > do {
> > ret = uvc_video_decode_start(stream, buf, mem,
> > urb->iso_frame_desc[i].actual_length);
> > - if (ret == -EAGAIN) {
> > - uvc_video_validate_buffer(stream, buf);
> > + if (ret == -EAGAIN)
> > uvc_video_next_buffers(stream, &buf, &meta_buf);
> > - }
> > } while (ret == -EAGAIN);
> >
> > if (ret < 0)
> > @@ -1277,10 +1276,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
> > uvc_video_decode_end(stream, buf, mem,
> > urb->iso_frame_desc[i].actual_length);
> >
> > - if (buf->state == UVC_BUF_STATE_READY) {
> > - uvc_video_validate_buffer(stream, buf);
> > + if (buf->state == UVC_BUF_STATE_READY)
> > uvc_video_next_buffers(stream, &buf, &meta_buf);
> > - }
> > }
> > }
> >
Hi Nicolas,
On Mon, Dec 09, 2019 at 09:30:44PM -0500, Nicolas Dufresne wrote:
> Le lundi 09 décembre 2019 à 21:27 -0500, Nicolas Dufresne a écrit :
> > Le mardi 05 juin 2018 à 19:46 -0400, Nicolas Dufresne a écrit :
> > > Just like for ISOC, validate the decoded BULK buffer size when possible.
> > > This avoids sending corrupted or partial buffers to userspace, which may
> > > lead to application crash or run-time failure.
> > >
> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> >
> > Ping. That was a year and a half ago and still applies.
>
> Please ignore, I was looking into the wrong branch by accident. Please,
> update patchwork, that might also help to avoid confusion.
Unless someone has changed the patch status in the last few days without
telling me, it was already marked as accepted.
> > > ---
> > > drivers/media/usb/uvc/uvc_video.c | 9 +++------
> > > 1 file changed, 3 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > > index aa0082fe5833..025ffac196f3 100644
> > > --- a/drivers/media/usb/uvc/uvc_video.c
> > > +++ b/drivers/media/usb/uvc/uvc_video.c
> > > @@ -1234,6 +1234,7 @@ static void uvc_video_next_buffers(struct uvc_streaming *stream,
> > > *meta_buf = uvc_queue_next_buffer(&stream->meta.queue,
> > > *meta_buf);
> > > }
> > > + uvc_video_validate_buffer(stream, *video_buf);
> > > *video_buf = uvc_queue_next_buffer(&stream->queue, *video_buf);
> > > }
> > >
> > > @@ -1258,10 +1259,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
> > > do {
> > > ret = uvc_video_decode_start(stream, buf, mem,
> > > urb->iso_frame_desc[i].actual_length);
> > > - if (ret == -EAGAIN) {
> > > - uvc_video_validate_buffer(stream, buf);
> > > + if (ret == -EAGAIN)
> > > uvc_video_next_buffers(stream, &buf, &meta_buf);
> > > - }
> > > } while (ret == -EAGAIN);
> > >
> > > if (ret < 0)
> > > @@ -1277,10 +1276,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
> > > uvc_video_decode_end(stream, buf, mem,
> > > urb->iso_frame_desc[i].actual_length);
> > >
> > > - if (buf->state == UVC_BUF_STATE_READY) {
> > > - uvc_video_validate_buffer(stream, buf);
> > > + if (buf->state == UVC_BUF_STATE_READY)
> > > uvc_video_next_buffers(stream, &buf, &meta_buf);
> > > - }
> > > }
> > > }
> > >
@@ -1234,6 +1234,7 @@ static void uvc_video_next_buffers(struct uvc_streaming *stream,
*meta_buf = uvc_queue_next_buffer(&stream->meta.queue,
*meta_buf);
}
+ uvc_video_validate_buffer(stream, *video_buf);
*video_buf = uvc_queue_next_buffer(&stream->queue, *video_buf);
}
@@ -1258,10 +1259,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
do {
ret = uvc_video_decode_start(stream, buf, mem,
urb->iso_frame_desc[i].actual_length);
- if (ret == -EAGAIN) {
- uvc_video_validate_buffer(stream, buf);
+ if (ret == -EAGAIN)
uvc_video_next_buffers(stream, &buf, &meta_buf);
- }
} while (ret == -EAGAIN);
if (ret < 0)
@@ -1277,10 +1276,8 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
uvc_video_decode_end(stream, buf, mem,
urb->iso_frame_desc[i].actual_length);
- if (buf->state == UVC_BUF_STATE_READY) {
- uvc_video_validate_buffer(stream, buf);
+ if (buf->state == UVC_BUF_STATE_READY)
uvc_video_next_buffers(stream, &buf, &meta_buf);
- }
}
}