Commit 3ea3091f authored by Shuah Khan's avatar Shuah Khan Committed by Greg Kroah-Hartman

usbip: usbip_host: fix stub_dev lock context imbalance regression

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: default avatarShuah Khan <skhan@linuxfoundation.org>
Fixes: 0c9e8b3c ("usbip: usbip_host: fix BUG: sleeping function called from invalid context")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent a4768663
...@@ -326,14 +326,17 @@ static int stub_probe(struct usb_device *udev) ...@@ -326,14 +326,17 @@ static int stub_probe(struct usb_device *udev)
* See driver_probe_device() in driver/base/dd.c * See driver_probe_device() in driver/base/dd.c
*/ */
rc = -ENODEV; rc = -ENODEV;
if (!busid_priv)
goto sdev_free; goto sdev_free;
goto call_put_busid_priv;
} }
if (udev->descriptor.bDeviceClass == USB_CLASS_HUB) { if (udev->descriptor.bDeviceClass == USB_CLASS_HUB) {
dev_dbg(&udev->dev, "%s is a usb hub device... skip!\n", dev_dbg(&udev->dev, "%s is a usb hub device... skip!\n",
udev_busid); udev_busid);
rc = -ENODEV; rc = -ENODEV;
goto sdev_free; goto call_put_busid_priv;
} }
if (!strcmp(udev->bus->bus_name, "vhci_hcd")) { if (!strcmp(udev->bus->bus_name, "vhci_hcd")) {
...@@ -342,7 +345,7 @@ static int stub_probe(struct usb_device *udev) ...@@ -342,7 +345,7 @@ static int stub_probe(struct usb_device *udev)
udev_busid); udev_busid);
rc = -ENODEV; rc = -ENODEV;
goto sdev_free; goto call_put_busid_priv;
} }
...@@ -361,6 +364,9 @@ static int stub_probe(struct usb_device *udev) ...@@ -361,6 +364,9 @@ static int stub_probe(struct usb_device *udev)
save_status = busid_priv->status; save_status = busid_priv->status;
busid_priv->status = STUB_BUSID_ALLOC; busid_priv->status = STUB_BUSID_ALLOC;
/* release the busid_lock */
put_busid_priv(busid_priv);
/* /*
* Claim this hub port. * Claim this hub port.
* It doesn't matter what value we pass as owner * It doesn't matter what value we pass as owner
...@@ -373,9 +379,6 @@ static int stub_probe(struct usb_device *udev) ...@@ -373,9 +379,6 @@ static int stub_probe(struct usb_device *udev)
goto err_port; goto err_port;
} }
/* release the busid_lock */
put_busid_priv(busid_priv);
rc = stub_add_files(&udev->dev); rc = stub_add_files(&udev->dev);
if (rc) { if (rc) {
dev_err(&udev->dev, "stub_add_files for %s\n", udev_busid); dev_err(&udev->dev, "stub_add_files for %s\n", udev_busid);
...@@ -395,11 +398,17 @@ static int stub_probe(struct usb_device *udev) ...@@ -395,11 +398,17 @@ static int stub_probe(struct usb_device *udev)
spin_lock(&busid_priv->busid_lock); spin_lock(&busid_priv->busid_lock);
busid_priv->sdev = NULL; busid_priv->sdev = NULL;
busid_priv->status = save_status; busid_priv->status = save_status;
sdev_free: spin_unlock(&busid_priv->busid_lock);
stub_device_free(sdev); /* lock is released - go to free */
goto sdev_free;
call_put_busid_priv:
/* release the busid_lock */ /* release the busid_lock */
put_busid_priv(busid_priv); put_busid_priv(busid_priv);
sdev_free:
stub_device_free(sdev);
return rc; return rc;
} }
...@@ -435,7 +444,9 @@ static void stub_disconnect(struct usb_device *udev) ...@@ -435,7 +444,9 @@ static void stub_disconnect(struct usb_device *udev)
/* get stub_device */ /* get stub_device */
if (!sdev) { if (!sdev) {
dev_err(&udev->dev, "could not get device"); 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); dev_set_drvdata(&udev->dev, NULL);
...@@ -465,7 +476,7 @@ static void stub_disconnect(struct usb_device *udev) ...@@ -465,7 +476,7 @@ static void stub_disconnect(struct usb_device *udev)
if (!busid_priv->shutdown_busid) if (!busid_priv->shutdown_busid)
busid_priv->shutdown_busid = 1; busid_priv->shutdown_busid = 1;
/* release busid_lock */ /* release busid_lock */
put_busid_priv(busid_priv); spin_unlock(&busid_priv->busid_lock);
/* shutdown the current connection */ /* shutdown the current connection */
shutdown_busid(busid_priv); shutdown_busid(busid_priv);
...@@ -480,10 +491,9 @@ static void stub_disconnect(struct usb_device *udev) ...@@ -480,10 +491,9 @@ static void stub_disconnect(struct usb_device *udev)
if (busid_priv->status == STUB_BUSID_ALLOC) if (busid_priv->status == STUB_BUSID_ALLOC)
busid_priv->status = STUB_BUSID_ADDED; busid_priv->status = STUB_BUSID_ADDED;
call_put_busid_priv:
/* release busid_lock */ /* release busid_lock */
put_busid_priv(busid_priv); spin_unlock(&busid_priv->busid_lock);
return;
} }
#ifdef CONFIG_PM #ifdef CONFIG_PM
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment