[PATCH 4/6] usb: keep interface and endpoint counts within what was parsed

[Ana Bobillo <anabobillo3@gmail.com>]

From: Ana Bobillo <anabobillo3@gmail.com>

usb_parse_config() stores at most USB_MAXINTERFACES interfaces, with
the endpoint descriptors it finds for each, but leaves bNumInterfaces
and each interface's bNumEndpoints as the device sent them. These counts
are what usb_set_maxpacket(), usb_set_interface() and the class drivers
(usb_storage, usb_kbd, usb_ether) walk if_desc[] and ep_desc[] by. A
device whose counts claim more than it describes, or more than there is
room for, makes them read past what was stored, and past the end of
those arrays.

Lower the counts to what was stored once the configuration is parsed.
For a device with more interfaces or endpoints than there is room for,
keep the ones that fit, as the comment there already intends, rather
than return an error that nothing checks: stop at the first interface
too many, and skip each endpoint too many. Don't take the companion
descriptor of a skipped endpoint for that of the endpoint before it.

Add tests for both cases.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 common/usb.c  |  24 ++++++--
 test/dm/usb.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+), 4 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 6a4ad346f4b..05502da6b30 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -385,6 +385,7 @@ static int usb_parse_config(struct usb_device *dev,
 	int index, ifno, epno, curr_if_num;
 	u16 ep_wMaxPacketSize;
 	struct usb_interface *if_desc = NULL;
+	int i;
 
 	ifno = -1;
 	epno = -1;
@@ -428,7 +429,7 @@ static int usb_parse_config(struct usb_device *dev,
 				if (ifno >= USB_MAXINTERFACES) {
 					puts("Too many USB interfaces!\n");
 					/* try to go on with what we have */
-					return -EINVAL;
+					goto out;
 				}
 				if_desc = &dev->config.if_desc[ifno];
 				dev->config.no_of_if++;
@@ -447,6 +448,8 @@ static int usb_parse_config(struct usb_device *dev,
 			}
 			break;
 		case USB_DT_ENDPOINT:
+			/* no companion descriptor goes with an endpoint we skip */
+			epno = -1;
 			if (head->bLength != USB_DT_ENDPOINT_SIZE &&
 			    head->bLength != USB_DT_ENDPOINT_AUDIO_SIZE) {
 				printf("ERROR: Invalid USB EP length (%d)\n",
@@ -462,14 +465,14 @@ static int usb_parse_config(struct usb_device *dev,
 				puts("Endpoint descriptor out of order!\n");
 				break;
 			}
-			epno = dev->config.if_desc[ifno].no_of_ep;
 			if_desc = &dev->config.if_desc[ifno];
-			if (epno >= USB_MAXENDPOINTS) {
+			if (if_desc->no_of_ep >= USB_MAXENDPOINTS) {
 				printf("Interface %d has too many endpoints!\n",
 					if_desc->desc.bInterfaceNumber);
-				return -EINVAL;
+				break;
 			}
 			/* found an endpoint */
+			epno = if_desc->no_of_ep;
 			if_desc->no_of_ep++;
 			memcpy(&if_desc->ep_desc[epno], head,
 				USB_DT_ENDPOINT_SIZE);
@@ -525,6 +528,19 @@ static int usb_parse_config(struct usb_device *dev,
 		index += head->bLength;
 		head = (struct usb_descriptor_header *)&buffer[index];
 	}
+out:
+	/*
+	 * Interfaces and endpoints are walked by the counts in their
+	 * descriptors later on, so don't let these claim more than we stored.
+	 */
+	if (dev->config.desc.bNumInterfaces > dev->config.no_of_if)
+		dev->config.desc.bNumInterfaces = dev->config.no_of_if;
+	for (i = 0; i < dev->config.no_of_if; i++) {
+		if_desc = &dev->config.if_desc[i];
+		if (if_desc->desc.bNumEndpoints > if_desc->no_of_ep)
+			if_desc->desc.bNumEndpoints = if_desc->no_of_ep;
+	}
+
 	return 0;
 }
 
diff --git a/test/dm/usb.c b/test/dm/usb.c
index 7f803f9fe01..145ce79dfec 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -500,3 +500,157 @@ static int dm_test_usb_hub_ports(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_usb_hub_ports, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
+/*
+ * Descriptors of a device with a vendor-specific interface, which is bound
+ * to the generic driver whatever its endpoints. The tests below give them,
+ * or copies with errors in them, to the emulator on port 1 in place of its
+ * own, to check how a configuration that a device gets wrong is parsed.
+ */
+static struct usb_device_descriptor vendor_device_desc = {
+	.bLength		= sizeof(vendor_device_desc),
+	.bDescriptorType	= USB_DT_DEVICE,
+	.bcdUSB			= cpu_to_le16(0x0200),
+	.idVendor		= cpu_to_le16(0x1234),
+	.idProduct		= cpu_to_le16(0x5678),
+	.bMaxPacketSize0	= 64,
+	.bNumConfigurations	= 1,
+};
+
+static struct usb_config_descriptor vendor_config0 = {
+	.bLength		= sizeof(vendor_config0),
+	.bDescriptorType	= USB_DT_CONFIG,
+	/* wTotalLength is set up by usb_emul_setup_device() */
+	.bNumInterfaces		= 1,
+	.bmAttributes		= 1 << 7,
+	.bMaxPower		= 50,
+};
+
+static struct usb_interface_descriptor vendor_interface0 = {
+	.bLength		= sizeof(vendor_interface0),
+	.bDescriptorType	= USB_DT_INTERFACE,
+	.bNumEndpoints		= 1,
+	.bInterfaceClass	= USB_CLASS_VENDOR_SPEC,
+};
+
+static struct usb_endpoint_descriptor vendor_endpoint1_in = {
+	.bLength		= USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType	= USB_DT_ENDPOINT,
+	.bEndpointAddress	= 1 | USB_DIR_IN,
+	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize		= cpu_to_le16(512),
+};
+
+/**
+ * scan_port1() - Scan the bus with a device on port 1 that has the given
+ * descriptors
+ *
+ * @uts:	Test state
+ * @desc_list:	Descriptors of the device, terminated by NULL
+ * @udevp:	Returns the device found on port 1, or NULL if none was set up
+ * Return: 0 if OK, -ve on error
+ */
+static int scan_port1(struct unit_test_state *uts, void **desc_list,
+		      struct usb_device **udevp)
+{
+	struct udevice *emul, *hub, *dev;
+
+	ut_assertok(uclass_find_device_by_name(UCLASS_USB_EMUL,
+					       "flash-stick@0", &emul));
+	ut_assertok(usb_emul_setup_device(emul, NULL, desc_list));
+
+	state_set_skip_delays(true);
+	ut_assertok(usb_init());
+
+	*udevp = NULL;
+	ut_assertok(uclass_find_device_by_name(UCLASS_USB_HUB, "hub", &hub));
+	device_foreach_child(dev, hub) {
+		struct usb_device *udev = dev_get_parent_priv(dev);
+
+		if (device_active(dev) && udev->portnr == 1)
+			*udevp = udev;
+	}
+
+	return 0;
+}
+
+/* test that interface and endpoint counts are limited to those described */
+static int dm_test_usb_config_counts(struct unit_test_state *uts)
+{
+	static struct usb_config_descriptor config0;
+	static struct usb_interface_descriptor interface0;
+	static void *desc_list[] = {
+		&vendor_device_desc,
+		&config0,
+		&interface0,
+		&vendor_endpoint1_in,
+		NULL,
+	};
+	struct usb_device *udev;
+
+	config0 = vendor_config0;
+	config0.bNumInterfaces = USB_MAXINTERFACES + 1;
+	interface0 = vendor_interface0;
+	interface0.bNumEndpoints = USB_MAXENDPOINTS + 1;
+	ut_assertok(scan_port1(uts, desc_list, &udev));
+	ut_assertnonnull(udev);
+	ut_asserteq(1, udev->config.no_of_if);
+	ut_asserteq(1, udev->config.desc.bNumInterfaces);
+	ut_asserteq(1, udev->config.if_desc[0].no_of_ep);
+	ut_asserteq(1, udev->config.if_desc[0].desc.bNumEndpoints);
+	ut_assertok(usb_stop());
+
+	return 0;
+}
+DM_TEST(dm_test_usb_config_counts, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
+/* test that a device with more interfaces and endpoints than we keep works */
+static int dm_test_usb_config_too_many(struct unit_test_state *uts)
+{
+	static struct usb_config_descriptor config0;
+	static struct usb_interface_descriptor interface[USB_MAXINTERFACES + 1];
+	static struct usb_endpoint_descriptor endpoint[USB_MAXENDPOINTS + 1];
+	static void *desc_list[3 + ARRAY_SIZE(interface) + ARRAY_SIZE(endpoint)];
+	struct usb_device *udev;
+	int i, n = 0;
+
+	config0 = vendor_config0;
+	config0.bNumInterfaces = ARRAY_SIZE(interface);
+	for (i = 0; i < ARRAY_SIZE(interface); i++) {
+		interface[i] = vendor_interface0;
+		interface[i].bInterfaceNumber = i;
+		interface[i].bNumEndpoints = 0;
+	}
+	desc_list[n++] = &vendor_device_desc;
+	desc_list[n++] = &config0;
+
+	/* The first interface has one endpoint too many */
+	interface[0].bNumEndpoints = ARRAY_SIZE(endpoint);
+	desc_list[n++] = &interface[0];
+	for (i = 0; i < ARRAY_SIZE(endpoint); i++) {
+		endpoint[i] = vendor_endpoint1_in;
+		endpoint[i].bEndpointAddress = 1 + i % 15;
+		if (i < 15)
+			endpoint[i].bEndpointAddress |= USB_DIR_IN;
+		desc_list[n++] = &endpoint[i];
+	}
+
+	/* and there is one interface too many */
+	for (i = 1; i < ARRAY_SIZE(interface); i++)
+		desc_list[n++] = &interface[i];
+	desc_list[n] = NULL;
+
+	ut_assertok(scan_port1(uts, desc_list, &udev));
+	ut_assertnonnull(udev);
+	ut_asserteq(USB_MAXINTERFACES, udev->config.no_of_if);
+	ut_asserteq(USB_MAXINTERFACES, udev->config.desc.bNumInterfaces);
+	for (i = 0; i < USB_MAXINTERFACES; i++)
+		ut_asserteq(i, udev->config.if_desc[i].desc.bInterfaceNumber);
+	ut_asserteq(USB_MAXENDPOINTS, udev->config.if_desc[0].no_of_ep);
+	ut_asserteq(USB_MAXENDPOINTS,
+		    udev->config.if_desc[0].desc.bNumEndpoints);
+	ut_assertok(usb_stop());
+
+	return 0;
+}
+DM_TEST(dm_test_usb_config_too_many, UTF_SCAN_PDATA | UTF_SCAN_FDT);