linux-kernel - Re: [PATCH] usb: host: ehci: skip STS

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210826152824.GC228824@rowland.harvard.edu>
Date:   Thu, 26 Aug 2021 11:28:24 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     neal_liu <neal_liu@...eedtech.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tony Prisk <linux@...sktech.co.nz>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Tao Ren <rentao.bupt@...il.com>, BMC-SW@...eedtech.com
Subject: Re: [PATCH] usb: host: ehci: skip STS_HALT check for aspeed platform

On Thu, Aug 26, 2021 at 03:15:25PM +0800, neal_liu wrote:
> STS_HALT also depends on ASS/PSS status for apseed.
> Skip this check on startup.
> 
> Signed-off-by: neal_liu <neal_liu@...eedtech.com>
> ---
>  drivers/usb/host/ehci-hcd.c      | 10 +++++++++-
>  drivers/usb/host/ehci-platform.c |  6 ++++++
>  drivers/usb/host/ehci.h          |  1 +
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 10b0365f3439..a539e11502ef 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -634,7 +634,15 @@ static int ehci_run (struct usb_hcd *hcd)
>  	/* Wait until HC become operational */
>  	ehci_readl(ehci, &ehci->regs->command);	/* unblock posted writes */
>  	msleep(5);
> -	rc = ehci_handshake(ehci, &ehci->regs->status, STS_HALT, 0, 100 * 1000);
> +
> +	/* For Aspeed, STS_HALT also depends on ASS/PSS status.
> +	 * Skip this check on startup.
> +	 */
> +	if (ehci->is_aspeed)
> +		rc = 0;
> +	else
> +		rc = ehci_handshake(ehci, &ehci->regs->status, STS_HALT,
> +				    0, 100 * 1000);

You must not do this.  You are assuming the controller will always begin 
operating within 5 ms of the time it is told to start.  What happens if you are 
wrong?

There must be some way for the aspeed controller's host computer to tell when 
the controller has started running.  Find out what it is and use it.

Alan Stern