From: Ana Bobillo <[email protected]> usb_hub_configure() takes the port count from the hub descriptor's bNbrPorts, which a hub may set to anything up to 255. The port bitmaps it then copies (DeviceRemovable, PortPowerCtrlMask), overcurrent_count[] and, without driver model, children[] only have room for USB_MAXCHILDREN ports. On a hub that claims more, the bitmap copies run past their arrays and the port scan indexes past the other two.
Only use the first USB_MAXCHILDREN ports of such a hub. Linux refuses the hub instead, but its xHCI driver first limits the root hub to the ports the hub code supports, while ours passes the controller's port count through. Limiting the ports keeps such a root hub working. Make the sandbox hub emulator refuse requests for ports it doesn't have, rather than index past its port state, and add a test with a hub that reports one port more than USB_MAXCHILDREN. Signed-off-by: Ana Bobillo <[email protected]> --- common/usb_hub.c | 21 ++++++++++++---- drivers/usb/emul/sandbox_hub.c | 29 +++++++++++++++++++--- test/dm/usb.c | 44 ++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/common/usb_hub.c b/common/usb_hub.c index 6a4bcec00bc..6bfe3784a97 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -658,6 +658,20 @@ static int usb_hub_configure(struct usb_device *dev) put_unaligned(le16_to_cpu(get_unaligned( &descriptor->wHubCharacteristics)), &hub->desc.wHubCharacteristics); + + /* + * The port bitmaps below and overcurrent_count[] (and children[] + * without driver model) hold USB_MAXCHILDREN ports, so don't use + * more than that. + */ + dev->maxchild = descriptor->bNbrPorts; + if (dev->maxchild > USB_MAXCHILDREN) { + printf("Hub has %d ports, only using %d\n", dev->maxchild, + USB_MAXCHILDREN); + dev->maxchild = USB_MAXCHILDREN; + } + debug("%d ports detected\n", dev->maxchild); + /* set the bitmap */ bitmap = (unsigned char *)&hub->desc.u.hs.DeviceRemovable[0]; /* devices not removable by default */ @@ -665,17 +679,14 @@ static int usb_hub_configure(struct usb_device *dev) bitmap = (unsigned char *)&hub->desc.u.hs.PortPowerCtrlMask[0]; memset(bitmap, 0xff, (USB_MAXCHILDREN+1+7)/8); /* PowerMask = 1B */ - for (i = 0; i < ((hub->desc.bNbrPorts + 1 + 7)/8); i++) + for (i = 0; i < ((dev->maxchild + 1 + 7) / 8); i++) hub->desc.u.hs.DeviceRemovable[i] = descriptor->u.hs.DeviceRemovable[i]; - for (i = 0; i < ((hub->desc.bNbrPorts + 1 + 7)/8); i++) + for (i = 0; i < ((dev->maxchild + 1 + 7) / 8); i++) hub->desc.u.hs.PortPowerCtrlMask[i] = descriptor->u.hs.PortPowerCtrlMask[i]; - dev->maxchild = descriptor->bNbrPorts; - debug("%d ports detected\n", dev->maxchild); - hubCharacteristics = get_unaligned(&hub->desc.wHubCharacteristics); switch (hubCharacteristics & HUB_CHAR_LPSM) { case 0x00: diff --git a/drivers/usb/emul/sandbox_hub.c b/drivers/usb/emul/sandbox_hub.c index 3b3e59f978f..066898f005e 100644 --- a/drivers/usb/emul/sandbox_hub.c +++ b/drivers/usb/emul/sandbox_hub.c @@ -198,6 +198,23 @@ static int clrset_post_state(struct udevice *hub, int port, int clear, int set) return ret; } +/** + * hub_port() - Get the port that a port request is for + * + * @setup: Request + * Return: port number (numbered from 0), or -EINVAL if the hub doesn't have + * the port + */ +static int hub_port(struct devrequest *setup) +{ + int port = (setup->index & 0xff) - 1; + + if (port < 0 || port >= SANDBOX_NUM_PORTS) + return -EINVAL; + + return port; +} + static int sandbox_hub_submit_control_msg(struct udevice *bus, struct usb_device *udev, unsigned long pipe, @@ -228,7 +245,9 @@ static int sandbox_hub_submit_control_msg(struct udevice *bus, struct usb_port_status *portsts = buffer; int port; - port = (setup->index & USB_HUB_PORT_MASK) - 1; + port = hub_port(setup); + if (port < 0) + return port; portsts->wPortStatus = priv->status[port]; portsts->wPortChange = priv->change[port]; udev->status = 0; @@ -247,7 +266,9 @@ static int sandbox_hub_submit_control_msg(struct udevice *bus, case USB_REQ_SET_FEATURE: { int port; - port = (setup->index & USB_HUB_PORT_MASK) - 1; + port = hub_port(setup); + if (port < 0) + return port; debug("set feature port=%x, feature=%x\n", port, setup->value); if (setup->value < USB_PORT_FEAT_C_CONNECTION) { @@ -261,7 +282,9 @@ static int sandbox_hub_submit_control_msg(struct udevice *bus, case USB_REQ_CLEAR_FEATURE: { int port; - port = (setup->index & USB_HUB_PORT_MASK) - 1; + port = hub_port(setup); + if (port < 0) + return port; debug("clear feature port=%x, feature=%x\n", port, setup->value); if (setup->value < USB_PORT_FEAT_C_CONNECTION) { diff --git a/test/dm/usb.c b/test/dm/usb.c index fa894c1096e..7f803f9fe01 100644 --- a/test/dm/usb.c +++ b/test/dm/usb.c @@ -456,3 +456,47 @@ static int dm_test_usb_keyb(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_usb_keyb, UTF_SCAN_PDATA | UTF_SCAN_FDT); + +/* test that a hub reporting more ports than we can track is limited */ +static int dm_test_usb_hub_ports(struct unit_test_state *uts) +{ + static struct usb_hub_descriptor hub_desc; + static void *desc_list[8]; + struct usb_generic_descriptor **ptr; + struct usb_dev_plat *plat; + struct udevice *emul, *hub, *dev; + struct usb_device *udev; + int i; + + /* Have the hub emulator report more ports than USB_MAXCHILDREN */ + ut_assertok(uclass_find_device_by_name(UCLASS_USB_EMUL, "hub-emul", + &emul)); + plat = dev_get_parent_plat(emul); + for (i = 0, ptr = plat->desc_list; *ptr; i++, ptr++) { + ut_assert(i < ARRAY_SIZE(desc_list) - 1); + desc_list[i] = *ptr; + if ((*ptr)->bDescriptorType == USB_DT_HUB) { + memcpy(&hub_desc, *ptr, sizeof(hub_desc)); + hub_desc.bNbrPorts = USB_MAXCHILDREN + 1; + desc_list[i] = &hub_desc; + } + } + desc_list[i] = NULL; + plat->desc_list = (struct usb_generic_descriptor **)desc_list; + + state_set_skip_delays(true); + ut_assertok(usb_init()); + ut_assertok(uclass_find_device_by_name(UCLASS_USB_HUB, "hub", &hub)); + udev = dev_get_parent_priv(hub); + ut_assertnonnull(udev); + ut_asserteq(USB_MAXCHILDREN, udev->maxchild); + + /* The devices on its first ports are still there */ + ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev)); + ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 1, &dev)); + ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 2, &dev)); + ut_assertok(usb_stop()); + + return 0; +} +DM_TEST(dm_test_usb_hub_ports, UTF_SCAN_PDATA | UTF_SCAN_FDT);
