[PATCH v1] misc: vmw_vmci: Leak queue pair endpoint if detach fails on exit

[PATCH v1] misc: vmw_vmci: Leak queue pair endpoint if detach fails on exit

[Yuho Choi]

When the VMCI guest device is removed, vmci_qp_guest_endpoints_exit()
asks the hypervisor to detach each remaining queue pair and then frees
its queue pages with qp_guest_endpoint_destroy(), even if the detach
hypercall failed.

If the detach fails, the hypervisor may still have the queue pair, and
the peer may keep writing to the queue pages after they have gone back
to the page allocator. The device reset that follows in
vmci_guest_remove_device() comes only after the pages have been freed.

qp_detatch_guest_work() already handles the same failure by keeping the
entry, because the peer "might still be accessing the shared memory",
and leaves the cleanup to this exit path, which then frees it anyway.

Leak the endpoint instead if the detach fails. This also keeps its
handle in the resource table, so the handle is not given to a new queue
pair while the hypervisor may still have the old one.

Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
Compile-tested only (x86_64 with VMWARE_VMCI, W=1); I do not have 
a VMware setup to test it on.

 drivers/misc/vmw_vmci/vmci_queue_pair.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index e1a11820be01..57a4f0ae4f49 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -2391,28 +2391,35 @@ int vmci_qp_broker_unmap(struct vmci_handle handle,
 /*
  * Destroys all guest queue pair endpoints. If active guest queue
  * pairs still exist, hypercalls to attempt detach from these
- * queue pairs will be made. Any failure to detach is silently
- * ignored.
+ * queue pairs will be made. If a detach fails, the endpoint is
+ * leaked, as the peer may still access its queues.
  */
 void vmci_qp_guest_endpoints_exit(void)
 {
 	struct qp_entry *entry;
 	struct qp_guest_endpoint *ep;
+	int result;
 
 	mutex_lock(&qp_guest_endpoints.mutex);
 
 	while ((entry = qp_list_get_head(&qp_guest_endpoints))) {
 		ep = (struct qp_guest_endpoint *)entry;
+		result = VMCI_SUCCESS;
 
 		/* Don't make a hypercall for local queue_pairs. */
 		if (!(entry->flags & VMCI_QPFLAG_LOCAL))
-			qp_detatch_hypercall(entry->handle);
+			result = qp_detatch_hypercall(entry->handle);
 
 		/* We cannot fail the exit, so let's reset ref_count. */
 		entry->ref_count = 0;
 		qp_list_remove_entry(&qp_guest_endpoints, entry);
 
-		qp_guest_endpoint_destroy(ep);
+		/*
+		 * If the detach failed, the peer may still access the
+		 * queues, so leak the endpoint.
+		 */
+		if (result >= VMCI_SUCCESS)
+			qp_guest_endpoint_destroy(ep);
 	}
 
 	mutex_unlock(&qp_guest_endpoints.mutex);

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0