[v1] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format()

[PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format()

Posted by Youngjun Lee 4 months ago

The buffer length check before calling uvc_parse_format() only ensured
that the buffer has at least 3 bytes (buflen > 2), buf the function
accesses buffer[3], requiring at least 4 bytes.

This can lead to an out-of-bounds read if the buffer has exactly 3 bytes.

Fix it by checking that the buffer has at least 4 bytes in
uvc_parse_format().

Signed-off-by: Youngjun Lee <yjjuny.lee@samsung.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index da24a655ab68..1100469a83a2 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -344,6 +344,9 @@ static int uvc_parse_format(struct uvc_device *dev,
 	u8 ftype;
 	int ret;
 
+	if (buflen < 4)
+		return -EINVAL;
+
 	format->type = buffer[2];
 	format->index = buffer[3];
 	format->frames = frames;
-- 
2.43.0

Re: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format()

Posted by Ricardo Ribalda 4 months ago

Hi Youngjun

You still miss the v2 (v3 in this case). and the trailers.

In the future you can use the b4 tool to take care of most of the details.
https://b4.docs.kernel.org/en/latest/contributor/overview.html
It has "dry-run" option that let you review the mails before you send
them to the mailing list

Please do not resubmit a new patch to fix this, only send a new patch
to fix more comments for other people.

Regards!

On Tue, 10 Jun 2025 at 14:41, Youngjun Lee <yjjuny.lee@samsung.com> wrote:
>
> The buffer length check before calling uvc_parse_format() only ensured
> that the buffer has at least 3 bytes (buflen > 2), buf the function
> accesses buffer[3], requiring at least 4 bytes.
>
> This can lead to an out-of-bounds read if the buffer has exactly 3 bytes.
>
> Fix it by checking that the buffer has at least 4 bytes in
> uvc_parse_format().
>
Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
Cc: stable@vger.kernel.org
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Youngjun Lee <yjjuny.lee@samsung.com>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index da24a655ab68..1100469a83a2 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -344,6 +344,9 @@ static int uvc_parse_format(struct uvc_device *dev,
>         u8 ftype;
>         int ret;
>
> +       if (buflen < 4)
> +               return -EINVAL;
> +
>         format->type = buffer[2];
>         format->index = buffer[3];
>         format->frames = frames;
> --
> 2.43.0
>


--
Ricardo Ribalda

Re: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format()

Posted by Laurent Pinchart 4 months ago

On Tue, Jun 10, 2025 at 02:58:25PM +0200, Ricardo Ribalda wrote:
> Hi Youngjun
> 
> You still miss the v2 (v3 in this case). and the trailers.
> 
> In the future you can use the b4 tool to take care of most of the details.
> https://b4.docs.kernel.org/en/latest/contributor/overview.html
> It has "dry-run" option that let you review the mails before you send
> them to the mailing list
> 
> Please do not resubmit a new patch to fix this, only send a new patch
> to fix more comments for other people.
> 
> Regards!
> 
> On Tue, 10 Jun 2025 at 14:41, Youngjun Lee <yjjuny.lee@samsung.com> wrote:
> >
> > The buffer length check before calling uvc_parse_format() only ensured
> > that the buffer has at least 3 bytes (buflen > 2), buf the function
> > accesses buffer[3], requiring at least 4 bytes.
> >
> > This can lead to an out-of-bounds read if the buffer has exactly 3 bytes.
> >
> > Fix it by checking that the buffer has at least 4 bytes in
> > uvc_parse_format().
>
> Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> Cc: stable@vger.kernel.org
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > Signed-off-by: Youngjun Lee <yjjuny.lee@samsung.com>
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index da24a655ab68..1100469a83a2 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -344,6 +344,9 @@ static int uvc_parse_format(struct uvc_device *dev,
> >         u8 ftype;
> >         int ret;
> >
> > +       if (buflen < 4)
> > +               return -EINVAL;
> > +
> >         format->type = buffer[2];
> >         format->index = buffer[3];
> >         format->frames = frames;

-- 
Regards,

Laurent Pinchart

RE: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format()

Posted by yjjuny.lee@samsung.com 4 months ago

The buffer length check before calling uvc_parse_format() only ensured
that the buffer has at least 3 bytes (buflen > 2), buf the function
accesses buffer[3], requiring at least 4 bytes.

This can lead to an out-of-bounds read if the buffer has exactly 3 bytes.

Fix it by checking that the buffer has at least 4 bytes in
uvc_parse_format().

Signed-off-by: Youngjun Lee <yjjuny.lee@samsung.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index da24a655ab68..1100469a83a2 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -344,6 +344,9 @@ static int uvc_parse_format(struct uvc_device *dev,
 	u8 ftype;
 	int ret;
 
+	if (buflen < 4)
+		return -EINVAL;
+
 	format->type = buffer[2];
 	format->index = buffer[3];
 	format->frames = frames;
-- 
2.43.0


> On Tue, Jun 10, 2025 at 02:58:25PM +0200, Ricardo Ribalda wrote:
> > Hi Youngjun
> > 
> > You still miss the v2 (v3 in this case). and the trailers.
> > 
> > In the future you can use the b4 tool to take care of most of the details.
> > https://b4.docs.kernel.org/en/latest/contributor/overview.html
> > It has "dry-run" option that let you review the mails before you send 
> > them to the mailing list
> > 
> > Please do not resubmit a new patch to fix this, only send a new patch 
> > to fix more comments for other people.
> > 
> > Regards!
> > 
> > On Tue, 10 Jun 2025 at 14:41, Youngjun Lee <yjjuny.lee@samsung.com> wrote:
> > >
> > > The buffer length check before calling uvc_parse_format() only 
> > > ensured that the buffer has at least 3 bytes (buflen > 2), buf the 
> > > function accesses buffer[3], requiring at least 4 bytes.
> > >
> > > This can lead to an out-of-bounds read if the buffer has exactly 3 bytes.
> > >
> > > Fix it by checking that the buffer has at least 4 bytes in 
> > > uvc_parse_format().
> >
> > Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> > Cc: stable@vger.kernel.org
> > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > > Signed-off-by: Youngjun Lee <yjjuny.lee@samsung.com>
> > > ---
> > >  drivers/media/usb/uvc/uvc_driver.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_driver.c 
> > > b/drivers/media/usb/uvc/uvc_driver.c
> > > index da24a655ab68..1100469a83a2 100644
> > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > @@ -344,6 +344,9 @@ static int uvc_parse_format(struct uvc_device *dev,
> > >         u8 ftype;
> > >         int ret;
> > >
> > > +       if (buflen < 4)
> > > +               return -EINVAL;
> > > +
> > >         format->type = buffer[2];
> > >         format->index = buffer[3];
> > >         format->frames = frames;

--
Thanks & Regards,

Youngjun Lee

[PATCH v3] media: uvcvideo: Fix 1-byte out-of-bounds read in uvc_parse_format()

Posted by Youngjun Lee 4 months ago

The buffer length check before calling uvc_parse_format() only ensured
that the buffer has at least 3 bytes (buflen > 2), buf the function
accesses buffer[3], requiring at least 4 bytes.

This can lead to an out-of-bounds read if the buffer has exactly 3 bytes.

Fix it by checking that the buffer has at least 4 bytes in
uvc_parse_format().

Signed-off-by: Youngjun Lee <yjjuny.lee@samsung.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index da24a655ab68..1100469a83a2 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -344,6 +344,9 @@ static int uvc_parse_format(struct uvc_device *dev,
 	u8 ftype;
 	int ret;
 
+	if (buflen < 4)
+		return -EINVAL;
+
 	format->type = buffer[2];
 	format->index = buffer[3];
 	format->frames = frames;
-- 
2.43.0