From: Artem Bityutskiy <dedekind1@gmail.com>
To: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Subject: [PATCH 3/5] UBI: fix attaching error path
Date: Wed, 27 Jan 2010 17:18:58 +0200 [thread overview]
Message-ID: <1264605540-13144-4-git-send-email-dedekind1@gmail.com> (raw)
In-Reply-To: <1264605540-13144-1-git-send-email-dedekind1@gmail.com>
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
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>
---
drivers/mtd/ubi/build.c | 63 ++++++++++++++++++++++------------------------
1 files changed, 30 insertions(+), 33 deletions(-)
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 99ff57d..bc45ef9 100644
--- 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)
@@ -449,36 +452,29 @@ static void kill_volumes(struct ubi_device *ubi)
}
/**
- * 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);
--
1.6.6
next prev parent reply other threads:[~2010-01-27 15:19 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-27 15:18 [PATCH 0/9] UBI patches for the next merge window Artem Bityutskiy
2010-01-27 15:18 ` [PATCH 1/5] UBI: mark few variables as __initdata Artem Bityutskiy
2010-01-27 15:18 ` [PATCH 2/5] UBI: support attaching by MTD character device name Artem Bityutskiy
2010-01-27 15:18 ` Artem Bityutskiy [this message]
2010-01-27 15:18 ` [PATCH 4/5] UBI: simplify debugging return codes Artem Bityutskiy
2010-01-27 15:19 ` [PATCH 5/5] UBI: add write checking Artem Bityutskiy
2010-01-27 17:11 ` [PATCH 0/9] UBI patches for the next merge window Artem Bityutskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1264605540-13144-4-git-send-email-dedekind1@gmail.com \
--to=dedekind1@gmail.com \
--cc=Artem.Bityutskiy@nokia.com \
--cc=linux-mtd@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox