[PATCH] 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()

[Youngjun Lee]

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 before passing it
to uvc_parse_format().

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

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index da24a655ab68..60367d9e1c05 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -729,7 +729,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
 	streaming->nformats = 0;
 
 	/* Parse the format descriptors. */
-	while (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE) {
+	while (buflen > 3 && buffer[1] == USB_DT_CS_INTERFACE) {
 		switch (buffer[2]) {
 		case UVC_VS_FORMAT_UNCOMPRESSED:
 		case UVC_VS_FORMAT_MJPEG:
-- 
2.43.0

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

[Ricardo Ribalda]

Hi Youngjun

On Tue, 10 Jun 2025 at 13:37, 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 before passing it
> to uvc_parse_format().
>

You probably want to add:
Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
Cc: stable@vger.kernel.org

> Signed-off-by: Youngjun Lee <yjjuny.lee@samsung.com>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index da24a655ab68..60367d9e1c05 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -729,7 +729,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
>         streaming->nformats = 0;
>
>         /* Parse the format descriptors. */
> -       while (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE) {
> +       while (buflen > 3 && buffer[1] == USB_DT_CS_INTERFACE) {
>                 switch (buffer[2]) {
>                 case UVC_VS_FORMAT_UNCOMPRESSED:
>                 case UVC_VS_FORMAT_MJPEG:
> --
> 2.43.0
>
>

I would have fixed it slightly different:

diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index 96eeb3aee546..1371a73e67e3 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;


I think it makes more sense to add the length check where it is going
to be used not on a caller function. If we every change
uvc_parse_format to read byte #5 we will probably miss the check on
uvc_parse_streaming()

-- 
Ricardo Ribalda

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

[yjjuny.lee]

Hi Ricardo Ribalda

> I think it makes more sense to add the length check where it is going to be used not on a caller function. If we every change 
> uvc_parse_format to read byte #5 we will probably miss the check on
> uvc_parse_streaming()
I agree. Moving the length check into the function itself is a better approach.
I'll send v2 patch.

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

[Youngjun Lee]

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()

[Ricardo Ribalda]

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()

[Laurent Pinchart]

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()

[yjjuny.lee]

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