linux-kernel - Re: [PATCH 1/2] usb: xhci: Make it possible to not have a secondary HCD (3.0)

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]

Date:   Mon, 6 May 2019 16:21:53 +0300
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Nicolas Boichat <drinkcat@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Mathias Nyman <mathias.nyman@...el.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Khuong Dinh <khuong@...amperecomputing.com>,
        Hoan Tran <hoan@...amperecomputing.com>
Subject: Re: [PATCH 1/2] usb: xhci: Make it possible to not have a secondary
 HCD (3.0)

On 2.5.2019 7.56, Nicolas Boichat wrote:
> Some XHCI controllers may not have any USB 3.0 port, in this case, it
> is not useful to create add hcd->shared_hcd, which has 2 main
> downsides:
>   - A useless USB 3.0 root hub is created.
>   - A warning is thrown on boot:
> hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19)
> 
> The change is mostly about checking if hcd->shared_hcd is NULL before
> accessing it. The one special case is in xhci_run, where we need to
> call xhci_run_finished immediately, if there is no secondary hcd.

To me it looks like this creates an controller starting issue for
xHC hardware that have both usb2 and usb3 ports.

When we have usb3 ports xhci->shared_hcd is not set yet when xhci_run is called
the first time. We will end up starting the xHC before properly setting up the secondary hcd.

See further down for details

> 
> Signed-off-by: Nicolas Boichat <drinkcat@...omium.org>
> ---
> 
> This is a respin of https://lore.kernel.org/patchwork/patch/863993/,
> hopefully addressing the comments there. Note that I dropped the change
> in xhci-plat.c, as I do not have a device to test it, but made a
> similar change in xhci-mtk.c, in the next patch.
> 
> (the @apm.com addresses seem to bounce, so I added some
> @amperecomputing.com instead, if somebody there can track back the
> original issue, I'm happy to provide a patch for xhci-plat.c as well)
> 
> drivers/usb/host/xhci-hub.c |  7 ++++--
>   drivers/usb/host/xhci.c     | 45 +++++++++++++++++++++++++++----------
>   2 files changed, 38 insertions(+), 14 deletions(-)
> 

...

> @@ -698,6 +703,10 @@ int xhci_run(struct usb_hcd *hcd)
>   
>   	xhci_debugfs_init(xhci);
>   
> +	/* There is no secondary HCD, start the host controller immediately. */
> +	if (!xhci->shared_hcd)
> +		return xhci_run_finished(xhci);
> +

PCI xHC controllers with both usb2 and usb3 ports will be started before usb3 parts are properly set up.

xhci_pci_probe()
   usb_hcd_pci_probe()
     usb_add_hcd()
       hcd->driver->start(hcd)  // .start = xhci_run
         xhci_run()
           if (!xhci->shared_hcd)  // TRUE as xhci->shared_hcd is not yet set,
	    return xhci_run_finished(xhci)  // starting controller too early here
   xhci->shared_hcd = usb_create_shared_hcd()   // now xhci->shared_hcd is set.

-Mathias