Re: USB hub code can dereference NULL hub and hub->ports

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jan 20, 2025 at 12:27:19PM -0500, rtm@csail.mit.edu wrote:
> The attached program, which acts via usbip as a USB device or hub,
> causes my linux machines to dereference some NULL pointers in
> drivers/usb/core/hub.c. These are places where udev->maxchild > 0, but
> either usb_hub_to_struct_hub(udev) returns NULL, or the returned hub
> has hub->ports == NULL.
> 
> This is one such place:
> 
> static void recursively_mark_NOTATTACHED(struct usb_device *udev)
> {
>         struct usb_hub *hub = usb_hub_to_struct_hub(udev);
>         int i;
> 
>         for (i = 0; i < udev->maxchild; ++i) {
>                 if (hub->ports[i]->child)
> 
> And this:
> 
> static void hub_disconnect_children(struct usb_device *udev)
> {
>         struct usb_hub *hub = usb_hub_to_struct_hub(udev);
>         int i;
> 
>         /* Free up all the children before we remove this device */
>         for (i = 0; i < udev->maxchild; i++) {
>                 if (hub->ports[i]->child)
> 
> This can see NULL hub->ports:
> 
> void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
>                 struct usb_hub_descriptor *desc)
> {
>         struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
>         enum usb_port_connect_type connect_type;
>         int i;
> 
>         if (!hub)
>                 return;
> 
>         if (!hub_is_superspeed(hdev)) {
>                 for (i = 1; i <= hdev->maxchild; i++) {
>                         struct usb_port *port_dev = hub->ports[i - 1];
> 
> This can see a NULL hub:
> 
> static int hub_set_address(struct usb_device *udev, int devnum)
> {
>         int retval;
>         unsigned int timeout_ms = USB_CTRL_SET_TIMEOUT;
>         struct usb_hcd *hcd = bus_to_hcd(udev->bus);
>         struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
> 
>         if (hub->hdev->quirks & USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT)
> 
> I've attached a demo that runs into some of these NULL dereferences.
> It depends on being able to run usbip (and modeprobe vhci-hcd).

Great, can you submit patches to fix these issues now that you have a
reliable test program to verify the problem?

thanks,

greg k-h