diff mbox

[1/5] mtd: do not assume oobsize is power of 2

Message ID 1314145056-5233-1-git-send-email-computersforpeace@gmail.com
State Accepted
Commit 305b93f180b221789a6213bf3d298c6735102da1
Headers show

Commit Message

Brian Norris Aug. 24, 2011, 12:17 a.m. UTC
Previous generations of MTDs all used OOB sizes that were powers of 2,
(e.g., 64, 128). However, newer generations of flash, especially NAND,
use irregular OOB sizes that are not powers of 2 (e.g., 218, 224, 448).
This means we cannot use masks like "mtd->oobsize - 1" to assume that we
will get a proper bitmask for OOB operations.

These masks are really only intended to hide the "page" portion of the
offset, leaving any OOB offset intact, so a masking with the writesize
(which *is* always a power of 2) is valid and makes more sense.

This has been tested for read/write of NAND devices (nanddump/nandwrite)
using nandsim and actual NAND flash.

Cc: stable@kernel.org [2.6.30+]
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
Changes since v1 as "RFC": instead of dropping the mask, we use
    writesize instead.

Stable: this error actually appeared way back in 2.6.17, but it was
    cut-and-pasted in the 2.6.30-rcX cycle, so it won't apply cleanly.
    Anyway, I don't think we don't maintain -stable that far back.

 drivers/mtd/mtdchar.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

Comments

Artem Bityutskiy Aug. 24, 2011, 1:10 p.m. UTC | #1
On Tue, 2011-08-23 at 17:17 -0700, Brian Norris wrote:
> Previous generations of MTDs all used OOB sizes that were powers of 2,
> (e.g., 64, 128). However, newer generations of flash, especially NAND,
> use irregular OOB sizes that are not powers of 2 (e.g., 218, 224, 448).
> This means we cannot use masks like "mtd->oobsize - 1" to assume that we
> will get a proper bitmask for OOB operations.
> 
> These masks are really only intended to hide the "page" portion of the
> offset, leaving any OOB offset intact, so a masking with the writesize
> (which *is* always a power of 2) is valid and makes more sense.
> 
> This has been tested for read/write of NAND devices (nanddump/nandwrite)
> using nandsim and actual NAND flash.
> 
> Cc: stable@kernel.org [2.6.30+]
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

I removed -stable Cc, because this is not a bug-fix, and I feel this is
a bit risky for -stable. And pushed, thanks.

Pushed also all the other patches, and amended the MEMOOBSEL removal
patch.
diff mbox

Patch

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index a75d555..b206254 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -410,7 +410,7 @@  static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
 		return ret;
 
 	ops.ooblen = length;
-	ops.ooboffs = start & (mtd->oobsize - 1);
+	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
 	ops.mode = MTD_OOB_PLACE;
 
@@ -421,7 +421,7 @@  static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
 	if (IS_ERR(ops.oobbuf))
 		return PTR_ERR(ops.oobbuf);
 
-	start &= ~((uint64_t)mtd->oobsize - 1);
+	start &= ~((uint64_t)mtd->writesize - 1);
 	ret = mtd->write_oob(mtd, start, &ops);
 
 	if (ops.oobretlen > 0xFFFFFFFFU)
@@ -452,7 +452,7 @@  static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
 		return ret;
 
 	ops.ooblen = length;
-	ops.ooboffs = start & (mtd->oobsize - 1);
+	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
 	ops.mode = MTD_OOB_PLACE;
 
@@ -463,7 +463,7 @@  static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
 	if (!ops.oobbuf)
 		return -ENOMEM;
 
-	start &= ~((uint64_t)mtd->oobsize - 1);
+	start &= ~((uint64_t)mtd->writesize - 1);
 	ret = mtd->read_oob(mtd, start, &ops);
 
 	if (put_user(ops.oobretlen, retp))