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

From: Sam Sun
Date: Tue Apr 16 2024 - 05:06:24 EST


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