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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 2 Dec 2016 14:35:22 -0500 (EST)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Guenter Roeck <linux@...ck-us.net>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH] usb: hub: Wait for connection to be reestablished after
 port reset

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@...omium.org>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>

Acked-by: Alan Stern <stern@...land.harvard.edu>

> ---
>  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 */
>