Re: [PATCH] usb: hub: Wait for connection to be reestablished after port reset

[Alan Stern]

On Thu, 1 Dec 2016, Guenter Roeck wrote:

> On a system with a defective USB device connected to an USB hub,
> an endless sequence of port connect events was observed. The sequence
> of events as observed is as follows:
> 
> - Port reports connected event (port status=USB_PORT_STAT_CONNECTION).
> - Event handler debounces port and resets it by calling hub_port_reset().
> - hub_port_reset() calls hub_port_wait_reset() to wait for the reset
>   to complete.
> - The reset completes, but USB_PORT_STAT_CONNECTION is not immediately
>   set in the port status register.
> - hub_port_wait_reset() returns -ENOTCONN.
> - Port initialization sequence is aborted.
> - A few milliseconds later, the port again reports a connected event,
>   and the sequence repeats.
> 
> This continues either forever or, randomly, stops if the connection
> is already re-established when the port status is read. It results in
> a high rate of udev events. This in turn destabilizes userspace since
> the above sequence holds the device mutex pretty much continuously
> and prevents userspace from actually reading the device status.
> 
> To prevent the problem from happening, let's wait for the connection
> to be re-established after a port reset. If the device was actually
> disconnected, the code will still return an error, but it will do so
> only after the long reset timeout.
> 
> Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>

Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

> ---
>  drivers/usb/core/hub.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index cbb146736f57..9bcb649e0e8c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2731,8 +2731,15 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
>  		if (ret < 0)
>  			return ret;
>  
> -		/* The port state is unknown until the reset completes. */
> -		if (!(portstatus & USB_PORT_STAT_RESET))
> +		/*
> +		 * The port state is unknown until the reset completes.
> +		 *
> +		 * On top of that, some chips may require additional time
> +		 * to re-establish a connection after the reset is complete,
> +		 * so also wait for the connection to be re-established.
> +		 */
> +		if (!(portstatus & USB_PORT_STAT_RESET) &&
> +		    (portstatus & USB_PORT_STAT_CONNECTION))
>  			break;
>  
>  		/* switch to the long delay after two short delay failures */
>