* [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() [not found] <CGME20250610124111epcas1p18fe9fd8ab47a424c2143d4e2912a8179@epcas1p1.samsung.com> @ 2025-06-10 12:41 ` Youngjun Lee 2025-06-10 12:58 ` Ricardo Ribalda 0 siblings, 1 reply; 8+ messages in thread From: Youngjun Lee @ 2025-06-10 12:41 UTC (permalink / raw) To: ribalda Cc: laurent.pinchart, hdegoede, mchehab, linux-media, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() 2025-06-10 12:41 ` [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() Youngjun Lee @ 2025-06-10 12:58 ` Ricardo Ribalda 2025-06-10 21:30 ` Laurent Pinchart 0 siblings, 1 reply; 8+ messages in thread From: Ricardo Ribalda @ 2025-06-10 12:58 UTC (permalink / raw) To: Youngjun Lee Cc: laurent.pinchart, hdegoede, mchehab, linux-media, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() 2025-06-10 12:58 ` Ricardo Ribalda @ 2025-06-10 21:30 ` Laurent Pinchart 2025-06-11 1:25 ` [PATCH v3] media: uvcvideo: " Youngjun Lee 2025-06-11 1:33 ` [PATCH] usb: uvc: " yjjuny.lee 0 siblings, 2 replies; 8+ messages in thread From: Laurent Pinchart @ 2025-06-10 21:30 UTC (permalink / raw) To: Ricardo Ribalda Cc: Youngjun Lee, hdegoede, mchehab, linux-media, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] media: uvcvideo: Fix 1-byte out-of-bounds read in uvc_parse_format() 2025-06-10 21:30 ` Laurent Pinchart @ 2025-06-11 1:25 ` Youngjun Lee 2025-06-11 1:33 ` [PATCH] usb: uvc: " yjjuny.lee 1 sibling, 0 replies; 8+ messages in thread From: Youngjun Lee @ 2025-06-11 1:25 UTC (permalink / raw) To: ribalda, laurent.pinchart Cc: hdegoede, mchehab, linux-media, linux-kernel, 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> 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() 2025-06-10 21:30 ` Laurent Pinchart 2025-06-11 1:25 ` [PATCH v3] media: uvcvideo: " Youngjun Lee @ 2025-06-11 1:33 ` yjjuny.lee 1 sibling, 0 replies; 8+ messages in thread From: yjjuny.lee @ 2025-06-11 1:33 UTC (permalink / raw) To: 'Laurent Pinchart', 'Ricardo Ribalda' Cc: hdegoede, mchehab, linux-media, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
[parent not found: <CGME20250610113726epcas1p449116bfeef2a102c90e9519a632eb0b9@epcas1p4.samsung.com>]
* [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() [not found] <CGME20250610113726epcas1p449116bfeef2a102c90e9519a632eb0b9@epcas1p4.samsung.com> @ 2025-06-10 11:37 ` Youngjun Lee 2025-06-10 11:56 ` Ricardo Ribalda 0 siblings, 1 reply; 8+ messages in thread From: Youngjun Lee @ 2025-06-10 11:37 UTC (permalink / raw) To: laurent.pinchart, hdegoede, mchehab Cc: linux-media, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() 2025-06-10 11:37 ` Youngjun Lee @ 2025-06-10 11:56 ` Ricardo Ribalda 2025-06-10 12:33 ` yjjuny.lee 0 siblings, 1 reply; 8+ messages in thread From: Ricardo Ribalda @ 2025-06-10 11:56 UTC (permalink / raw) To: Youngjun Lee Cc: laurent.pinchart, hdegoede, mchehab, linux-media, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() 2025-06-10 11:56 ` Ricardo Ribalda @ 2025-06-10 12:33 ` yjjuny.lee 0 siblings, 0 replies; 8+ messages in thread From: yjjuny.lee @ 2025-06-10 12:33 UTC (permalink / raw) To: 'Ricardo Ribalda' Cc: laurent.pinchart, hdegoede, mchehab, linux-media, linux-kernel, 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-06-11 1:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20250610124111epcas1p18fe9fd8ab47a424c2143d4e2912a8179@epcas1p1.samsung.com>
2025-06-10 12:41 ` [PATCH] usb: uvc: Fix 1-byte out-of-bounds read in uvc_parse_format() Youngjun Lee
2025-06-10 12:58 ` Ricardo Ribalda
2025-06-10 21:30 ` Laurent Pinchart
2025-06-11 1:25 ` [PATCH v3] media: uvcvideo: " Youngjun Lee
2025-06-11 1:33 ` [PATCH] usb: uvc: " yjjuny.lee
[not found] <CGME20250610113726epcas1p449116bfeef2a102c90e9519a632eb0b9@epcas1p4.samsung.com>
2025-06-10 11:37 ` Youngjun Lee
2025-06-10 11:56 ` Ricardo Ribalda
2025-06-10 12:33 ` yjjuny.lee
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.