gunyah: gh_msgq: Disallow multiple registrations with same label

Multiple clients racing with each other to register with the same label
could possibly succeed in doing so, contrary to design, which mandates
that only one client should be able to register with a given label, and
others should receive an -EBUSY. This is due to two reasons:

  1. Checking for a label's cap_table_entry in the global
     gh_msgq_cap_list and then allocating one if none is found is
     not an atomic operation all under one spinlock.

  2. The cap_entry_lock spinlock protecting the cap_table_entry is
     relinquished prematurely, before the client_desc can be set in
     cap_table_entry.

Two clients attempting to register by passing in the same label could
potentially each find no corresponding cap_table_entry and then each
proceed to allocate a new entry (adding it to the global
gh_msgq_cap_list). Continuing with this scenario, both freshly-allocated
cap_table_entry's will have their client_desc set to NULL and so will
have a client_desc allocated and return successfully.

Fix this by:

1. Bringing the cap_table_entry existence check and allocation steps
   under the same spinlock, thereby preventing further allocations if
   the cap_table_entry already exists.

2. Removing the spinlock from within gh_mgsq_alloc_entry() because it is
   now being called with the same spinlock held.

3. Extending cap_entry_lock's critical section to cover the allocation
   of client_desc as well. This will prevent the overwriting of
   client_desc in the case of a race condition where two clients obtain
   the same cap_table_entry and both of them find their client_desc's to
   be NULL and then each proceed to allocate one and assign it to the
   same cap_table_entry one after the other.

4. Changing the allocation flags to GFP_ATOMIC to avoid sleeping within
   a critical section.

Change-Id: I99072d466e91151302a50e5f35f2b2a8d5ee5c48
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>

drivers/virt/gunyah/gh_msgq.c

@@ -56,7 +56,7 @@ struct gh_msgq_cap_table *gh_msgq_alloc_entry(int label)
 	int ret;
 	struct gh_msgq_cap_table *cap_table_entry = NULL;
 
-	cap_table_entry = kzalloc(sizeof(struct gh_msgq_cap_table), GFP_KERNEL);
+	cap_table_entry = kzalloc(sizeof(struct gh_msgq_cap_table), GFP_ATOMIC);
 	if (!cap_table_entry)
 		return ERR_PTR(-ENOMEM);
 
@@ -85,9 +85,7 @@ struct gh_msgq_cap_table *gh_msgq_alloc_entry(int label)
 		goto err;
 	}
 
-	spin_lock(&gh_msgq_cap_list_lock);
 	list_add(&cap_table_entry->entry, &gh_msgq_cap_list);
-	spin_unlock(&gh_msgq_cap_list_lock);
 	return cap_table_entry;
 err:
 	kfree(cap_table_entry->tx_irq_name);
@@ -406,32 +404,35 @@ void *gh_msgq_register(int label)
 			break;
 		}
 	}
-	spin_unlock(&gh_msgq_cap_list_lock);
 
 	if (cap_table_entry == NULL) {
 		cap_table_entry = gh_msgq_alloc_entry(label);
-		if (IS_ERR(cap_table_entry))
+		if (IS_ERR(cap_table_entry)) {
+			spin_unlock(&gh_msgq_cap_list_lock);
 			return cap_table_entry;
+		}
 	}
+	spin_unlock(&gh_msgq_cap_list_lock);
 
 	spin_lock(&cap_table_entry->cap_entry_lock);
 
 	/* Multiple clients cannot register to the same label (msgq) */
 	if (cap_table_entry->client_desc) {
 		spin_unlock(&cap_table_entry->cap_entry_lock);
+		pr_err("%s: Client already exists for label %d\n",
+				__func__, label);
 		return ERR_PTR(-EBUSY);
 	}
 
-	spin_unlock(&cap_table_entry->cap_entry_lock);
-
-	client_desc = kzalloc(sizeof(*client_desc), GFP_KERNEL);
-	if (!client_desc)
+	client_desc = kzalloc(sizeof(*client_desc), GFP_ATOMIC);
+	if (!client_desc) {
+		spin_unlock(&cap_table_entry->cap_entry_lock);
 		return ERR_PTR(ENOMEM);
+	}
 
 	client_desc->label = label;
 	client_desc->cap_table = cap_table_entry;
 
-	spin_lock(&cap_table_entry->cap_entry_lock);
 	cap_table_entry->client_desc = client_desc;
 	spin_unlock(&cap_table_entry->cap_entry_lock);
 
@@ -503,13 +504,15 @@ int gh_msgq_populate_cap_info(int label, u64 cap_id, int direction, int irq)
 			break;
 		}
 	}
-	spin_unlock(&gh_msgq_cap_list_lock);
 
 	if (cap_table_entry == NULL) {
 		cap_table_entry = gh_msgq_alloc_entry(label);
-		if (IS_ERR(cap_table_entry))
+		if (IS_ERR(cap_table_entry)) {
+			spin_unlock(&gh_msgq_cap_list_lock);
 			return PTR_ERR(cap_table_entry);
+		}
 	}
+	spin_unlock(&gh_msgq_cap_list_lock);
 
 	if (direction == GH_MSGQ_DIRECTION_TX) {
 		ret = request_irq(irq, gh_msgq_tx_isr, 0,