usb: misc: diag_ipc_bridge: Move dev cleanup to delete function

Currently the driver does the diag_briddge_dev's members' cleanup
in disconnect function. This can lead to race between read/write
and disconnect functions where the read/write mutex is being
destroyed when it is in locked state. Also, the read/write
function can be called after disconnect leading to mutex_lock
warning on a destroyed mutex. Finally, since the close function
can be called after disconnect, it can lead to null pointer
dereference from dev->ifc since it is being assigned null in
disconnect. Also, there can be a use-after-free if the interface
structure is used after disconnect function has been called and
core has freed the intf.
Fix this by moving the dev member cleanup from disconnect to the
delete function. This will ensure that mutex and dev->ifc exists
when the diag core can still queue read/write and call close.
Also do a get and put of interface from probe and delete
respectively to prevent the use-after-free issue.

Change-Id: I1a1fa4440560b0c0b77880fb3f5a37c3c24c7e67
Signed-off-by: Ajay Agarwal <ajaya@codeaurora.org>
This commit is contained in:
Ajay Agarwal 2019-01-08 11:10:18 +05:30
parent a3073dde63
commit 64287585dc

View file

@ -131,9 +131,12 @@ static int ipc_bridge_open(struct platform_device *pdev)
static void diag_bridge_delete(struct kref *kref)
{
struct diag_bridge *dev = container_of(kref, struct diag_bridge, kref);
struct usb_interface *ifc = dev->ifc;
int id = dev->id;
dev_dbg(&dev->ifc->dev, "%s\n", __func__);
usb_set_intfdata(ifc, NULL);
usb_put_intf(ifc);
usb_put_dev(dev->udev);
__dev[id] = 0;
kfree(dev);
@ -308,6 +311,8 @@ int diag_bridge_read(int id, char *data, int size)
goto free_error;
}
usb_autopm_put_interface(dev->ifc);
if (id == IPC_BRIDGE) {
wait_for_completion(&dev->read_done);
ret = dev->read_result;
@ -615,7 +620,7 @@ diag_bridge_probe(struct usb_interface *ifc, const struct usb_device_id *id)
dev->id = devid;
dev->udev = usb_get_dev(interface_to_usbdev(ifc));
dev->ifc = ifc;
dev->ifc = usb_get_intf(ifc);
kref_init(&dev->kref);
mutex_init(&dev->ifc_mutex);
mutex_init(&dev->read_mutex);
@ -705,13 +710,7 @@ static void diag_bridge_disconnect(struct usb_interface *ifc)
platform_device_unregister(dev->pdev);
diag_bridge_debugfs_cleanup();
mutex_lock(&dev->ifc_mutex);
dev->ifc = NULL;
mutex_unlock(&dev->ifc_mutex);
usb_set_intfdata(ifc, NULL);
mutex_destroy(&dev->write_mutex);
mutex_destroy(&dev->read_mutex);
mutex_destroy(&dev->ifc_mutex);
dev->err = -ENODEV;
kref_put(&dev->kref, diag_bridge_delete);
}