Re: [RFT PATCH] xhci: Fix null pointer dereference in resume if xhci has only one roothub

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



*On 9.6.2022 18.41, Matthias Kaehlcke wrote:
On Thu, Jun 09, 2022 at 03:03:36PM +0300, Mathias Nyman wrote:
In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
however this field isn't initialized by __usb_create_hcd() for a HCD
without secondary controller.

xhci_resume() is called once per xHC device, not per hcd, so the extra
checking for primary hcd can be removed.

Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
Reported-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
---
  drivers/usb/host/xhci.c | 15 +++++----------
  1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f0ab63138016..9ac56e9ffc64 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1107,7 +1107,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
  {
  	u32			command, temp = 0;
  	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
-	struct usb_hcd		*secondary_hcd;
  	int			retval = 0;
  	bool			comp_timer_running = false;
  	bool			pending_portevent = false;
@@ -1214,23 +1213,19 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
  		 * first with the primary HCD, and then with the secondary HCD.
  		 * If we don't do the same, the host will never be started.
  		 */
-		if (!usb_hcd_is_primary_hcd(hcd))
-			secondary_hcd = hcd;
-		else
-			secondary_hcd = xhci->shared_hcd;
-
  		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
-		retval = xhci_init(hcd->primary_hcd);
+		retval = xhci_init(hcd);
  		if (retval)
  			return retval;
  		comp_timer_running = true;
xhci_dbg(xhci, "Start the primary HCD\n");

Is the log still correct? IIUC this now isn't necessarily the primary HCD.

It's still correct as this is always the xhci->main_hcd, the one that is created first.
There could be a better word than "primary", but my brain is accustomed to seeing this
line while debugging.



-		retval = xhci_run(hcd->primary_hcd);
-		if (!retval && secondary_hcd) {
+		retval = xhci_run(hcd);
+		if (!retval && xhci->shared_hcd) {
  			xhci_dbg(xhci, "Start the secondary HCD\n");

ditto

same, always xhci->shared_hcd, the one that is created second.


-			retval = xhci_run(secondary_hcd);
+			retval = xhci_run(xhci->shared_hcd);
  		}
+
  		hcd->state = HC_STATE_SUSPENDED;
  		if (xhci->shared_hcd)
  			xhci->shared_hcd->state = HC_STATE_SUSPENDED;

Tested-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>

Thanks for testing
-Mathias



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux