linux-kernel - Re: [PATCH v2 1/2] USB: reset-resume the device when PORT

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20210510150203.GD863718@rowland.harvard.edu>
Date:   Mon, 10 May 2021 11:02:03 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     chris.chiu@...onical.com
Cc:     gregkh@...uxfoundation.org, m.v.b@...box.com, hadess@...ess.net,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] USB: reset-resume the device when PORT_SUSPEND is
 set but timeout

On Mon, May 10, 2021 at 10:50:29PM +0800, chris.chiu@...onical.com wrote:
> From: Chris Chiu <chris.chiu@...onical.com>
> 
> On the Realtek high-speed Hub(0bda:5487), the port which has wakeup
> enabled_descendants will sometimes timeout when setting PORT_SUSPEND
> feature. After checking the PORT_SUSPEND bit in wPortStatus, it is
> already set. However, the hub will fail to activate because the
> PORT_SUSPEND feature of that port is not cleared during resume. All
> connected devices are lost after resume.
> 
> This commit force reset-resume the device connected to the timeout
> but suspended port so that the hub will have chance to clear the
> PORT_SUSPEND feature during resume.

Are you certain that the reset-resume is needed?  What happens if you 
leave out the line that sets udev->reset_resume?  The rest of the patch 
will cause the kernel to realize that the port really is suspended, so 
maybe the suspend feature will get cleared properly during resume.

It's worthwhile to try the experiement and see what happens.

Alan Stern

> Signed-off-by: Chris Chiu <chris.chiu@...onical.com>
> ---
> 
> Changelog:
>   v2:
>     - create a new variable to keep the result of hub_port_status
>       when suspend timeout.
> 
>  drivers/usb/core/hub.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index b2bc4b7c4289..3c823544e425 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -3385,6 +3385,21 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
>  		status = 0;
>  	}
>  	if (status) {
> +		if (status == -ETIMEDOUT) {
> +			u16 portstatus, portchange;
> +
> +			int ret = hub_port_status(hub, port1, &portstatus,
> +					&portchange);
> +
> +			dev_dbg(&port_dev->dev,
> +				"suspend timeout, status %04x\n", portstatus);
> +
> +			if (ret == 0 && port_is_suspended(hub, portstatus)) {
> +				udev->reset_resume = 1;
> +				goto err_wakeup;
> +			}
> +		}
> +
>  		dev_dbg(&port_dev->dev, "can't suspend, status %d\n", status);
>  
>  		/* Try to enable USB3 LTM again */
> -- 
> 2.20.1
>