md: fix error handling in md_alloc
authorChristoph Hellwig <hch@lst.de>
Tue, 19 Jul 2022 09:18:16 +0000 (11:18 +0200)
committerJens Axboe <axboe@kernel.dk>
Tue, 2 Aug 2022 23:22:43 +0000 (17:22 -0600)
Error handling in md_alloc is a mess.  Untangle it to just free the mddev
directly before add_disk is called and thus the gendisk is globally
visible.  After that clear the hold flag and let the mddev_put take care
of cleaning up the mddev through the usual mechanisms.

Fixes: 5e55e2f5fc95 ("[PATCH] md: convert compile time warnings into runtime warnings")
Fixes: 9be68dd7ac0e ("md: add error handling support for add_disk()")
Fixes: 7ad1069166c0 ("md: properly unwind when failing to add the kobject in md_alloc")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Song Liu <song@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
drivers/md/md.c

index f8b6d37c5bdf0e8d59f3baffc4fb6b6b0499429b..c1439d5ab9b1a49aecdf81e54836353161b27195 100644 (file)
@@ -790,6 +790,15 @@ out_free_new:
        return ERR_PTR(error);
 }
 
+static void mddev_free(struct mddev *mddev)
+{
+       spin_lock(&all_mddevs_lock);
+       list_del(&mddev->all_mddevs);
+       spin_unlock(&all_mddevs_lock);
+
+       kfree(mddev);
+}
+
 static const struct attribute_group md_redundancy_group;
 
 void mddev_unlock(struct mddev *mddev)
@@ -5661,8 +5670,8 @@ int md_alloc(dev_t dev, char *name)
        mutex_lock(&disks_mutex);
        mddev = mddev_alloc(dev);
        if (IS_ERR(mddev)) {
-               mutex_unlock(&disks_mutex);
-               return PTR_ERR(mddev);
+               error = PTR_ERR(mddev);
+               goto out_unlock;
        }
 
        partitioned = (MAJOR(mddev->unit) != MD_MAJOR);
@@ -5680,7 +5689,7 @@ int md_alloc(dev_t dev, char *name)
                            strcmp(mddev2->gendisk->disk_name, name) == 0) {
                                spin_unlock(&all_mddevs_lock);
                                error = -EEXIST;
-                               goto out_unlock_disks_mutex;
+                               goto out_free_mddev;
                        }
                spin_unlock(&all_mddevs_lock);
        }
@@ -5693,7 +5702,7 @@ int md_alloc(dev_t dev, char *name)
        error = -ENOMEM;
        disk = blk_alloc_disk(NUMA_NO_NODE);
        if (!disk)
-               goto out_unlock_disks_mutex;
+               goto out_free_mddev;
 
        disk->major = MAJOR(mddev->unit);
        disk->first_minor = unit << shift;
@@ -5714,26 +5723,36 @@ int md_alloc(dev_t dev, char *name)
        mddev->gendisk = disk;
        error = add_disk(disk);
        if (error)
-               goto out_cleanup_disk;
+               goto out_put_disk;
 
        kobject_init(&mddev->kobj, &md_ktype);
        error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
-       if (error)
-               goto out_del_gendisk;
+       if (error) {
+               /*
+                * The disk is already live at this point.  Clear the hold flag
+                * and let mddev_put take care of the deletion, as it isn't any
+                * different from a normal close on last release now.
+                */
+               mddev->hold_active = 0;
+               goto done;
+       }
 
        kobject_uevent(&mddev->kobj, KOBJ_ADD);
        mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
        mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
-       goto out_unlock_disks_mutex;
 
-out_del_gendisk:
-       del_gendisk(disk);
-out_cleanup_disk:
-       put_disk(disk);
-out_unlock_disks_mutex:
+done:
        mutex_unlock(&disks_mutex);
        mddev_put(mddev);
        return error;
+
+out_put_disk:
+       put_disk(disk);
+out_free_mddev:
+       mddev_free(mddev);
+out_unlock:
+       mutex_unlock(&disks_mutex);
+       return error;
 }
 
 static void md_probe(dev_t dev)