Re: [Linux kernel bug] general protection fault in disable_store

[Sam Sun]

On Mon, Apr 15, 2024 at 10:47 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Actually, I've got a completely different patch which I think will fix
> the problem you encountered.  Instead of using mutual exclusion to
> avoid the race, it prevents the two routines from being called at the
> same time so the race can't occur in the first place.  It also should
> guarantee the usb_hub_to_struct_hub() doesn't return NULL when
> disable_store() calls it.
>
> Can you try the patch below, instead of (not along with) the first
> patch?  Thanks.
>
> Alan Stern
>
>
>
> Index: usb-devel/drivers/usb/core/hub.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/hub.c
> +++ usb-devel/drivers/usb/core/hub.c
> @@ -1788,16 +1788,15 @@ static void hub_disconnect(struct usb_in
>
>         mutex_lock(&usb_port_peer_mutex);
>
> +       for (port1 = hdev->maxchild; port1 > 0; --port1)
> +               usb_hub_remove_port_device(hub, port1);
> +
>         /* Avoid races with recursively_mark_NOTATTACHED() */
>         spin_lock_irq(&device_state_lock);
> -       port1 = hdev->maxchild;
>         hdev->maxchild = 0;
>         usb_set_intfdata(intf, NULL);
>         spin_unlock_irq(&device_state_lock);
>
> -       for (; port1 > 0; --port1)
> -               usb_hub_remove_port_device(hub, port1);
> -
>         mutex_unlock(&usb_port_peer_mutex);
>
>         if (hub->hdev->speed == USB_SPEED_HIGH)
>

I tried this patch and it worked. I agree this patch is better and it
avoids introducing new locks.

Best,
Yue