Module Name:    src
Committed By:   riastradh
Date:           Sun Mar 13 11:30:13 UTC 2022

Modified Files:
        src/sys/dev/usb: usb_subr.c usbdi.c usbdi_util.c

Log Message:
usb: Parse descriptors a little more robustly.

- Avoid reading past the end in the event of bogus bLength.
- Avoid arithmetic overflow by rearranging inequalities.

Reported-by: syzbot+511227c050a2f164e...@syzkaller.appspotmail.com


To generate a diff of this commit:
cvs rdiff -u -r1.271 -r1.272 src/sys/dev/usb/usb_subr.c
cvs rdiff -u -r1.236 -r1.237 src/sys/dev/usb/usbdi.c
cvs rdiff -u -r1.84 -r1.85 src/sys/dev/usb/usbdi_util.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/usb/usb_subr.c
diff -u src/sys/dev/usb/usb_subr.c:1.271 src/sys/dev/usb/usb_subr.c:1.272
--- src/sys/dev/usb/usb_subr.c:1.271	Sun Mar 13 11:28:42 2022
+++ src/sys/dev/usb/usb_subr.c	Sun Mar 13 11:30:12 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: usb_subr.c,v 1.271 2022/03/13 11:28:42 riastradh Exp $	*/
+/*	$NetBSD: usb_subr.c,v 1.272 2022/03/13 11:30:12 riastradh Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usb_subr.c,v 1.18 1999/11/17 22:33:47 n_hibma Exp $	*/
 
 /*
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.271 2022/03/13 11:28:42 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.272 2022/03/13 11:30:12 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -326,7 +326,7 @@ usbd_find_idesc(usb_config_descriptor_t 
 	usb_interface_descriptor_t *idesc;
 	int curidx, lastidx, curaidx = 0;
 
-	for (curidx = lastidx = -1; p < end; ) {
+	for (curidx = lastidx = -1; end - p >= sizeof(*desc);) {
 		desc = (usb_descriptor_t *)p;
 
 		DPRINTFN(4, "idx=%jd(%jd) altidx=%jd(%jd)", ifaceidx, curidx,
@@ -336,15 +336,15 @@ usbd_find_idesc(usb_config_descriptor_t 
 
 		if (desc->bLength < USB_DESCRIPTOR_SIZE)
 			break;
-		p += desc->bLength;
-		if (p > end)
+		if (desc->bLength > end - p)
 			break;
+		p += desc->bLength;
 
 		if (desc->bDescriptorType != UDESC_INTERFACE)
 			continue;
-		idesc = (usb_interface_descriptor_t *)desc;
-		if (idesc->bLength < USB_INTERFACE_DESCRIPTOR_SIZE)
+		if (desc->bLength < USB_INTERFACE_DESCRIPTOR_SIZE)
 			break;
+		idesc = (usb_interface_descriptor_t *)desc;
 
 		if (idesc->bInterfaceNumber != lastidx) {
 			lastidx = idesc->bInterfaceNumber;
@@ -378,23 +378,23 @@ usbd_find_edesc(usb_config_descriptor_t 
 		return NULL;
 
 	curidx = -1;
-	for (p = (char *)idesc + idesc->bLength; p < end; ) {
+	for (p = (char *)idesc + idesc->bLength; end - p >= sizeof(*edesc);) {
 		desc = (usb_descriptor_t *)p;
 
 		if (desc->bLength < USB_DESCRIPTOR_SIZE)
 			break;
-		p += desc->bLength;
-		if (p > end)
+		if (desc->bLength > end - p)
 			break;
+		p += desc->bLength;
 
 		if (desc->bDescriptorType == UDESC_INTERFACE)
 			break;
 		if (desc->bDescriptorType != UDESC_ENDPOINT)
 			continue;
 
-		edesc = (usb_endpoint_descriptor_t *)desc;
-		if (edesc->bLength < USB_ENDPOINT_DESCRIPTOR_SIZE)
+		if (desc->bLength < USB_ENDPOINT_DESCRIPTOR_SIZE)
 			break;
+		edesc = (usb_endpoint_descriptor_t *)desc;
 
 		curidx++;
 		if (curidx == endptidx)
@@ -553,23 +553,26 @@ usbd_fill_iface_data(struct usbd_device 
 
 	p = (char *)idesc + idesc->bLength;
 	end = (char *)dev->ud_cdesc + UGETW(dev->ud_cdesc->wTotalLength);
+	KASSERTMSG((char *)dev->ud_cdesc <= (char *)idesc, "cdesc=%p idesc=%p",
+	    dev->ud_cdesc, idesc);
+	KASSERTMSG((char *)idesc < end, "idesc=%p end=%p", idesc, end);
 #define ed ((usb_endpoint_descriptor_t *)p)
 	for (endpt = 0; endpt < nendpt; endpt++) {
 		DPRINTFN(10, "endpt=%jd", endpt, 0, 0, 0);
-		for (; p < end; p += ed->bLength) {
+		for (; end - p >= sizeof(*ed); p += ed->bLength) {
 			DPRINTFN(10, "p=%#jx end=%#jx len=%jd type=%jd",
 			    (uintptr_t)p, (uintptr_t)end, ed->bLength,
 			    ed->bDescriptorType);
-			if (p + ed->bLength <= end &&
-			    ed->bLength >= USB_ENDPOINT_DESCRIPTOR_SIZE &&
-			    ed->bDescriptorType == UDESC_ENDPOINT)
-				goto found;
-			if (ed->bLength == 0 ||
+			if (ed->bLength < sizeof(*ed) ||
+			    ed->bLength > end - p ||
 			    ed->bDescriptorType == UDESC_INTERFACE)
 				break;
+			if (ed->bLength >= USB_ENDPOINT_DESCRIPTOR_SIZE &&
+			    ed->bDescriptorType == UDESC_ENDPOINT)
+				goto found;
 		}
 		/* passed end, or bad desc */
-		if (p < end) {
+		if (end - p >= sizeof(*ed)) {
 			if (ed->bLength == 0) {
 				printf("%s: bad descriptor: 0 length\n",
 				    __func__);
@@ -607,6 +610,8 @@ usbd_fill_iface_data(struct usbd_device 
 		}
 		endpoints[endpt].ue_refcnt = 0;
 		endpoints[endpt].ue_toggle = 0;
+		KASSERTMSG(end - p >= ed->bLength, "p=%p end=%p length=%u",
+		    p, end, ed->bLength);
 		p += ed->bLength;
 	}
 #undef ed

Index: src/sys/dev/usb/usbdi.c
diff -u src/sys/dev/usb/usbdi.c:1.236 src/sys/dev/usb/usbdi.c:1.237
--- src/sys/dev/usb/usbdi.c:1.236	Sun Mar 13 11:29:01 2022
+++ src/sys/dev/usb/usbdi.c	Sun Mar 13 11:30:13 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: usbdi.c,v 1.236 2022/03/13 11:29:01 riastradh Exp $	*/
+/*	$NetBSD: usbdi.c,v 1.237 2022/03/13 11:30:13 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.236 2022/03/13 11:29:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.237 2022/03/13 11:30:13 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -976,10 +976,11 @@ usbd_get_no_alts(usb_config_descriptor_t
 	usb_interface_descriptor_t *d;
 	int n;
 
-	for (n = 0; p < end; p += d->bLength) {
+	for (n = 0; end - p >= sizeof(*d); p += d->bLength) {
 		d = (usb_interface_descriptor_t *)p;
-		if (p + d->bLength <= end &&
-		    d->bDescriptorType == UDESC_INTERFACE &&
+		if (d->bLength < sizeof(*d) || d->bLength > end - p)
+			break;
+		if (d->bDescriptorType == UDESC_INTERFACE &&
 		    d->bInterfaceNumber == ifaceno)
 			n++;
 	}

Index: src/sys/dev/usb/usbdi_util.c
diff -u src/sys/dev/usb/usbdi_util.c:1.84 src/sys/dev/usb/usbdi_util.c:1.85
--- src/sys/dev/usb/usbdi_util.c:1.84	Tue Jun 16 17:25:56 2020
+++ src/sys/dev/usb/usbdi_util.c	Sun Mar 13 11:30:13 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: usbdi_util.c,v 1.84 2020/06/16 17:25:56 maxv Exp $	*/
+/*	$NetBSD: usbdi_util.c,v 1.85 2022/03/13 11:30:13 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi_util.c,v 1.84 2020/06/16 17:25:56 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi_util.c,v 1.85 2022/03/13 11:30:13 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -599,10 +599,11 @@ usbd_get_hid_descriptor(struct usbd_inte
 	p = (char *)idesc + idesc->bLength;
 	end = (char *)cdesc + UGETW(cdesc->wTotalLength);
 
-	for (; p < end; p += hd->bLength) {
+	for (; end - p >= sizeof(*hd); p += hd->bLength) {
 		hd = (usb_hid_descriptor_t *)p;
-		if (p + hd->bLength <= end &&
-		    hd->bLength >= USB_HID_DESCRIPTOR_SIZE(0) &&
+		if (hd->bLength < sizeof(*hd) || hd->bLength > end - p)
+			break;
+		if (hd->bLength >= USB_HID_DESCRIPTOR_SIZE(0) &&
 		    hd->bDescriptorType == UDESC_HID)
 			return hd;
 		if (hd->bDescriptorType == UDESC_INTERFACE)
@@ -725,7 +726,7 @@ usb_desc_iter_peek(usbd_desc_iter_t *ite
 {
 	const usb_descriptor_t *desc;
 
-	if (iter->cur + sizeof(usb_descriptor_t) > iter->end) {
+	if (iter->end - iter->cur < sizeof(usb_descriptor_t)) {
 		if (iter->cur != iter->end)
 			printf("%s: bad descriptor\n", __func__);
 		return NULL;
@@ -735,7 +736,7 @@ usb_desc_iter_peek(usbd_desc_iter_t *ite
 		printf("%s: descriptor length too small\n", __func__);
 		return NULL;
 	}
-	if (iter->cur + desc->bLength > iter->end) {
+	if (desc->bLength > iter->end - iter->cur) {
 		printf("%s: descriptor length too large\n", __func__);
 		return NULL;
 	}
@@ -748,6 +749,7 @@ usb_desc_iter_next(usbd_desc_iter_t *ite
 	const usb_descriptor_t *desc = usb_desc_iter_peek(iter);
 	if (desc == NULL)
 		return NULL;
+	KASSERT(desc->bLength <= iter->end - iter->cur);
 	iter->cur += desc->bLength;
 	return desc;
 }

Reply via email to