usb: diag_bridge: Fix NULL pointer crash during disconnect
DIAG I/O could be executing concurrently when the device/interface
is disconnected. In such cases the usb_interface pointer could
become NULL while the read or write functions are about to access
it. Prevent these NULL pointer dereferences by guarding the
pointer with a mutex.
Change-Id: I0e2660dc53dae811e0a8686e69636808281ec53c
CRs-fixed: 393826
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Neha Pandey <nehap@codeaurora.org>
diff --git a/drivers/usb/misc/diag_bridge.c b/drivers/usb/misc/diag_bridge.c
index 09b3203..38e7786 100644
--- a/drivers/usb/misc/diag_bridge.c
+++ b/drivers/usb/misc/diag_bridge.c
@@ -20,6 +20,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/kref.h>
+#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/ratelimit.h>
#include <linux/uaccess.h>
@@ -38,6 +39,7 @@
__u8 out_epAddr;
int err;
struct kref kref;
+ struct mutex ifc_mutex;
struct diag_bridge_ops *ops;
struct platform_device *pdev;
@@ -124,24 +126,34 @@
pr_debug("reading %d bytes", size);
- if (!dev || !dev->ifc) {
+ if (!dev) {
pr_err("device is disconnected");
return -ENODEV;
}
+ mutex_lock(&dev->ifc_mutex);
+ if (!dev->ifc) {
+ ret = -ENODEV;
+ goto error;
+ }
+
if (!dev->ops) {
pr_err("bridge is not open");
- return -ENODEV;
+ ret = -ENODEV;
+ goto error;
}
if (!size) {
dev_err(&dev->ifc->dev, "invalid size:%d\n", size);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
/* if there was a previous unrecoverable error, just quit */
- if (dev->err)
- return -ENODEV;
+ if (dev->err) {
+ ret = -ENODEV;
+ goto error;
+ }
kref_get(&dev->kref);
@@ -149,7 +161,7 @@
if (!urb) {
dev_err(&dev->ifc->dev, "unable to allocate urb\n");
ret = -ENOMEM;
- goto error;
+ goto put_error;
}
ret = usb_autopm_get_interface(dev->ifc);
@@ -174,9 +186,11 @@
free_error:
usb_free_urb(urb);
-error:
+put_error:
if (ret) /* otherwise this is done in the completion handler */
kref_put(&dev->kref, diag_bridge_delete);
+error:
+ mutex_unlock(&dev->ifc_mutex);
return ret;
}
EXPORT_SYMBOL(diag_bridge_read);
@@ -218,24 +232,34 @@
pr_debug("writing %d bytes", size);
- if (!dev || !dev->ifc) {
+ if (!dev) {
pr_err("device is disconnected");
return -ENODEV;
}
+ mutex_lock(&dev->ifc_mutex);
+ if (!dev->ifc) {
+ ret = -ENODEV;
+ goto error;
+ }
+
if (!dev->ops) {
pr_err("bridge is not open");
- return -ENODEV;
+ ret = -ENODEV;
+ goto error;
}
if (!size) {
dev_err(&dev->ifc->dev, "invalid size:%d\n", size);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
/* if there was a previous unrecoverable error, just quit */
- if (dev->err)
- return -ENODEV;
+ if (dev->err) {
+ ret = -ENODEV;
+ goto error;
+ }
kref_get(&dev->kref);
@@ -243,7 +267,7 @@
if (!urb) {
dev_err(&dev->ifc->dev, "unable to allocate urb\n");
ret = -ENOMEM;
- goto error;
+ goto put_error;
}
ret = usb_autopm_get_interface(dev->ifc);
@@ -270,9 +294,11 @@
free_error:
usb_free_urb(urb);
-error:
+put_error:
if (ret) /* otherwise this is done in the completion handler */
kref_put(&dev->kref, diag_bridge_delete);
+error:
+ mutex_unlock(&dev->ifc_mutex);
return ret;
}
EXPORT_SYMBOL(diag_bridge_write);
@@ -389,6 +415,7 @@
dev->udev = usb_get_dev(interface_to_usbdev(ifc));
dev->ifc = ifc;
kref_init(&dev->kref);
+ mutex_init(&dev->ifc_mutex);
init_usb_anchor(&dev->submitted);
ifc_desc = ifc->cur_altsetting;
@@ -430,7 +457,9 @@
dev_dbg(&dev->ifc->dev, "%s:\n", __func__);
platform_device_unregister(dev->pdev);
+ mutex_lock(&dev->ifc_mutex);
dev->ifc = NULL;
+ mutex_unlock(&dev->ifc_mutex);
diag_bridge_debugfs_cleanup();
kref_put(&dev->kref, diag_bridge_delete);
usb_set_intfdata(ifc, NULL);