linux-kernel - [PATCH] mtd: offset align to block size bofore block operation

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>] [thread-next>] [day] [month] [year] [list]
Message-Id: <1589266715-4168-1-git-send-email-liaoweixiong@allwinnertech.com>
Date:   Tue, 12 May 2020 14:58:35 +0800
From:   WeiXiong Liao <liaoweixiong@...winnertech.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Kees Cook <keescook@...omium.org>,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>
Cc:     linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        WeiXiong Liao <liaoweixiong@...winnertech.com>
Subject: [PATCH] mtd: offset align to block size bofore block operation

The off parameter on mtdpsore_block_*() does not align to block size,
which makes some bugs. For example, a block contains dmesg zones
with number 0 to 3. When user remove all these files, mapped to
these zones, mtdpstore is expected to check whether No.0 to No.3 is
unused then erase this block. However it check No.3 to No.6 because
it get wrongly beginning zonenum from misaligned off.

Signed-off-by: WeiXiong Liao <liaoweixiong@...winnertech.com>
---

This patch bases on series v8 of pstore/blk.
Series Link: https://lore.kernel.org/lkml/20200511233229.27745-1-keescook@chromium.org/

 drivers/mtd/mtdpstore.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/mtdpstore.c b/drivers/mtd/mtdpstore.c
index 06084eff1004..a4fe6060b960 100644
--- a/drivers/mtd/mtdpstore.c
+++ b/drivers/mtd/mtdpstore.c
@@ -27,7 +27,10 @@ static int mtdpstore_block_isbad(struct mtdpstore_context *cxt, loff_t off)
 {
 	int ret;
 	struct mtd_info *mtd = cxt->mtd;
-	u64 blknum = div_u64(off, mtd->erasesize);
+	u64 blknum;
+
+	off = ALIGN_DOWN(off, mtd->erasesize);
+	blknum = div_u64(off, mtd->erasesize);
 
 	if (test_bit(blknum, cxt->badmap))
 		return true;
@@ -46,8 +49,10 @@ static inline int mtdpstore_panic_block_isbad(struct mtdpstore_context *cxt,
 		loff_t off)
 {
 	struct mtd_info *mtd = cxt->mtd;
-	u64 blknum = div_u64(off, mtd->erasesize);
+	u64 blknum;
 
+	off = ALIGN_DOWN(off, mtd->erasesize);
+	blknum = div_u64(off, mtd->erasesize);
 	return test_bit(blknum, cxt->badmap);
 }
 
@@ -75,9 +80,11 @@ static inline void mtdpstore_block_mark_unused(struct mtdpstore_context *cxt,
 		loff_t off)
 {
 	struct mtd_info *mtd = cxt->mtd;
-	u64 zonenum = div_u64(off, cxt->info.kmsg_size);
-	u32 zonecnt = cxt->mtd->erasesize / cxt->info.kmsg_size;
+	u32 zonecnt = mtd->erasesize / cxt->info.kmsg_size;
+	u64 zonenum;
 
+	off = ALIGN_DOWN(off, mtd->erasesize);
+	zonenum = div_u64(off, cxt->info.kmsg_size);
 	while (zonecnt > 0) {
 		dev_dbg(&mtd->dev, "mark zone %llu unused\n", zonenum);
 		clear_bit(zonenum, cxt->usedmap);
@@ -99,9 +106,12 @@ static inline int mtdpstore_is_used(struct mtdpstore_context *cxt, loff_t off)
 static int mtdpstore_block_is_used(struct mtdpstore_context *cxt,
 		loff_t off)
 {
-	u64 zonenum = div_u64(off, cxt->info.kmsg_size);
-	u32 zonecnt = cxt->mtd->erasesize / cxt->info.kmsg_size;
+	struct mtd_info *mtd = cxt->mtd;
+	u32 zonecnt = mtd->erasesize / cxt->info.kmsg_size;
+	u64 zonenum;
 
+	off = ALIGN_DOWN(off, mtd->erasesize);
+	zonenum = div_u64(off, cxt->info.kmsg_size);
 	while (zonecnt > 0) {
 		if (test_bit(zonenum, cxt->usedmap))
 			return true;
@@ -138,9 +148,12 @@ static void mtdpstore_mark_removed(struct mtdpstore_context *cxt, loff_t off)
 static void mtdpstore_block_clear_removed(struct mtdpstore_context *cxt,
 		loff_t off)
 {
-	u64 zonenum = div_u64(off, cxt->info.kmsg_size);
-	u32 zonecnt = cxt->mtd->erasesize / cxt->info.kmsg_size;
+	struct mtd_info *mtd = cxt->mtd;
+	u32 zonecnt = mtd->erasesize / cxt->info.kmsg_size;
+	u64 zonenum;
 
+	off = ALIGN_DOWN(off, mtd->erasesize);
+	zonenum = div_u64(off, cxt->info.kmsg_size);
 	while (zonecnt > 0) {
 		clear_bit(zonenum, cxt->rmmap);
 		zonenum++;
@@ -151,9 +164,12 @@ static void mtdpstore_block_clear_removed(struct mtdpstore_context *cxt,
 static int mtdpstore_block_is_removed(struct mtdpstore_context *cxt,
 		loff_t off)
 {
-	u64 zonenum = div_u64(off, cxt->info.kmsg_size);
-	u32 zonecnt = cxt->mtd->erasesize / cxt->info.kmsg_size;
+	struct mtd_info *mtd = cxt->mtd;
+	u32 zonecnt = mtd->erasesize / cxt->info.kmsg_size;
+	u64 zonenum;
 
+	off = ALIGN_DOWN(off, mtd->erasesize);
+	zonenum = div_u64(off, cxt->info.kmsg_size);
 	while (zonecnt > 0) {
 		if (test_bit(zonenum, cxt->rmmap))
 			return true;
@@ -169,6 +185,7 @@ static int mtdpstore_erase_do(struct mtdpstore_context *cxt, loff_t off)
 	struct erase_info erase;
 	int ret;
 
+	off = ALIGN_DOWN(off, cxt->mtd->erasesize);
 	dev_dbg(&mtd->dev, "try to erase off 0x%llx\n", off);
 	erase.len = cxt->mtd->erasesize;
 	erase.addr = off;
@@ -205,7 +222,6 @@ static ssize_t mtdpstore_erase(size_t size, loff_t off)
 	}
 
 	/* all zones are unused, erase it */
-	off = ALIGN_DOWN(off, cxt->mtd->erasesize);
 	return mtdpstore_erase_do(cxt, off);
 }
 
@@ -235,7 +251,6 @@ static int mtdpstore_security(struct mtdpstore_context *cxt, loff_t off)
 	}
 
 	/* If there is no any empty zone, we have no way but to do erase */
-	off = ALIGN_DOWN(off, erasesize);
 	while (blkcnt--) {
 		div64_u64_rem(off + erasesize, cxt->mtd->size, (u64 *)&off);
 
-- 
1.9.1