Skip to content

Commit

Permalink
block: update add_partition() error handling
Browse files Browse the repository at this point in the history
d805dda 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>
  • Loading branch information
Tejun Heo authored and Jens Axboe committed Oct 9, 2008
1 parent ec2cded commit 88e3412
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 24 deletions.
7 changes: 2 additions & 5 deletions block/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
40 changes: 21 additions & 19 deletions fs/partitions/check.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down

0 comments on commit 88e3412

Please sign in to comment.