diff options
author | Sebastian Siewior <bigeasy@linutronix.de> | 2015-11-26 15:23:48 -0500 |
---|---|---|
committer | Richard Weinberger <richard@nod.at> | 2015-12-16 16:52:46 -0500 |
commit | 1a31b20cd81d5cbc7ec6e24cb08066009a1ca32d (patch) | |
tree | cc247bb7803e3576995306bf143c76a95b34c040 | |
parent | 2e69d4912f2fc9d4cd952311d58ceae1cd83057b (diff) |
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>
-rw-r--r-- | drivers/mtd/ubi/wl.c | 52 |
1 files changed, 28 insertions, 24 deletions
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index eb4489f9082f..f73233fa737c 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c | |||
@@ -603,6 +603,7 @@ static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e, | |||
603 | return 0; | 603 | return 0; |
604 | } | 604 | } |
605 | 605 | ||
606 | static int __erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk); | ||
606 | /** | 607 | /** |
607 | * do_sync_erase - run the erase worker synchronously. | 608 | * do_sync_erase - run the erase worker synchronously. |
608 | * @ubi: UBI device description object | 609 | * @ubi: UBI device description object |
@@ -615,20 +616,16 @@ static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e, | |||
615 | static int do_sync_erase(struct ubi_device *ubi, struct ubi_wl_entry *e, | 616 | static int do_sync_erase(struct ubi_device *ubi, struct ubi_wl_entry *e, |
616 | int vol_id, int lnum, int torture) | 617 | int vol_id, int lnum, int torture) |
617 | { | 618 | { |
618 | struct ubi_work *wl_wrk; | 619 | struct ubi_work wl_wrk; |
619 | 620 | ||
620 | dbg_wl("sync erase of PEB %i", e->pnum); | 621 | dbg_wl("sync erase of PEB %i", e->pnum); |
621 | 622 | ||
622 | wl_wrk = kmalloc(sizeof(struct ubi_work), GFP_NOFS); | 623 | wl_wrk.e = e; |
623 | if (!wl_wrk) | 624 | wl_wrk.vol_id = vol_id; |
624 | return -ENOMEM; | 625 | wl_wrk.lnum = lnum; |
625 | 626 | wl_wrk.torture = torture; | |
626 | wl_wrk->e = e; | ||
627 | wl_wrk->vol_id = vol_id; | ||
628 | wl_wrk->lnum = lnum; | ||
629 | wl_wrk->torture = torture; | ||
630 | 627 | ||
631 | return erase_worker(ubi, wl_wrk, 0); | 628 | return __erase_worker(ubi, &wl_wrk); |
632 | } | 629 | } |
633 | 630 | ||
634 | /** | 631 | /** |
@@ -1014,7 +1011,7 @@ out_unlock: | |||
1014 | } | 1011 | } |
1015 | 1012 | ||
1016 | /** | 1013 | /** |
1017 | * erase_worker - physical eraseblock erase worker function. | 1014 | * __erase_worker - physical eraseblock erase worker function. |
1018 | * @ubi: UBI device description object | 1015 | * @ubi: UBI device description object |
1019 | * @wl_wrk: the work object | 1016 | * @wl_wrk: the work object |
1020 | * @shutdown: non-zero if the worker has to free memory and exit | 1017 | * @shutdown: non-zero if the worker has to free memory and exit |
@@ -1025,8 +1022,7 @@ out_unlock: | |||
1025 | * needed. Returns zero in case of success and a negative error code in case of | 1022 | * needed. Returns zero in case of success and a negative error code in case of |
1026 | * failure. | 1023 | * failure. |
1027 | */ | 1024 | */ |
1028 | static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk, | 1025 | static int __erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk) |
1029 | int shutdown) | ||
1030 | { | 1026 | { |
1031 | struct ubi_wl_entry *e = wl_wrk->e; | 1027 | struct ubi_wl_entry *e = wl_wrk->e; |
1032 | int pnum = e->pnum; | 1028 | int pnum = e->pnum; |
@@ -1034,21 +1030,11 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk, | |||
1034 | int lnum = wl_wrk->lnum; | 1030 | int lnum = wl_wrk->lnum; |
1035 | int err, available_consumed = 0; | 1031 | int err, available_consumed = 0; |
1036 | 1032 | ||
1037 | if (shutdown) { | ||
1038 | dbg_wl("cancel erasure of PEB %d EC %d", pnum, e->ec); | ||
1039 | kfree(wl_wrk); | ||
1040 | wl_entry_destroy(ubi, e); | ||
1041 | return 0; | ||
1042 | } | ||
1043 | |||
1044 | dbg_wl("erase PEB %d EC %d LEB %d:%d", | 1033 | dbg_wl("erase PEB %d EC %d LEB %d:%d", |
1045 | pnum, e->ec, wl_wrk->vol_id, wl_wrk->lnum); | 1034 | pnum, e->ec, wl_wrk->vol_id, wl_wrk->lnum); |
1046 | 1035 | ||
1047 | err = sync_erase(ubi, e, wl_wrk->torture); | 1036 | err = sync_erase(ubi, e, wl_wrk->torture); |
1048 | if (!err) { | 1037 | if (!err) { |
1049 | /* Fine, we've erased it successfully */ | ||
1050 | kfree(wl_wrk); | ||
1051 | |||
1052 | spin_lock(&ubi->wl_lock); | 1038 | spin_lock(&ubi->wl_lock); |
1053 | wl_tree_add(e, &ubi->free); | 1039 | wl_tree_add(e, &ubi->free); |
1054 | ubi->free_count++; | 1040 | ubi->free_count++; |
@@ -1066,7 +1052,6 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk, | |||
1066 | } | 1052 | } |
1067 | 1053 | ||
1068 | ubi_err(ubi, "failed to erase PEB %d, error %d", pnum, err); | 1054 | ubi_err(ubi, "failed to erase PEB %d, error %d", pnum, err); |
1069 | kfree(wl_wrk); | ||
1070 | 1055 | ||
1071 | if (err == -EINTR || err == -ENOMEM || err == -EAGAIN || | 1056 | if (err == -EINTR || err == -ENOMEM || err == -EAGAIN || |
1072 | err == -EBUSY) { | 1057 | err == -EBUSY) { |
@@ -1150,6 +1135,25 @@ out_ro: | |||
1150 | return err; | 1135 | return err; |
1151 | } | 1136 | } |
1152 | 1137 | ||
1138 | static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk, | ||
1139 | int shutdown) | ||
1140 | { | ||
1141 | int ret; | ||
1142 | |||
1143 | if (shutdown) { | ||
1144 | struct ubi_wl_entry *e = wl_wrk->e; | ||
1145 | |||
1146 | dbg_wl("cancel erasure of PEB %d EC %d", e->pnum, e->ec); | ||
1147 | kfree(wl_wrk); | ||
1148 | wl_entry_destroy(ubi, e); | ||
1149 | return 0; | ||
1150 | } | ||
1151 | |||
1152 | ret = __erase_worker(ubi, wl_wrk); | ||
1153 | kfree(wl_wrk); | ||
1154 | return ret; | ||
1155 | } | ||
1156 | |||
1153 | /** | 1157 | /** |
1154 | * ubi_wl_put_peb - return a PEB to the wear-leveling sub-system. | 1158 | * ubi_wl_put_peb - return a PEB to the wear-leveling sub-system. |
1155 | * @ubi: UBI device description object | 1159 | * @ubi: UBI device description object |