aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSebastian Siewior <bigeasy@linutronix.de>2015-11-26 15:23:48 -0500
committerRichard Weinberger <richard@nod.at>2015-12-16 16:52:46 -0500
commit1a31b20cd81d5cbc7ec6e24cb08066009a1ca32d (patch)
treecc247bb7803e3576995306bf143c76a95b34c040
parent2e69d4912f2fc9d4cd952311d58ceae1cd83057b (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.c52
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
606static 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,
615static int do_sync_erase(struct ubi_device *ubi, struct ubi_wl_entry *e, 616static 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 */
1028static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk, 1025static 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
1138static 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