mtd: ubi: fixup error correction in do_sync_erase()

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>
This commit is contained in:
Sebastian Siewior 2015-11-26 21:23:48 +01:00 committed by Richard Weinberger
parent 2e69d4912f
commit 1a31b20cd8

View File

@ -603,6 +603,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
@ -615,20 +616,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);
}
/**
@ -1014,7 +1011,7 @@ static int ensure_wear_leveling(struct ubi_device *ubi, int nested)
}
/**
* 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
@ -1025,8 +1022,7 @@ static int ensure_wear_leveling(struct ubi_device *ubi, int nested)
* 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;
@ -1034,21 +1030,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);
spin_lock(&ubi->wl_lock);
wl_tree_add(e, &ubi->free);
ubi->free_count++;
@ -1066,7 +1052,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) {
@ -1150,6 +1135,25 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
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