commit ac0cc128f0cde52a23b2c2f76317076591727ee3
author Zach Reizner <zachr@google.com> Fri Nov 03 18:41:00 2017 -0700
committer Tim Detwiler <tjdetwiler@google.com> Tue Oct 09 16:34:09 2018 -0700
tree 86c0c22e7872a33f0abbf679aa129ecb1a09ff9a
parent 470712f4101752ea3ab956e7b80d94dc6a252550

CHROMIUM: virtwl: fix deadlock while receiving VFDs

TEST=insert 10ms delay after vq_handle_recv's first lock;
     deadlocks will happen before this change but not after
BUG=chromium:780395

Change-Id: I8e6237f7d3a9c5ef8fa105411fb8d69744f9011a
Signed-off-by: Zach Reizner <zachr@google.com>
Reviewed-on: https://chromium-review.googlesource.com/756136
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/780815
Commit-Ready: Dylan Reid <dgreid@chromium.org>
Tested-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

drivers/virtio/virtio_wl.c

diff --git a/drivers/virtio/virtio_wl.c b/drivers/virtio/virtio_wl.c
index 0ed18556..df9f190 100644
--- a/drivers/virtio/virtio_wl.c
+++ b/drivers/virtio/virtio_wl.c
@@ -487,6 +487,7 @@
 	return read_count;
 }
 
+/* must hold both vfd->lock and vi->vfds_lock */
 static size_t vfd_out_vfds_locked(struct virtwl_vfd *vfd,
 				  struct virtwl_vfd **vfds, size_t count)
 {
@@ -515,17 +516,7 @@
 
 		for (i = 0; i < vfds_to_read; i++) {
 			uint32_t vfd_id = le32_to_cpu(vfds_le[i]);
-			/*
-			 * This is an inversion of the typical locking order
-			 * (vi->vfds_lock before vfd->lock). The reason this is
-			 * safe from deadlocks is because the lock held as a
-			 * precondition of this function call is always for a
-			 * different vfd than the one received on this vfd's
-			 * queue.
-			 */
-			mutex_lock(&vi->vfds_lock);
 			vfds[read_count] = idr_find(&vi->vfds, vfd_id);
-			mutex_unlock(&vi->vfds_lock);
 			if (vfds[read_count]) {
 				read_count++;
 			} else {
@@ -586,14 +577,17 @@
 			       size_t *vfd_count)
 {
 	struct virtwl_vfd *vfd = filp->private_data;
+	struct virtwl_info *vi = vfd->vi;
 	ssize_t read_count = 0;
 	size_t vfd_read_count = 0;
 
+	mutex_lock(&vi->vfds_lock);
 	mutex_lock(&vfd->lock);
 
 	while (read_count == 0 && vfd_read_count == 0) {
 		while (list_empty(&vfd->in_queue)) {
 			mutex_unlock(&vfd->lock);
+			mutex_unlock(&vi->vfds_lock);
 			if (filp->f_flags & O_NONBLOCK)
 				return -EAGAIN;
 
@@ -601,6 +595,7 @@
 				!list_empty(&vfd->in_queue)))
 				return -ERESTARTSYS;
 
+			mutex_lock(&vi->vfds_lock);
 			mutex_lock(&vfd->lock);
 		}
 
@@ -616,6 +611,7 @@
 
 out_unlock:
 	mutex_unlock(&vfd->lock);
+	mutex_unlock(&vi->vfds_lock);
 	return read_count;
 }