block: update add_partition() error handling

d805dda4 tried to fix error case handling in add_partition() but had a
few problems.

* disk->part[] entry is set early and left dangling if operation
  fails.

* Once device initialized, the last put_device() is responsible for
  freeing all the resources.  The failure path freed part_stats and p
  regardless of put_device() causing double free.

* holders subdir holds reference to the disk device, so failure path
  should remove it to release resources properly which was missing.

This patch fixes the above problems and while at it move partition
slot busy check into add_partition() for completeness and inlines
holders subdirectory creation.  Using separate function for it just
obfuscates the code.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Abdel Benamrouche <draconux@gmail.com>
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>

diff --git a/block/ioctl.c b/block/ioctl.c
index c722de0ef2eebf3bcfdcf05bbb6c0f97808e2cb2..eb046aeede8ae1219d2848defff777f972b157a7 100644 (file)
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -43,12 +43,9 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
                                    || pstart < 0 || plength < 0)
                                        return -EINVAL;
                        }
-                       /* partition number in use? */
+
                        mutex_lock(&bdev->bd_mutex);
-                       if (disk->part[part - 1]) {
-                               mutex_unlock(&bdev->bd_mutex);
-                               return -EBUSY;
-                       }
+
                        /* overlap? */
                        for (i = 0; i < disk->minors - 1; i++) {
                                struct hd_struct *s = disk->part[i];

diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 68f3e41ae66f02e36894fcc8442afd90d52b4808..16f98d824608f03fa2f537cd31a9d65366b01c5b 100644 (file)
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -300,15 +300,6 @@ struct device_type part_type = {
        .release        = part_release,
 };
 
-static inline void partition_sysfs_add_subdir(struct hd_struct *p)
-{
-       struct kobject *k;
-
-       k = kobject_get(&p->dev.kobj);
-       p->holder_dir = kobject_create_and_add("holders", k);
-       kobject_put(k);
-}
-
 static inline void disk_sysfs_add_subdirs(struct gendisk *disk)
 {
        struct kobject *k;
@@ -347,13 +338,16 @@ int add_partition(struct gendisk *disk, int part, sector_t start, sector_t len,
        struct hd_struct *p;
        int err;
 
+       if (disk->part[part - 1])
+               return -EBUSY;
+
        p = kzalloc(sizeof(*p), GFP_KERNEL);
        if (!p)
                return -ENOMEM;
 
        if (!init_part_stats(p)) {
                err = -ENOMEM;
-               goto out0;
+               goto out_free;
        }
        p->start_sect = start;
        p->nr_sects = len;
@@ -372,34 +366,42 @@ int add_partition(struct gendisk *disk, int part, sector_t start, sector_t len,
        p->dev.class = &block_class;
        p->dev.type = &part_type;
        p->dev.parent = &disk->dev;
-       disk->part[part-1] = p;
 
        /* delay uevent until 'holders' subdir is created */
        p->dev.uevent_suppress = 1;
        err = device_add(&p->dev);
        if (err)
-               goto out1;
-       partition_sysfs_add_subdir(p);
+               goto out_put;
+
+       err = -ENOMEM;
+       p->holder_dir = kobject_create_and_add("holders", &p->dev.kobj);
+       if (!p->holder_dir)
+               goto out_del;
+
        p->dev.uevent_suppress = 0;
        if (flags & ADDPART_FLAG_WHOLEDISK) {
                err = device_create_file(&p->dev, &dev_attr_whole_disk);
                if (err)
-                       goto out2;
+                       goto out_del;
        }
 
+       /* everything is up and running, commence */
+       disk->part[part - 1] = p;
+
        /* suppress uevent if the disk supresses it */
        if (!disk->dev.uevent_suppress)
                kobject_uevent(&p->dev.kobj, KOBJ_ADD);
 
        return 0;
 
-out2:
+out_free:
+       kfree(p);
+       return err;
+out_del:
+       kobject_put(p->holder_dir);
        device_del(&p->dev);
-out1:
+out_put:
        put_device(&p->dev);
-       free_part_stats(p);
-out0:
-       kfree(p);
        return err;
 }