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 --- 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