Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/usb uvideo(4): Parse descriptors more robustly.
details: https://anonhg.NetBSD.org/src/rev/d55f8eba5cc8
branches: trunk
changeset: 365287:d55f8eba5cc8
user: riastradh <riastradh%NetBSD.org@localhost>
date: Sun Apr 17 13:17:30 2022 +0000
description:
uvideo(4): Parse descriptors more robustly.
Validate lengths and types before barging ahead.
Not sure exactly which missing validation syzbot tripped on here, but
I'm pretty sure I caught all the cases.
Reported-by: syzbot+60f0a25c077b67547f57%syzkaller.appspotmail.com@localhost
diffstat:
sys/dev/usb/uvideo.c | 92 +++++++++++++++++++++++++++++++++++++++------------
1 files changed, 70 insertions(+), 22 deletions(-)
diffs (207 lines):
diff -r 91beb7bcd248 -r d55f8eba5cc8 sys/dev/usb/uvideo.c
--- a/sys/dev/usb/uvideo.c Sun Apr 17 13:17:19 2022 +0000
+++ b/sys/dev/usb/uvideo.c Sun Apr 17 13:17:30 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: uvideo.c,v 1.77 2022/04/17 13:17:19 riastradh Exp $ */
+/* $NetBSD: uvideo.c,v 1.78 2022/04/17 13:17:30 riastradh Exp $ */
/*
* Copyright (c) 2008 Patrick Mahoney
@@ -42,7 +42,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvideo.c,v 1.77 2022/04/17 13:17:19 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvideo.c,v 1.78 2022/04/17 13:17:30 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -442,8 +442,12 @@
const uvideo_vs_format_dv_descriptor_t *);
#endif /* !UVIDEO_DEBUG */
-#define GET(type, descp, field) (((const type *)(descp))->field)
-#define GETP(type, descp, field) (&(((const type *)(descp))->field))
+#define GET(type, descp, field) \
+ (KASSERT((descp)->bLength >= sizeof(type)), \
+ ((const type *)(descp))->field)
+#define GETP(type, descp, field) \
+ (KASSERT((descp)->bLength >= sizeof(type)), \
+ &(((const type *)(descp))->field))
/*
* Given a format descriptor and frame descriptor, copy values common
@@ -789,10 +793,11 @@
/* count number of units and terminals */
nunits = 0;
while ((desc = usb_desc_iter_next_non_interface(iter)) != NULL) {
+ if (desc->bDescriptorType != UDESC_CS_INTERFACE ||
+ desc->bLength < sizeof(*uvdesc))
+ continue;
uvdesc = (const uvideo_descriptor_t *)desc;
- if (uvdesc->bDescriptorType != UDESC_CS_INTERFACE)
- continue;
if (uvdesc->bDescriptorSubtype < UDESC_INPUT_TERMINAL ||
uvdesc->bDescriptorSubtype > UDESC_EXTENSION_UNIT)
continue;
@@ -815,10 +820,11 @@
/* iterate again, initializing the units */
while ((desc = usb_desc_iter_next_non_interface(iter)) != NULL) {
+ if (desc->bDescriptorType != UDESC_CS_INTERFACE ||
+ desc->bLength < sizeof(*uvdesc))
+ continue;
uvdesc = (const uvideo_descriptor_t *)desc;
- if (uvdesc->bDescriptorType != UDESC_CS_INTERFACE)
- continue;
if (uvdesc->bDescriptorSubtype < UDESC_INPUT_TERMINAL ||
uvdesc->bDescriptorSubtype > UDESC_EXTENSION_UNIT)
continue;
@@ -1116,9 +1122,6 @@
* multiple times because there may be several alternate interfaces
* associated with the same interface number.
*/
-/*
- * XXX XXX XXX: This function accesses descriptors in an unsafe manner.
- */
static usbd_status
uvideo_stream_init_desc(struct uvideo_stream *vs,
const usb_interface_descriptor_t *ifdesc,
@@ -1142,10 +1145,10 @@
* interface.
*/
while ((desc = usb_desc_iter_next_non_interface(iter)) != NULL) {
- uvdesc = (const uvideo_descriptor_t *)desc;
-
- switch (uvdesc->bDescriptorType) {
+ switch (desc->bDescriptorType) {
case UDESC_ENDPOINT:
+ if (desc->bLength < sizeof(usb_endpoint_descriptor_t))
+ goto baddesc;
bmAttributes = GET(usb_endpoint_descriptor_t,
desc, bmAttributes);
bEndpointAddress = GET(usb_endpoint_descriptor_t,
@@ -1204,6 +1207,9 @@
}
break;
case UDESC_CS_INTERFACE:
+ if (desc->bLength < sizeof(*uvdesc))
+ goto baddesc;
+ uvdesc = (const uvideo_descriptor_t *)desc;
if (ifdesc->bAlternateSetting != 0) {
DPRINTF(("uvideo_stream_init_alternate: "
"unexpected class-specific descriptor "
@@ -1244,11 +1250,12 @@
}
break;
default:
+ baddesc:
DPRINTF(("uvideo_stream_init_desc: "
- "unknown descriptor "
+ "bad descriptor "
"len=%d type=0x%02x\n",
- uvdesc->bLength,
- uvdesc->bDescriptorType));
+ desc->bLength,
+ desc->bDescriptorType));
break;
}
}
@@ -1300,8 +1307,9 @@
struct uvideo_pixel_format *pformat, *pfiter;
enum video_pixel_format pixel_format;
struct uvideo_format *format;
+ const usb_descriptor_t *desc;
const uvideo_descriptor_t *uvdesc;
- uint8_t subtype, default_index, index;
+ uint8_t subtype, subtypelen, default_index, index;
uint32_t frame_interval;
const usb_guid_t *guid;
@@ -1313,7 +1321,14 @@
switch (format_desc->bDescriptorSubtype) {
case UDESC_VS_FORMAT_UNCOMPRESSED:
DPRINTF(("%s: uncompressed\n", __func__));
+ if (format_desc->bLength <
+ sizeof(uvideo_vs_format_uncompressed_descriptor_t)) {
+ DPRINTF(("uvideo: truncated uncompressed format: %d",
+ format_desc->bLength));
+ return USBD_INVAL;
+ }
subtype = UDESC_VS_FRAME_UNCOMPRESSED;
+ subtypelen = sizeof(uvideo_vs_frame_uncompressed_descriptor_t);
default_index = GET(uvideo_vs_format_uncompressed_descriptor_t,
format_desc,
bDefaultFrameIndex);
@@ -1336,14 +1351,28 @@
break;
case UDESC_VS_FORMAT_FRAME_BASED:
DPRINTF(("%s: frame-based\n", __func__));
+ if (format_desc->bLength <
+ sizeof(uvideo_format_frame_based_descriptor_t)) {
+ DPRINTF(("uvideo: truncated frame-based format: %d",
+ format_desc->bLength));
+ return USBD_INVAL;
+ }
subtype = UDESC_VS_FRAME_FRAME_BASED;
+ subtypelen = sizeof(uvideo_frame_frame_based_descriptor_t);
default_index = GET(uvideo_format_frame_based_descriptor_t,
format_desc,
bDefaultFrameIndex);
break;
case UDESC_VS_FORMAT_MJPEG:
DPRINTF(("%s: mjpeg\n", __func__));
+ if (format_desc->bLength <
+ sizeof(uvideo_vs_format_mjpeg_descriptor_t)) {
+ DPRINTF(("uvideo: truncated mjpeg format: %d",
+ format_desc->bLength));
+ return USBD_INVAL;
+ }
subtype = UDESC_VS_FRAME_MJPEG;
+ subtypelen = sizeof(uvideo_vs_frame_mjpeg_descriptor_t);
default_index = GET(uvideo_vs_format_mjpeg_descriptor_t,
format_desc,
bDefaultFrameIndex);
@@ -1355,6 +1384,8 @@
return USBD_INVAL;
}
+ KASSERT(subtypelen >= sizeof(*uvdesc));
+
pformat = NULL;
SIMPLEQ_FOREACH(pfiter, &vs->vs_pixel_formats, entries) {
if (pfiter->pixel_format == pixel_format) {
@@ -1376,10 +1407,27 @@
* format descriptor, and add a format to the format list for
* each frame descriptor.
*/
- while ((uvdesc = (const uvideo_descriptor_t *)usb_desc_iter_peek(iter)) &&
- (uvdesc != NULL) && (uvdesc->bDescriptorSubtype == subtype))
- {
- uvdesc = (const uvideo_descriptor_t *) usb_desc_iter_next(iter);
+ while ((desc = usb_desc_iter_peek(iter)) != NULL) {
+ if (desc->bDescriptorType != UDESC_CS_INTERFACE)
+ break;
+ if (desc->bLength < sizeof(*uvdesc)) {
+ DPRINTF(("uvideo: truncated CS descriptor, length %d",
+ desc->bLength));
+ break;
+ }
+ uvdesc = (const uvideo_descriptor_t *)desc;
+ if (uvdesc->bDescriptorSubtype != subtype)
+ break;
+ if (uvdesc->bLength < subtypelen) {
+ DPRINTF(("uvideo:"
+ " truncated CS subtype-0x%x descriptor,"
+ " length %d < %d",
+ uvdesc->bLength, subtypelen));
+ break;
+ }
+
+ /* We peeked; now consume. */
+ (void)usb_desc_iter_next(iter);
format = kmem_zalloc(sizeof(*format), KM_SLEEP);
format->format.pixel_format = pixel_format;
Home |
Main Index |
Thread Index |
Old Index