UBI: fix attaching error path

In the error path of 'ubi_attach_mtd_dev()' we have a tricky situation:
we have to release things differently depending on at which point
the failure happening. Namely, if @ubi->dev is not initialized, we have
to free everything ourselves. But if it was, we should not free the @ubi
object, because it will be freed in the 'dev_release()' function. And
we did not get this situation right.

This patch introduces additional argument to the 'uif_init()' function.
On exit, this argument indicates whether the final 'free(ubi)' will
happen in 'dev_release()' or not. So the caller always knows how to
properly release the resources.

Impact: all memory is now correctly released when UBI fails to attach
        an MTD device.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 99ff57d3ac683cc7e5155295eace106d4dbf069f..bc45ef9af17d8f94b3cd2bed56e6771837296082 100644 (file)
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -365,11 +365,13 @@ static void dev_release(struct device *dev)
 /**
  * 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)
+static int ubi_sysfs_init(struct ubi_device *ubi, int *ref)
 {
        int err;
 
@@ -381,6 +383,7 @@ static int ubi_sysfs_init(struct ubi_device *ubi)
        if (err)
                return err;
 
+       *ref = 1;
        err = device_create_file(&ubi->dev, &dev_eraseblock_size);
        if (err)
                return err;
@@ -436,7 +439,7 @@ static void ubi_sysfs_close(struct ubi_device *ubi)
 }
 
 /**
- * kill_volumes - destroy all volumes.
+ * kill_volumes - destroy all user volumes.
  * @ubi: UBI device description object
  */
 static void kill_volumes(struct ubi_device *ubi)
@@ -448,37 +451,30 @@ static void kill_volumes(struct ubi_device *ubi)
                        ubi_free_volume(ubi, ubi->volumes[i]);
 }
 
-/**
- * free_user_volumes - free all user volumes.
- * @ubi: UBI device description object
- *
- * Normally the volumes are freed at the release function of the volume device
- * objects. However, on error paths the volumes have to be freed before the
- * device objects have been initialized.
- */
-static void free_user_volumes(struct ubi_device *ubi)
-{
-       int i;
-
-       for (i = 0; i < ubi->vtbl_slots; i++)
-               if (ubi->volumes[i]) {
-                       kfree(ubi->volumes[i]->eba_tbl);
-                       kfree(ubi->volumes[i]);
-               }
-}
-
 /**
  * uif_init - initialize user interfaces 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, otherwise set to %0
+ *
+ * This function initializes various user interfaces for an UBI device. If 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,
+ * 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
- * case of failure. Note, this function destroys all volumes if it fails.
+ * case of failure.
  */
-static int uif_init(struct ubi_device *ubi)
+static int uif_init(struct ubi_device *ubi, int *ref)
 {
        int i, err;
        dev_t dev;
 
+       *ref = 0;
        sprintf(ubi->ubi_name, UBI_NAME_STR "%d", ubi->ubi_num);
 
        /*
@@ -506,7 +502,7 @@ static int uif_init(struct ubi_device *ubi)
                goto out_unreg;
        }
 
-       err = ubi_sysfs_init(ubi);
+       err = ubi_sysfs_init(ubi, ref);
        if (err)
                goto out_sysfs;
 
@@ -524,6 +520,8 @@ static int uif_init(struct ubi_device *ubi)
 out_volumes:
        kill_volumes(ubi);
 out_sysfs:
+       if (*ref)
+               get_device(&ubi->dev);
        ubi_sysfs_close(ubi);
        cdev_del(&ubi->cdev);
 out_unreg:
@@ -877,7 +875,7 @@ static int ubi_reboot_notifier(struct notifier_block *n, unsigned long state,
 int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
 {
        struct ubi_device *ubi;
-       int i, err, do_free = 1;
+       int i, err, ref = 0;
 
        /*
         * Check if we already have the same MTD device attached.
@@ -977,9 +975,9 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
                        goto out_detach;
        }
 
-       err = uif_init(ubi);
+       err = uif_init(ubi, &ref);
        if (err)
-               goto out_nofree;
+               goto out_detach;
 
        ubi->bgt_thread = kthread_create(ubi_thread, ubi, ubi->bgt_name);
        if (IS_ERR(ubi->bgt_thread)) {
@@ -1027,12 +1025,8 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
 
 out_uif:
        uif_close(ubi);
-out_nofree:
-       do_free = 0;
 out_detach:
        ubi_wl_close(ubi);
-       if (do_free)
-               free_user_volumes(ubi);
        free_internal_volumes(ubi);
        vfree(ubi->vtbl);
 out_free:
@@ -1041,7 +1035,10 @@ out_free:
 #ifdef CONFIG_MTD_UBI_DEBUG_PARANOID
        vfree(ubi->dbg_peb_buf);
 #endif
-       kfree(ubi);
+       if (ref)
+               put_device(&ubi->dev);
+       else
+               kfree(ubi);
        return err;
 }
 
@@ -1098,7 +1095,7 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway)
 
        /*
         * Get a reference to the device in order to prevent 'dev_release()'
-        * from freeing @ubi object.
+        * from freeing the @ubi object.
         */
        get_device(&ubi->dev);