linux-kernel - [PATCH] UBI: Fastmap: Fix possible fastmap inconsistency

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Mon,  8 Dec 2014 10:19:26 +0100
From:	Richard Weinberger <richard@....at>
To:	dedekind1@...il.com
Cc:	tlinder@...eaurora.org, linux-mtd@...ts.infradead.org,
	linux-kernel@...r.kernel.org, Richard Weinberger <richard@....at>
Subject: [PATCH] UBI: Fastmap: Fix possible fastmap inconsistency

Currently we call ubi_refill_pools() before all UBI meta data
is collected for the fastmap on-flash data structure.
It can happen that a second thread requests a new PEB and returns
it immediately to the free list between ubi_refill_pools()
and collecting meta data.
In this case the same PEB will be in the free list and a fastmap
pool.
Upon attach time fastmap is able to detect that inconsistency
and will fall back to scanning mode.
But we can do better.

To bypass the issue just call ubi_refill_pools() under the same
wl_lock as the meta data collecting happens.
If fastmap is disabled we can still call ubi_refill_pools() early.

Signed-off-by: Richard Weinberger <richard@....at>
---
 drivers/mtd/ubi/fastmap-wl.c |  4 ----
 drivers/mtd/ubi/fastmap.c    | 17 +++++++++--------
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/ubi/fastmap-wl.c b/drivers/mtd/ubi/fastmap-wl.c
index 8f109cc..6883146 100644
--- a/drivers/mtd/ubi/fastmap-wl.c
+++ b/drivers/mtd/ubi/fastmap-wl.c
@@ -226,8 +226,6 @@ void ubi_refill_pools(struct ubi_device *ubi)
 	struct ubi_wl_entry *e;
 	int enough;
 
-	spin_lock(&ubi->wl_lock);
-
 	return_unused_pool_pebs(ubi, wl_pool);
 	return_unused_pool_pebs(ubi, pool);
 
@@ -270,8 +268,6 @@ void ubi_refill_pools(struct ubi_device *ubi)
 
 	wl_pool->used = 0;
 	pool->used = 0;
-
-	spin_unlock(&ubi->wl_lock);
 }
 
 /**
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 976e498..05b3c00 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -1121,6 +1121,8 @@ static int ubi_write_fastmap(struct ubi_device *ubi,
 	spin_lock(&ubi->volumes_lock);
 	spin_lock(&ubi->wl_lock);
 
+	ubi_refill_pools(ubi);
+
 	fmsb = (struct ubi_fm_sb *)fm_raw;
 	fm_pos += sizeof(*fmsb);
 	ubi_assert(fm_pos <= ubi->fm_size);
@@ -1477,19 +1479,14 @@ int ubi_update_fastmap(struct ubi_device *ubi, struct ubi_wl_entry **forced_anch
 
 	down_write(&ubi->fm_protect);
 
-	ubi_refill_pools(ubi);
-
 	if (ubi->ro_mode || ubi->fm_disabled) {
+		spin_lock(&ubi->wl_lock);
+		ubi_refill_pools(ubi);
+		spin_unlock(&ubi->wl_lock);
 		up_write(&ubi->fm_protect);
 		return 0;
 	}
 
-	ret = ubi_ensure_anchor_pebs(ubi);
-	if (ret) {
-		up_write(&ubi->fm_protect);
-		return ret;
-	}
-
 	new_fm = kzalloc(sizeof(*new_fm), GFP_KERNEL);
 	if (!new_fm) {
 		up_write(&ubi->fm_protect);
@@ -1614,6 +1611,10 @@ int ubi_update_fastmap(struct ubi_device *ubi, struct ubi_wl_entry **forced_anch
 	if (ret)
 		goto err;
 
+	ret = ubi_ensure_anchor_pebs(ubi);
+	if (ret)
+		goto err;
+
 out_unlock:
 	up_write(&ubi->fm_protect);
 	kfree(old_fm);
-- 
1.8.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/