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.
CRs-fixed: 393826
Change-Id: I16a4a8d928f5e18531627b03f2ce6358ec3a9d1f
Signed-off-by: Jack Pham <jackp@codeaurora.org>
diff --git a/drivers/usb/misc/diag_bridge.c b/drivers/usb/misc/diag_bridge.c
index ba7658b..cad411d 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;
@@ -120,24 +122,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);
@@ -145,7 +157,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);
@@ -170,9 +182,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);
@@ -210,24 +224,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);
@@ -235,7 +259,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);
@@ -262,9 +286,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);
@@ -381,6 +407,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;
@@ -422,7 +449,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);