usb: diag_ipc_bridge: Fix kref_put handling in case of transfers

Currently the driver puts kref in the read/write completion
callback irrespective of whether the transaction was successful
or not. This is fine for diag transfers because the read/write
function is not waiting for completion.
But in case of IPC transfers, the read/write function waits for
completion. If the transfer fails for some reason, then it will
do a kref_put as well, along with the completion callback. This
leads to double put of kref counter leading to null pointer
dereference from diag_bridge_disconnect(on cable disconnect).
Fix this by doing kref_put in the completion callback only if the
URB is submitted successfully. Else do it from the error handling
in read/write functions.

Change-Id: I50645cac757293dd0b1df9afb356281b2922401b
Signed-off-by: Ajay Agarwal <ajaya@codeaurora.org>

1 files changed, 13 insertions, 5 deletions

diff --git a/drivers/usb/misc/diag_ipc_bridge.c b/drivers/usb/misc/diag_ipc_bridge.c
index cb3aa9375ca3..a487f7c458ed 100644
--- a/drivers/usb/misc/diag_ipc_bridge.c
+++ b/drivers/usb/misc/diag_ipc_bridge.c
@@ -300,6 +300,8 @@ int diag_bridge_read(int id, char *data, int size)
 		pr_err_ratelimited("submitting urb failed err:%d", ret);
 		dev->pending_reads--;
 		usb_unanchor_urb(urb);
+		usb_autopm_put_interface(dev->ifc);
+		goto free_error;
 	}
 
 	if (id == IPC_BRIDGE) {
@@ -307,13 +309,15 @@ int diag_bridge_read(int id, char *data, int size)
 		ret = dev->read_result;
 	}
 
-	usb_autopm_put_interface(dev->ifc);
+	usb_free_urb(urb);
+	mutex_unlock(&dev->read_mutex);
+	return ret;
 
 free_error:
 	usb_free_urb(urb);
 put_error:
-	if (ret < 0) /* otherwise this is done in the completion handler */
-		kref_put(&dev->kref, diag_bridge_delete);
+	/* If URB submit successful, this is done in the completion handler */
+	kref_put(&dev->kref, diag_bridge_delete);
 error:
 	mutex_unlock(&dev->read_mutex);
 	return ret;
@@ -449,11 +453,15 @@ int diag_bridge_write(int id, char *data, int size)
 		ret = dev->write_result;
 	}
 
+	usb_free_urb(urb);
+	mutex_unlock(&dev->write_mutex);
+	return ret;
+
 free_error:
 	usb_free_urb(urb);
 put_error:
-	if (ret < 0) /* otherwise this is done in the completion handler */
-		kref_put(&dev->kref, diag_bridge_delete);
+	/* If URB submit successful, this is done in the completion handler */
+	kref_put(&dev->kref, diag_bridge_delete);
 error:
 	mutex_unlock(&dev->write_mutex);
 	return ret;