Commit 493cfaea authored by Logan Gunthorpe's avatar Logan Gunthorpe Committed by Greg Kroah-Hartman

mtd: utilize new cdev_device_add helper function

This is not as straightforward a conversion as the others
in this series. These drivers did not originally make use of
kobj.parent so they likely suffered from a use after free bug if
someone unregistered the devices while they are being used.

In order to make the conversions, switch from device_register
to device_initialize / cdev_device_add.

In build.c, this patch unwinds a complicated mess of extra
get_device/put_devices and reference tracking by moving device_initialize
early in the attach process. Then it always uses put_device and instead of
using device_unregister and extra get_devices everywhere we just use
cdev_device_del and one put_device once everything is completely done.
This simplifies things dramatically and makes it easier to reason about.

In vmt.c, the patch pushes device initialization up to the beginning of the
device creation and then that function only needs to use put_device
in the error path which simplifies things a good deal.
Signed-off-by: default avatarLogan Gunthorpe <logang@deltatee.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 857313e5
...@@ -420,41 +420,6 @@ static void dev_release(struct device *dev) ...@@ -420,41 +420,6 @@ static void dev_release(struct device *dev)
kfree(ubi); kfree(ubi);
} }
/**
* ubi_sysfs_init - initialize sysfs for an UBI device.
* @ubi: UBI device description object
* @ref: set to %1 on exit in case of failure if a reference to @ubi->dev was
* taken
*
* This function returns zero in case of success and a negative error code in
* case of failure.
*/
static int ubi_sysfs_init(struct ubi_device *ubi, int *ref)
{
int err;
ubi->dev.release = dev_release;
ubi->dev.devt = ubi->cdev.dev;
ubi->dev.class = &ubi_class;
ubi->dev.groups = ubi_dev_groups;
dev_set_name(&ubi->dev, UBI_NAME_STR"%d", ubi->ubi_num);
err = device_register(&ubi->dev);
if (err)
return err;
*ref = 1;
return 0;
}
/**
* ubi_sysfs_close - close sysfs for an UBI device.
* @ubi: UBI device description object
*/
static void ubi_sysfs_close(struct ubi_device *ubi)
{
device_unregister(&ubi->dev);
}
/** /**
* kill_volumes - destroy all user volumes. * kill_volumes - destroy all user volumes.
* @ubi: UBI device description object * @ubi: UBI device description object
...@@ -471,27 +436,19 @@ static void kill_volumes(struct ubi_device *ubi) ...@@ -471,27 +436,19 @@ static void kill_volumes(struct ubi_device *ubi)
/** /**
* uif_init - initialize user interfaces for an UBI device. * uif_init - initialize user interfaces for an UBI device.
* @ubi: UBI device description object * @ubi: UBI device description object
* @ref: set to %1 on exit in case of failure if a reference to @ubi->dev was
* taken, otherwise set to %0
* *
* This function initializes various user interfaces for an UBI device. If the * This function initializes various user interfaces for an UBI device. If the
* initialization fails at an early stage, this function frees all the * initialization fails at an early stage, this function frees all the
* resources it allocated, returns an error, and @ref is set to %0. However, * resources it allocated, returns an error.
* if the initialization fails after the UBI device was registered in the
* driver core subsystem, this function takes a reference to @ubi->dev, because
* otherwise the release function ('dev_release()') would free whole @ubi
* object. The @ref argument is set to %1 in this case. The caller has to put
* this reference.
* *
* This function returns zero in case of success and a negative error code in * This function returns zero in case of success and a negative error code in
* case of failure. * case of failure.
*/ */
static int uif_init(struct ubi_device *ubi, int *ref) static int uif_init(struct ubi_device *ubi)
{ {
int i, err; int i, err;
dev_t dev; dev_t dev;
*ref = 0;
sprintf(ubi->ubi_name, UBI_NAME_STR "%d", ubi->ubi_num); sprintf(ubi->ubi_name, UBI_NAME_STR "%d", ubi->ubi_num);
/* /*
...@@ -508,20 +465,17 @@ static int uif_init(struct ubi_device *ubi, int *ref) ...@@ -508,20 +465,17 @@ static int uif_init(struct ubi_device *ubi, int *ref)
return err; return err;
} }
ubi->dev.devt = dev;
ubi_assert(MINOR(dev) == 0); ubi_assert(MINOR(dev) == 0);
cdev_init(&ubi->cdev, &ubi_cdev_operations); cdev_init(&ubi->cdev, &ubi_cdev_operations);
dbg_gen("%s major is %u", ubi->ubi_name, MAJOR(dev)); dbg_gen("%s major is %u", ubi->ubi_name, MAJOR(dev));
ubi->cdev.owner = THIS_MODULE; ubi->cdev.owner = THIS_MODULE;
err = cdev_add(&ubi->cdev, dev, 1); dev_set_name(&ubi->dev, UBI_NAME_STR "%d", ubi->ubi_num);
if (err) { err = cdev_device_add(&ubi->cdev, &ubi->dev);
ubi_err(ubi, "cannot add character device");
goto out_unreg;
}
err = ubi_sysfs_init(ubi, ref);
if (err) if (err)
goto out_sysfs; goto out_unreg;
for (i = 0; i < ubi->vtbl_slots; i++) for (i = 0; i < ubi->vtbl_slots; i++)
if (ubi->volumes[i]) { if (ubi->volumes[i]) {
...@@ -536,11 +490,7 @@ static int uif_init(struct ubi_device *ubi, int *ref) ...@@ -536,11 +490,7 @@ static int uif_init(struct ubi_device *ubi, int *ref)
out_volumes: out_volumes:
kill_volumes(ubi); kill_volumes(ubi);
out_sysfs: cdev_device_del(&ubi->cdev, &ubi->dev);
if (*ref)
get_device(&ubi->dev);
ubi_sysfs_close(ubi);
cdev_del(&ubi->cdev);
out_unreg: out_unreg:
unregister_chrdev_region(ubi->cdev.dev, ubi->vtbl_slots + 1); unregister_chrdev_region(ubi->cdev.dev, ubi->vtbl_slots + 1);
ubi_err(ubi, "cannot initialize UBI %s, error %d", ubi_err(ubi, "cannot initialize UBI %s, error %d",
...@@ -559,8 +509,7 @@ static int uif_init(struct ubi_device *ubi, int *ref) ...@@ -559,8 +509,7 @@ static int uif_init(struct ubi_device *ubi, int *ref)
static void uif_close(struct ubi_device *ubi) static void uif_close(struct ubi_device *ubi)
{ {
kill_volumes(ubi); kill_volumes(ubi);
ubi_sysfs_close(ubi); cdev_device_del(&ubi->cdev, &ubi->dev);
cdev_del(&ubi->cdev);
unregister_chrdev_region(ubi->cdev.dev, ubi->vtbl_slots + 1); unregister_chrdev_region(ubi->cdev.dev, ubi->vtbl_slots + 1);
} }
...@@ -857,7 +806,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ...@@ -857,7 +806,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
int vid_hdr_offset, int max_beb_per1024) int vid_hdr_offset, int max_beb_per1024)
{ {
struct ubi_device *ubi; struct ubi_device *ubi;
int i, err, ref = 0; int i, err;
if (max_beb_per1024 < 0 || max_beb_per1024 > MAX_MTD_UBI_BEB_LIMIT) if (max_beb_per1024 < 0 || max_beb_per1024 > MAX_MTD_UBI_BEB_LIMIT)
return -EINVAL; return -EINVAL;
...@@ -919,6 +868,11 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ...@@ -919,6 +868,11 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
if (!ubi) if (!ubi)
return -ENOMEM; return -ENOMEM;
device_initialize(&ubi->dev);
ubi->dev.release = dev_release;
ubi->dev.class = &ubi_class;
ubi->dev.groups = ubi_dev_groups;
ubi->mtd = mtd; ubi->mtd = mtd;
ubi->ubi_num = ubi_num; ubi->ubi_num = ubi_num;
ubi->vid_hdr_offset = vid_hdr_offset; ubi->vid_hdr_offset = vid_hdr_offset;
...@@ -995,7 +949,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ...@@ -995,7 +949,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
/* Make device "available" before it becomes accessible via sysfs */ /* Make device "available" before it becomes accessible via sysfs */
ubi_devices[ubi_num] = ubi; ubi_devices[ubi_num] = ubi;
err = uif_init(ubi, &ref); err = uif_init(ubi);
if (err) if (err)
goto out_detach; goto out_detach;
...@@ -1045,8 +999,6 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ...@@ -1045,8 +999,6 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
out_debugfs: out_debugfs:
ubi_debugfs_exit_dev(ubi); ubi_debugfs_exit_dev(ubi);
out_uif: out_uif:
get_device(&ubi->dev);
ubi_assert(ref);
uif_close(ubi); uif_close(ubi);
out_detach: out_detach:
ubi_devices[ubi_num] = NULL; ubi_devices[ubi_num] = NULL;
...@@ -1056,10 +1008,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ...@@ -1056,10 +1008,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
out_free: out_free:
vfree(ubi->peb_buf); vfree(ubi->peb_buf);
vfree(ubi->fm_buf); vfree(ubi->fm_buf);
if (ref) put_device(&ubi->dev);
put_device(&ubi->dev);
else
kfree(ubi);
return err; return err;
} }
...@@ -1120,12 +1069,6 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway) ...@@ -1120,12 +1069,6 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway)
if (ubi->bgt_thread) if (ubi->bgt_thread)
kthread_stop(ubi->bgt_thread); kthread_stop(ubi->bgt_thread);
/*
* Get a reference to the device in order to prevent 'dev_release()'
* from freeing the @ubi object.
*/
get_device(&ubi->dev);
ubi_debugfs_exit_dev(ubi); ubi_debugfs_exit_dev(ubi);
uif_close(ubi); uif_close(ubi);
......
...@@ -155,11 +155,10 @@ static void vol_release(struct device *dev) ...@@ -155,11 +155,10 @@ static void vol_release(struct device *dev)
*/ */
int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req) int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
{ {
int i, err, vol_id = req->vol_id, do_free = 1; int i, err, vol_id = req->vol_id;
struct ubi_volume *vol; struct ubi_volume *vol;
struct ubi_vtbl_record vtbl_rec; struct ubi_vtbl_record vtbl_rec;
struct ubi_eba_table *eba_tbl = NULL; struct ubi_eba_table *eba_tbl = NULL;
dev_t dev;
if (ubi->ro_mode) if (ubi->ro_mode)
return -EROFS; return -EROFS;
...@@ -168,6 +167,12 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req) ...@@ -168,6 +167,12 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
if (!vol) if (!vol)
return -ENOMEM; return -ENOMEM;
device_initialize(&vol->dev);
vol->dev.release = vol_release;
vol->dev.parent = &ubi->dev;
vol->dev.class = &ubi_class;
vol->dev.groups = volume_dev_groups;
spin_lock(&ubi->volumes_lock); spin_lock(&ubi->volumes_lock);
if (vol_id == UBI_VOL_NUM_AUTO) { if (vol_id == UBI_VOL_NUM_AUTO) {
/* Find unused volume ID */ /* Find unused volume ID */
...@@ -268,24 +273,13 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req) ...@@ -268,24 +273,13 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
/* Register character device for the volume */ /* Register character device for the volume */
cdev_init(&vol->cdev, &ubi_vol_cdev_operations); cdev_init(&vol->cdev, &ubi_vol_cdev_operations);
vol->cdev.owner = THIS_MODULE; vol->cdev.owner = THIS_MODULE;
dev = MKDEV(MAJOR(ubi->cdev.dev), vol_id + 1);
err = cdev_add(&vol->cdev, dev, 1);
if (err) {
ubi_err(ubi, "cannot add character device");
goto out_mapping;
}
vol->dev.release = vol_release;
vol->dev.parent = &ubi->dev;
vol->dev.devt = dev;
vol->dev.class = &ubi_class;
vol->dev.groups = volume_dev_groups;
vol->dev.devt = MKDEV(MAJOR(ubi->cdev.dev), vol_id + 1);
dev_set_name(&vol->dev, "%s_%d", ubi->ubi_name, vol->vol_id); dev_set_name(&vol->dev, "%s_%d", ubi->ubi_name, vol->vol_id);
err = device_register(&vol->dev); err = cdev_device_add(&vol->cdev, &vol->dev);
if (err) { if (err) {
ubi_err(ubi, "cannot register device"); ubi_err(ubi, "cannot add device");
goto out_cdev; goto out_mapping;
} }
/* Fill volume table record */ /* Fill volume table record */
...@@ -318,28 +312,17 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req) ...@@ -318,28 +312,17 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
* We have registered our device, we should not free the volume * We have registered our device, we should not free the volume
* description object in this function in case of an error - it is * description object in this function in case of an error - it is
* freed by the release function. * freed by the release function.
*
* Get device reference to prevent the release function from being
* called just after sysfs has been closed.
*/ */
do_free = 0; cdev_device_del(&vol->cdev, &vol->dev);
get_device(&vol->dev);
device_unregister(&vol->dev);
out_cdev:
cdev_del(&vol->cdev);
out_mapping: out_mapping:
if (do_free) ubi_eba_destroy_table(eba_tbl);
ubi_eba_destroy_table(eba_tbl);
out_acc: out_acc:
spin_lock(&ubi->volumes_lock); spin_lock(&ubi->volumes_lock);
ubi->rsvd_pebs -= vol->reserved_pebs; ubi->rsvd_pebs -= vol->reserved_pebs;
ubi->avail_pebs += vol->reserved_pebs; ubi->avail_pebs += vol->reserved_pebs;
out_unlock: out_unlock:
spin_unlock(&ubi->volumes_lock); spin_unlock(&ubi->volumes_lock);
if (do_free) put_device(&vol->dev);
kfree(vol);
else
put_device(&vol->dev);
ubi_err(ubi, "cannot create volume %d, error %d", vol_id, err); ubi_err(ubi, "cannot create volume %d, error %d", vol_id, err);
return err; return err;
} }
...@@ -391,8 +374,8 @@ int ubi_remove_volume(struct ubi_volume_desc *desc, int no_vtbl) ...@@ -391,8 +374,8 @@ int ubi_remove_volume(struct ubi_volume_desc *desc, int no_vtbl)
goto out_err; goto out_err;
} }
cdev_del(&vol->cdev); cdev_device_del(&vol->cdev, &vol->dev);
device_unregister(&vol->dev); put_device(&vol->dev);
spin_lock(&ubi->volumes_lock); spin_lock(&ubi->volumes_lock);
ubi->rsvd_pebs -= reserved_pebs; ubi->rsvd_pebs -= reserved_pebs;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment