greybus: operation/esx: fix message-cancellation lifetime bugs

The current host-controller message-cancellation implementation suffer
from a lifetime bug as dynamically allocated URBs would complete and be
deallocated while being unlinked as part of cancellation.

The current locking is also insufficient to prevent the related race
where the URB is deallocated before being unlinked.

Fix this by pushing the cancellation implementation from greybus core
down to the host-controller drivers, and replace the "cookie" pointer
with a hcpriv field that those drivers can use to maintain their state
with the required locking and reference counting in place.

Specifically the drivers need to acquire a reference to the URB under a
lock before calling usb_kill_urb as part of cancellation.

Note that this also removes the insufficient gb_message_mutex, which
also effectively prevented us from implementing support for submissions
from atomic context.

Instead the host-controller drivers must now explicitly make sure that
the pre-allocated URBs are not reused while cancellation is in progress.

Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

1 files changed, 3 insertions, 24 deletions

diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index a713dafabf6e..b125bde32249 100644
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -23,9 +23,6 @@ static struct kmem_cache *gb_message_cache;
 /* Workqueue to handle Greybus operation completions. */
 static struct workqueue_struct *gb_operation_workqueue;
 
-/* Protects the cookie representing whether a message is in flight */
-static DEFINE_MUTEX(gb_message_mutex);
-
 /*
  * Protects access to connection operations lists, as well as
  * updates to operation->errno.
@@ -135,21 +132,11 @@ gb_operation_find(struct gb_connection *connection, u16 operation_id)
 static int gb_message_send(struct gb_message *message)
 {
 	struct gb_connection *connection = message->operation->connection;
-	int ret = 0;
-	void *cookie;
 
-	mutex_lock(&gb_message_mutex);
-	cookie = connection->hd->driver->message_send(connection->hd,
+	return connection->hd->driver->message_send(connection->hd,
 					connection->hd_cport_id,
 					message,
 					GFP_KERNEL);
-	if (IS_ERR(cookie))
-		ret = PTR_ERR(cookie);
-	else
-		message->cookie = cookie;
-	mutex_unlock(&gb_message_mutex);
-
-	return ret;
 }
 
 /*
@@ -157,14 +144,9 @@ static int gb_message_send(struct gb_message *message)
  */
 static void gb_message_cancel(struct gb_message *message)
 {
-	mutex_lock(&gb_message_mutex);
-	if (message->cookie) {
-		struct greybus_host_device *hd;
+	struct greybus_host_device *hd = message->operation->connection->hd;
 
-		hd = message->operation->connection->hd;
-		hd->driver->message_cancel(message->cookie);
-	}
-	mutex_unlock(&gb_message_mutex);
+	hd->driver->message_cancel(message);
 }
 
 static void gb_operation_request_handle(struct gb_operation *operation)
@@ -719,9 +701,6 @@ void greybus_message_sent(struct greybus_host_device *hd,
 {
 	struct gb_operation *operation;
 
-	/* Get the message and record that it is no longer in flight */
-	message->cookie = NULL;
-
 	/*
 	 * If the message was a response, we just need to drop our
 	 * reference to the operation.  If an error occurred, report