mtd: nand_base: always initialise oob_poi before writing OOB data

[Linux-MTD Mailing List]

Gitweb:     http://git.infradead.org/?p=mtd-2.6.git;a=commit;h=f722013ee9fd24623df31dec9a91a6d02c3e2f2f
Commit:     f722013ee9fd24623df31dec9a91a6d02c3e2f2f
Parent:     e8a0e41266e9c207ad8ac158cee9547ef1bc90ac
Author:     THOMSON, Adam (Adam) <adam.thomson at alcatel-lucent.com>
AuthorDate: Tue Jun 14 16:52:38 2011 +0200
Committer:  Artem Bityutskiy <artem.bityutskiy at intel.com>
CommitDate: Sun Sep 11 15:02:10 2011 +0300

    mtd: nand_base: always initialise oob_poi before writing OOB data
    
    In nand_do_write_ops() code it is possible for a caller to provide
    ops.oobbuf populated and ops.mode == MTD_OOB_AUTO, which currently
    means that the chip->oob_poi buffer isn't initialised to all 0xFF.
    The nand_fill_oob() method then carries out the task of copying
    the provided OOB data to oob_poi, but with MTD_OOB_AUTO it skips
    areas marked as unavailable by the layout struct, including the
    bad block marker bytes.
    
    An example of this causing issues is when the last OOB data read
    was from the start of a bad block where the markers are not 0xFF,
    and the caller wishes to write new OOB data at the beginning of
    another block. In this scenario the caller would provide OOB data,
    but nand_fill_oob() would skip the bad block marker bytes in
    oob_poi before copying the OOB data provided by the caller.
    This means that when the OOB data is written back to NAND,
    the block is inadvertently marked as bad without the caller knowing.
    This has been witnessed when using YAFFS2 where tags are stored
    in the OOB.
    
    To avoid this oob_poi is always initialised to 0xFF to make sure
    no left over data is inadvertently written back to the OOB area.
    
    Credits to Brian Norris <computersforpeace at gmail.com> for fixing this
    patch.
    
    Signed-off-by: Adam Thomson <adam.thomson at alcatel-lucent.com>
    Signed-off-by: Artem Bityutskiy <dedekind1 at gmail.com>
    Cc: stable at kernel.org [2.6.20+]
---
 drivers/mtd/nand/nand_base.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 1525cd0..408e1d0a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2088,14 +2088,22 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 /**
  * nand_fill_oob - [Internal] Transfer client buffer to oob
- * @chip: nand chip structure
+ * @mtd: MTD device structure
  * @oob: oob data buffer
  * @len: oob data write length
  * @ops: oob ops structure
  */
-static uint8_t *nand_fill_oob(struct nand_chip *chip, uint8_t *oob, size_t len,
-						struct mtd_oob_ops *ops)
+static uint8_t *nand_fill_oob(struct mtd_info *mtd, uint8_t *oob, size_t len,
+			      struct mtd_oob_ops *ops)
 {
+	struct nand_chip *chip = mtd->priv;
+
+	/*
+	 * Initialise to all 0xFF, to avoid the possibility of left over OOB
+	 * data from a previous OOB read.
+	 */
+	memset(chip->oob_poi, 0xff, mtd->oobsize);
+
 	switch (ops->mode) {
 
 	case MTD_OOB_PLACE:
@@ -2192,10 +2200,6 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 	    (chip->pagebuf << chip->page_shift) < (to + ops->len))
 		chip->pagebuf = -1;
 
-	/* If we're not given explicit OOB data, let it be 0xFF */
-	if (likely(!oob))
-		memset(chip->oob_poi, 0xff, mtd->oobsize);
-
 	/* Don't allow multipage oob writes with offset */
 	if (oob && ops->ooboffs && (ops->ooboffs + ops->ooblen > oobmaxlen))
 		return -EINVAL;
@@ -2217,8 +2221,11 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 
 		if (unlikely(oob)) {
 			size_t len = min(oobwritelen, oobmaxlen);
-			oob = nand_fill_oob(chip, oob, len, ops);
+			oob = nand_fill_oob(mtd, oob, len, ops);
 			oobwritelen -= len;
+		} else {
+			/* We still need to erase leftover OOB data */
+			memset(chip->oob_poi, 0xff, mtd->oobsize);
 		}
 
 		ret = chip->write_page(mtd, chip, wbuf, page, cached,
@@ -2392,10 +2399,8 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 	if (page == chip->pagebuf)
 		chip->pagebuf = -1;
 
-	memset(chip->oob_poi, 0xff, mtd->oobsize);
-	nand_fill_oob(chip, ops->oobbuf, ops->ooblen, ops);
+	nand_fill_oob(mtd, ops->oobbuf, ops->ooblen, ops);
 	status = chip->ecc.write_oob(mtd, chip, page & chip->pagemask);
-	memset(chip->oob_poi, 0xff, mtd->oobsize);
 
 	if (status)
 		return status;