[PATCH v3] netdevsim: fix deadlock in del_device_store() and nsim_bus_exit()

[PATCH v3] netdevsim: fix deadlock in del_device_store() and nsim_bus_exit()

[Moksh Panicker]

del_device_store() and nsim_bus_exit() both hold nsim_bus_dev_list_lock
while calling nsim_bus_dev_del(), which internally calls
device_unregister() and acquires the device lock. If another thread
already holds the device lock and tries to acquire
nsim_bus_dev_list_lock, a deadlock occurs.

Fix del_device_store() by releasing nsim_bus_dev_list_lock before
calling nsim_bus_dev_del(), after the device has already been removed
from the list with list_del().

Fix nsim_bus_exit() by using list_splice_init() to move all entries to
a local list while holding the lock, then calling nsim_bus_dev_del() on
each entry outside the lock.

Reported-by: syzbot+1cf303af03cf30b1275a@syzkaller.appspot.com
Closes: https://syzkaller.appspot.com/bug?extid=1cf303af03cf30b1275a
Signed-off-by: Moksh Panicker <mokshpanicker.7@gmail.com>
---
 drivers/net/netdevsim/bus.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 41483e371..deb937077 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -155,6 +155,8 @@ static const struct device_type nsim_bus_dev_type = {
 static struct nsim_bus_dev *
 nsim_bus_dev_new(unsigned int id, unsigned int port_count, unsigned int num_queues);
 
+static void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev);
+
 static ssize_t
 new_device_store(const struct bus_type *bus, const char *buf, size_t count)
 {
@@ -182,7 +184,6 @@ new_device_store(const struct bus_type *bus, const char *buf, size_t count)
 	}
 
 	mutex_lock(&nsim_bus_dev_list_lock);
-	/* Prevent to use resource before initialization. */
 	if (!smp_load_acquire(&nsim_bus_enable)) {
 		err = -EBUSY;
 		goto err;
@@ -195,12 +196,9 @@ new_device_store(const struct bus_type *bus, const char *buf, size_t count)
 	}
 
 	refcount_inc(&nsim_bus_devs);
-	/* Allow using nsim_bus_dev */
 	smp_store_release(&nsim_bus_dev->init, true);
-
 	list_add_tail(&nsim_bus_dev->list, &nsim_bus_dev_list);
 	mutex_unlock(&nsim_bus_dev_list_lock);
-
 	return count;
 err:
 	mutex_unlock(&nsim_bus_dev_list_lock);
@@ -208,7 +206,6 @@ new_device_store(const struct bus_type *bus, const char *buf, size_t count)
 }
 static BUS_ATTR_WO(new_device);
 
-static void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev);
 
 static ssize_t
 del_device_store(const struct bus_type *bus, const char *buf, size_t count)
@@ -241,11 +238,12 @@ del_device_store(const struct bus_type *bus, const char *buf, size_t count)
 		if (nsim_bus_dev->dev.id != id)
 			continue;
 		list_del(&nsim_bus_dev->list);
-		nsim_bus_dev_del(nsim_bus_dev);
 		err = 0;
 		break;
 	}
 	mutex_unlock(&nsim_bus_dev_list_lock);
+	if (!err)
+		nsim_bus_dev_del(nsim_bus_dev);
 	return !err ? count : err;
 }
 static BUS_ATTR_WO(del_device);
@@ -520,6 +518,7 @@ int nsim_bus_init(void)
 void nsim_bus_exit(void)
 {
 	struct nsim_bus_dev *nsim_bus_dev, *tmp;
+	LIST_HEAD(delete_list);
 
 	/* Disallow using resources */
 	smp_store_release(&nsim_bus_enable, false);
@@ -527,11 +526,10 @@ void nsim_bus_exit(void)
 		complete(&nsim_bus_devs_released);
 
 	mutex_lock(&nsim_bus_dev_list_lock);
-	list_for_each_entry_safe(nsim_bus_dev, tmp, &nsim_bus_dev_list, list) {
-		list_del(&nsim_bus_dev->list);
-		nsim_bus_dev_del(nsim_bus_dev);
-	}
+	list_splice_init(&nsim_bus_dev_list, &delete_list);
 	mutex_unlock(&nsim_bus_dev_list_lock);
+	list_for_each_entry_safe(nsim_bus_dev, tmp, &delete_list, list)
+		nsim_bus_dev_del(nsim_bus_dev);
 
 	wait_for_completion(&nsim_bus_devs_released);
 
-- 
2.34.1