[PATCH 3.16 36/87] usbip: usbip_host: fix stub_dev lock context imbalance regression

From: Ben Hutchings
Date: Wed Oct 02 2019 - 15:12:45 EST


3.16.75-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>

commit 3ea3091f1bd8586125848c62be295910e9802af0 upstream.

Fix the following sparse context imbalance regression introduced in
a patch that fixed sleeping function called from invalid context bug.

kbuild test robot reported on:

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus

Regressions in current branch:

drivers/usb/usbip/stub_dev.c:399:9: sparse: sparse: context imbalance in 'stub_probe' - different lock contexts for basic block
drivers/usb/usbip/stub_dev.c:418:13: sparse: sparse: context imbalance in 'stub_disconnect' - different lock contexts for basic block
drivers/usb/usbip/stub_dev.c:464:1-10: second lock on line 476

Error ids grouped by kconfigs:

recent_errors
âââ i386-allmodconfig
â âââ drivers-usb-usbip-stub_dev.c:second-lock-on-line
âââ x86_64-allmodconfig
â âââ drivers-usb-usbip-stub_dev.c:sparse:sparse:context-imbalance-in-stub_disconnect-different-lock-contexts-for-basic-block
â âââ drivers-usb-usbip-stub_dev.c:sparse:sparse:context-imbalance-in-stub_probe-different-lock-contexts-for-basic-block
âââ x86_64-allyesconfig
âââ drivers-usb-usbip-stub_dev.c:second-lock-on-line

This is a real problem in an error leg where spin_lock() is called on an
already held lock.

Fix the imbalance in stub_probe() and stub_disconnect().

Signed-off-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
Fixes: 0c9e8b3cad65 ("usbip: usbip_host: fix BUG: sleeping function called from invalid context")
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
[bwh: Backported to 3.16: adjust filename]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/staging/usbip/stub_dev.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)

--- a/drivers/staging/usbip/stub_dev.c
+++ b/drivers/staging/usbip/stub_dev.c
@@ -367,14 +367,17 @@ static int stub_probe(struct usb_device
* See driver_probe_device() in driver/base/dd.c
*/
rc = -ENODEV;
- goto sdev_free;
+ if (!busid_priv)
+ goto sdev_free;
+
+ goto call_put_busid_priv;
}

if (udev->descriptor.bDeviceClass == USB_CLASS_HUB) {
dev_dbg(&udev->dev, "%s is a usb hub device... skip!\n",
udev_busid);
rc = -ENODEV;
- goto sdev_free;
+ goto call_put_busid_priv;
}

if (!strcmp(udev->bus->bus_name, "vhci_hcd")) {
@@ -383,7 +386,7 @@ static int stub_probe(struct usb_device
udev_busid);

rc = -ENODEV;
- goto sdev_free;
+ goto call_put_busid_priv;
}


@@ -402,6 +405,9 @@ static int stub_probe(struct usb_device
save_status = busid_priv->status;
busid_priv->status = STUB_BUSID_ALLOC;

+ /* release the busid_lock */
+ put_busid_priv(busid_priv);
+
/*
* Claim this hub port.
* It doesn't matter what value we pass as owner
@@ -414,9 +420,6 @@ static int stub_probe(struct usb_device
goto err_port;
}

- /* release the busid_lock */
- put_busid_priv(busid_priv);
-
rc = stub_add_files(&udev->dev);
if (rc) {
dev_err(&udev->dev, "stub_add_files for %s\n", udev_busid);
@@ -437,11 +440,17 @@ err_port:
spin_lock(&busid_priv->busid_lock);
busid_priv->sdev = NULL;
busid_priv->status = save_status;
-sdev_free:
- stub_device_free(sdev);
+ spin_unlock(&busid_priv->busid_lock);
+ /* lock is released - go to free */
+ goto sdev_free;
+
+call_put_busid_priv:
/* release the busid_lock */
put_busid_priv(busid_priv);

+sdev_free:
+ stub_device_free(sdev);
+
return rc;
}

@@ -477,7 +486,9 @@ static void stub_disconnect(struct usb_d
/* get stub_device */
if (!sdev) {
dev_err(&udev->dev, "could not get device");
- goto call_put_busid_priv;
+ /* release busid_lock */
+ put_busid_priv(busid_priv);
+ return;
}

dev_set_drvdata(&udev->dev, NULL);
@@ -507,7 +518,7 @@ static void stub_disconnect(struct usb_d
if (!busid_priv->shutdown_busid)
busid_priv->shutdown_busid = 1;
/* release busid_lock */
- put_busid_priv(busid_priv);
+ spin_unlock(&busid_priv->busid_lock);

/* shutdown the current connection */
shutdown_busid(busid_priv);
@@ -522,10 +533,9 @@ static void stub_disconnect(struct usb_d

if (busid_priv->status == STUB_BUSID_ALLOC)
busid_priv->status = STUB_BUSID_ADDED;
-
-call_put_busid_priv:
/* release busid_lock */
- put_busid_priv(busid_priv);
+ spin_unlock(&busid_priv->busid_lock);
+ return;
}

#ifdef CONFIG_PM