From: Teresa Remmet <t.remmet@phytec.de>
To: barebox@lists.infradead.org
Subject: [PATCH v2 58/62] mtd: ubi: fixup error correction in do_sync_erase()
Date: Fri, 27 May 2016 09:44:51 +0200 [thread overview]
Message-ID: <1464335095-35180-59-git-send-email-t.remmet@phytec.de> (raw)
In-Reply-To: <1464335095-35180-1-git-send-email-t.remmet@phytec.de>
From: Sebastian Siewior <bigeasy@linutronix.de>
Since fastmap we gained do_sync_erase(). This function can return an error
and its error handling isn't obvious. First the memory allocation for
struct ubi_work can fail and as such struct ubi_wl_entry is leaked.
However if the memory allocation succeeds then the tail function takes
care of the struct ubi_wl_entry. A free here could result in a double
free.
To make the error handling simpler, I split the tail function into one
piece which does the work and another which frees the struct ubi_work
which is passed as argument. As result do_sync_erase() can keep the
struct on stack and we get rid of one error source.
Cc: <stable@vger.kernel.org>
Fixes: 8199b901a ("UBI: Add fastmap support to the WL sub-system")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Richard Weinberger <richard@nod.at>
Conflicts:
drivers/mtd/ubi/wl.c
Fixed conflicts.
Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
drivers/mtd/ubi/wl.c | 52 ++++++++++++++++++++++++++++------------------------
1 file changed, 28 insertions(+), 24 deletions(-)
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 2d6c6ae..1feef3f 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -578,6 +578,7 @@ static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e,
return 0;
}
+static int __erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk);
/**
* do_sync_erase - run the erase worker synchronously.
* @ubi: UBI device description object
@@ -590,20 +591,16 @@ static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e,
static int do_sync_erase(struct ubi_device *ubi, struct ubi_wl_entry *e,
int vol_id, int lnum, int torture)
{
- struct ubi_work *wl_wrk;
+ struct ubi_work wl_wrk;
dbg_wl("sync erase of PEB %i", e->pnum);
- wl_wrk = kmalloc(sizeof(struct ubi_work), GFP_NOFS);
- if (!wl_wrk)
- return -ENOMEM;
-
- wl_wrk->e = e;
- wl_wrk->vol_id = vol_id;
- wl_wrk->lnum = lnum;
- wl_wrk->torture = torture;
+ wl_wrk.e = e;
+ wl_wrk.vol_id = vol_id;
+ wl_wrk.lnum = lnum;
+ wl_wrk.torture = torture;
- return erase_worker(ubi, wl_wrk, 0);
+ return __erase_worker(ubi, &wl_wrk);
}
/**
@@ -971,7 +968,7 @@ out_unlock:
}
/**
- * erase_worker - physical eraseblock erase worker function.
+ * __erase_worker - physical eraseblock erase worker function.
* @ubi: UBI device description object
* @wl_wrk: the work object
* @shutdown: non-zero if the worker has to free memory and exit
@@ -982,8 +979,7 @@ out_unlock:
* needed. Returns zero in case of success and a negative error code in case of
* failure.
*/
-static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
- int shutdown)
+static int __erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk)
{
struct ubi_wl_entry *e = wl_wrk->e;
int pnum = e->pnum;
@@ -991,21 +987,11 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
int lnum = wl_wrk->lnum;
int err, available_consumed = 0;
- if (shutdown) {
- dbg_wl("cancel erasure of PEB %d EC %d", pnum, e->ec);
- kfree(wl_wrk);
- wl_entry_destroy(ubi, e);
- return 0;
- }
-
dbg_wl("erase PEB %d EC %d LEB %d:%d",
pnum, e->ec, wl_wrk->vol_id, wl_wrk->lnum);
err = sync_erase(ubi, e, wl_wrk->torture);
if (!err) {
- /* Fine, we've erased it successfully */
- kfree(wl_wrk);
-
wl_tree_add(e, &ubi->free);
ubi->free_count++;
@@ -1021,7 +1007,6 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
}
ubi_err(ubi, "failed to erase PEB %d, error %d", pnum, err);
- kfree(wl_wrk);
if (err == -EINTR || err == -ENOMEM || err == -EAGAIN ||
err == -EBUSY) {
@@ -1093,6 +1078,25 @@ out_ro:
return err;
}
+static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
+ int shutdown)
+{
+ int ret;
+
+ if (shutdown) {
+ struct ubi_wl_entry *e = wl_wrk->e;
+
+ dbg_wl("cancel erasure of PEB %d EC %d", e->pnum, e->ec);
+ kfree(wl_wrk);
+ wl_entry_destroy(ubi, e);
+ return 0;
+ }
+
+ ret = __erase_worker(ubi, wl_wrk);
+ kfree(wl_wrk);
+ return ret;
+}
+
/**
* ubi_wl_put_peb - return a PEB to the wear-leveling sub-system.
* @ubi: UBI device description object
--
1.9.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
next prev parent reply other threads:[~2016-05-27 7:46 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-27 7:43 [PATCH v2 00/62] Update UBI Teresa Remmet
2016-05-27 7:43 ` [PATCH v2 01/62] UBI: add missing kmem_cache_free() in process_pool_aeb error path Teresa Remmet
2016-05-27 7:43 ` [PATCH v2 02/62] UBI: Improve comment on work_sem Teresa Remmet
2016-05-27 7:43 ` [PATCH v2 03/62] UBI: ubi_eba_read_leb: Remove in vain variable assignment Teresa Remmet
2016-05-27 7:43 ` [PATCH v2 04/62] UBI: wl: Rename cancel flag to shutdown Teresa Remmet
2016-05-27 7:43 ` [PATCH v2 05/62] UBI: Fix trivial typo in __schedule_ubi_work Teresa Remmet
2016-05-27 7:43 ` [PATCH v2 06/62] UBI: Fastmap: Calc fastmap size correctly Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 07/62] UBI: Extend UBI layer debug/messaging capabilities Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 08/62] UBI: vtbl: Use ubi_eba_atomic_leb_change() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 09/62] UBI: Fix double free after do_sync_erase() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 10/62] UBI: Fix invalid vfree() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 11/62] UBI: extend UBI layer debug/messaging capabilities - cosmetics Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 12/62] UBI: clean-up printing helpers Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 13/62] UBI: do propagate positive error codes up Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 14/62] UBI: Fastmap: Care about the protection queue Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 15/62] UBI: fix missing brace control flow Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 16/62] UBI: account for bitflips in both the VID header and data Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 17/62] UBI: fix out of bounds write Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 18/62] UBI: initialize LEB number variable Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 19/62] UBI: align comment for readability Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 20/62] UBI: Split __wl_get_peb() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 21/62] UBI: Fastmap: Make ubi_refill_pools() fair Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 22/62] UBI: Fastmap: Don't allocate new ubi_wl_entry objects Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 23/62] UBI: Fastmap: Fix memory leaks while closing the WL sub-system Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 24/62] UBI: Fastmap: Notify user in case of an ubi_update_fastmap() failure Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 25/62] UBI: Fastmap: Wrap fastmap specific function in a ifdef Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 26/62] UBI: Fastmap: Fix fastmap usage in ubi_volume_notify() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 27/62] UBI: Fastmap: Fix race in ubi_eba_atomic_leb_change() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 28/62] UBI: Fastmap: Remove bogus ubi_assert() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 29/62] UBI: Fastmap: Remove eba_orphans logic Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 30/62] UBI: Fastmap: Switch to ro mode if invalidate_fastmap() fails Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 31/62] UBI: Fastmap: Make WL pool size 50% of user pool size Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 32/62] UBI: Fastmap: Fix leb_count unbalance Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 33/62] UBI: Fastmap: Set used_ebs only for static volumes Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 34/62] UBI: Fastmap: Prepare for variable sized fastmaps Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 35/62] UBI: Fastmap: Rework fastmap error paths Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 36/62] UBI: Fix stale pointers in ubi->lookuptbl Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 37/62] UBI: Move fastmap specific functions out of wl.c Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 38/62] UBI: Add accessor functions for WL data structures Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 39/62] UBI: Fastmap: Wire up WL accessor functions Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 40/62] UBI: Fastmap: Introduce ubi_fastmap_init() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 41/62] UBI: Fastmap: Introduce may_reserve_for_fm() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 42/62] UBI: Fastmap: Remove is_fm_block() Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 43/62] UBI: Fastmap: Fall back to scanning mode after ECC error Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 44/62] UBI: Fastmap: Use max() to get the larger value Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 45/62] UBI: Fastmap: Remove unnecessary `\' Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 46/62] UBI: Fastmap: Rename variables to make them meaningful Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 47/62] UBI: Init vol->reserved_pebs by assignment Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 48/62] UBI: Fastmap: Do not add vol if it already exists Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 49/62] UBI: add a helper function for updatting on-flash layout volumes Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 50/62] UBI: Remove unnecessary `\' Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 51/62] UBI: Validate data_size Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 52/62] UBI: return ENOSPC if no enough space available Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 53/62] UBI: Fastmap: Simplify expression Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 54/62] UBI: Fix typo in comment Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 55/62] UBI: Fix debug message Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 56/62] UBI: Fastmap: Fix PEB array type Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 57/62] UBI: fix use of "VID" vs. "EC" in header self-check Teresa Remmet
2016-05-27 7:44 ` Teresa Remmet [this message]
2016-05-27 7:44 ` [PATCH v2 59/62] mtd: ubi: don't leak e if schedule_erase() fails Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 60/62] mtd: ubi: wl: avoid erasing a PEB which is empty Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 61/62] ubi: Fix out of bounds write in volume update code Teresa Remmet
2016-05-27 7:44 ` [PATCH v2 62/62] mtd: UBI: Remove ubi_free_fastmap Teresa Remmet
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=1464335095-35180-59-git-send-email-t.remmet@phytec.de \
--to=t.remmet@phytec.de \
--cc=barebox@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