usbip: usbip_host: fix stub_dev lock context imbalance regression
authorShuah Khan <skhan@linuxfoundation.org>
Wed, 29 May 2019 19:46:15 +0000 (13:46 -0600)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 9 Jun 2019 07:17:11 +0000 (09:17 +0200)
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@linuxfoundation.org>
Fixes: 0c9e8b3cad65 ("usbip: usbip_host: fix BUG: sleeping function called from invalid context")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/usbip/stub_dev.c

index d094c96..7931e6c 100644 (file)
@@ -326,14 +326,17 @@ static int stub_probe(struct usb_device *udev)
                 * 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")) {
@@ -342,7 +345,7 @@ static int stub_probe(struct usb_device *udev)
                        udev_busid);
 
                rc = -ENODEV;
-               goto sdev_free;
+               goto call_put_busid_priv;
        }
 
 
@@ -361,6 +364,9 @@ static int stub_probe(struct usb_device *udev)
        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
@@ -373,9 +379,6 @@ static int stub_probe(struct usb_device *udev)
                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);
@@ -395,11 +398,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;
 }
 
@@ -435,7 +444,9 @@ static void stub_disconnect(struct usb_device *udev)
        /* 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);
@@ -465,7 +476,7 @@ static void stub_disconnect(struct usb_device *udev)
        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);
@@ -480,10 +491,9 @@ static void stub_disconnect(struct usb_device *udev)
 
        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