Module Name:    src
Committed By:   maxv
Date:           Wed Aug  7 08:47:09 UTC 2019

Modified Files:
        src/sys/dev/usb: usb.h usb_subr.c

Log Message:
Introduce USB_DESCRIPTOR_SIZE (3), and fix two bugs:

 1) In usbd_find_idesc(), make sure the tables we're reading fit in the
    allocated buffer, otherwise small overflow (seen on KASAN, with
    bLength=1).
 2) Modify usbd_find_edesc(), to fix the same issues as 1).

ok mrg@


To generate a diff of this commit:
cvs rdiff -u -r1.116 -r1.117 src/sys/dev/usb/usb.h
cvs rdiff -u -r1.236 -r1.237 src/sys/dev/usb/usb_subr.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.h
diff -u src/sys/dev/usb/usb.h:1.116 src/sys/dev/usb/usb.h:1.117
--- src/sys/dev/usb/usb.h:1.116	Tue Jul 31 16:44:30 2018
+++ src/sys/dev/usb/usb.h	Wed Aug  7 08:47:09 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: usb.h,v 1.116 2018/07/31 16:44:30 khorben Exp $	*/
+/*	$NetBSD: usb.h,v 1.117 2019/08/07 08:47:09 maxv Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -207,6 +207,7 @@ typedef struct {
 	uByte		bDescriptorType;
 	uByte		bDescriptorSubtype;
 } UPACKED usb_descriptor_t;
+#define USB_DESCRIPTOR_SIZE 3
 
 typedef struct {
 	uByte		bLength;

Index: src/sys/dev/usb/usb_subr.c
diff -u src/sys/dev/usb/usb_subr.c:1.236 src/sys/dev/usb/usb_subr.c:1.237
--- src/sys/dev/usb/usb_subr.c:1.236	Wed Jul 31 19:40:59 2019
+++ src/sys/dev/usb/usb_subr.c	Wed Aug  7 08:47:09 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: usb_subr.c,v 1.236 2019/07/31 19:40:59 maxv Exp $	*/
+/*	$NetBSD: usb_subr.c,v 1.237 2019/08/07 08:47:09 maxv 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.236 2019/07/31 19:40:59 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.237 2019/08/07 08:47:09 maxv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -359,29 +359,41 @@ usbd_find_idesc(usb_config_descriptor_t 
 	USBHIST_FUNC(); USBHIST_CALLED(usbdebug);
 	char *p = (char *)cd;
 	char *end = p + UGETW(cd->wTotalLength);
-	usb_interface_descriptor_t *d;
+	usb_descriptor_t *desc;
+	usb_interface_descriptor_t *idesc;
 	int curidx, lastidx, curaidx = 0;
 
 	for (curidx = lastidx = -1; p < end; ) {
-		d = (usb_interface_descriptor_t *)p;
+		desc = (usb_descriptor_t *)p;
+
 		DPRINTFN(4, "idx=%jd(%jd) altidx=%jd(%jd)", ifaceidx, curidx,
 		    altidx, curaidx);
-		DPRINTFN(4, "len=%jd type=%jd", d->bLength, d->bDescriptorType,
-		    0, 0);
-		if (d->bLength == 0)
-			break; /* bad descriptor */
-		p += d->bLength;
-		if (p <= end && d->bDescriptorType == UDESC_INTERFACE) {
-			if (d->bInterfaceNumber != lastidx) {
-				lastidx = d->bInterfaceNumber;
-				curidx++;
-				curaidx = 0;
-			} else
-				curaidx++;
-			if (ifaceidx == curidx && altidx == curaidx)
-				return d;
+		DPRINTFN(4, "len=%jd type=%jd", desc->bLength,
+		    desc->bDescriptorType, 0, 0);
+
+		if (desc->bLength < USB_DESCRIPTOR_SIZE)
+			break;
+		p += desc->bLength;
+		if (p > end)
+			break;
+
+		if (desc->bDescriptorType != UDESC_INTERFACE)
+			continue;
+		idesc = (usb_interface_descriptor_t *)desc;
+		if (idesc->bLength < USB_INTERFACE_DESCRIPTOR_SIZE)
+			break;
+
+		if (idesc->bInterfaceNumber != lastidx) {
+			lastidx = idesc->bInterfaceNumber;
+			curidx++;
+			curaidx = 0;
+		} else {
+			curaidx++;
 		}
+		if (ifaceidx == curidx && altidx == curaidx)
+			return idesc;
 	}
+
 	return NULL;
 }
 
@@ -391,29 +403,39 @@ usbd_find_edesc(usb_config_descriptor_t 
 {
 	char *p = (char *)cd;
 	char *end = p + UGETW(cd->wTotalLength);
-	usb_interface_descriptor_t *d;
-	usb_endpoint_descriptor_t *e;
+	usb_interface_descriptor_t *idesc;
+	usb_endpoint_descriptor_t *edesc;
+	usb_descriptor_t *desc;
 	int curidx;
 
-	d = usbd_find_idesc(cd, ifaceidx, altidx);
-	if (d == NULL)
+	idesc = usbd_find_idesc(cd, ifaceidx, altidx);
+	if (idesc == NULL)
 		return NULL;
-	if (endptidx >= d->bNumEndpoints) /* quick exit */
+	if (endptidx >= idesc->bNumEndpoints) /* quick exit */
 		return NULL;
 
 	curidx = -1;
-	for (p = (char *)d + d->bLength; p < end; ) {
-		e = (usb_endpoint_descriptor_t *)p;
-		if (e->bLength == 0)
-			break; /* bad descriptor */
-		p += e->bLength;
-		if (p <= end && e->bDescriptorType == UDESC_INTERFACE)
-			return NULL;
-		if (p <= end && e->bDescriptorType == UDESC_ENDPOINT) {
-			curidx++;
-			if (curidx == endptidx)
-				return e;
-		}
+	for (p = (char *)idesc + idesc->bLength; p < end; ) {
+		desc = (usb_descriptor_t *)p;
+
+		if (desc->bLength < USB_DESCRIPTOR_SIZE)
+			break;
+		p += desc->bLength;
+		if (p > end)
+			break;
+
+		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)
+			break;
+
+		curidx++;
+		if (curidx == endptidx)
+			return edesc;
 	}
 	return NULL;
 }

Reply via email to