Patchwork [01/12] mtd: nand: initialize chip->oob_poi before write

login
register
mail settings
Submitter Brian Norris
Date Aug. 31, 2011, 1:45 a.m.
Message ID <1314755147-17756-2-git-send-email-computersforpeace@gmail.com>
Download mbox | patch
Permalink /patch/112419/
State New
Headers show

Comments

Brian Norris - Aug. 31, 2011, 1:45 a.m.
For raw (i.e., w/o ECC) page writes (i.e., w/o OOB), we may not have
initialized and filled the chip->oob_poi buffer. This can end up writing
junk to the flash if we're not careful. Say, for example, we use
`nandwrite -n' (without OOB). Then nand_do_write_ops calls
chip->write_page, which writes OOB data with some previous, junk data.

This fixes a bug with this commit (from l2-mtd-2.6.git):

  commit a8ee364bbf14861d5d0af39c4da06c30441895fb
  mtd: nand_base: always initialise oob_poi before writing OOB data

That commit removed the memset from under a conditional for:

  if (likely(!oob))

and moved it (indirectly) to the `nand_fill_oob()' function, which was
under:

  if (unlikely(oob))

Though the "likely" and "unlikely" can be confusing, these are not the
same conditions :)

And if the buggy commit is going stable, this should go stable (or just
amend it) as well.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Adam Thomson <adam.thomson@alcatel-lucent.com>
---
If the buggy commit is going into -stable, this should go -stable as
well (or just amend the original).

 drivers/mtd/nand/nand_base.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Artem Bityutskiy - Sept. 11, 2011, 11:31 a.m.
On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> And if the buggy commit is going stable, this should go stable (or just
> amend it) as well.

Amended Adam's patch, thanks.
THOMSON, Adam (Adam) - Sept. 12, 2011, 9:20 a.m.
Brian Norris wrote:

> For raw (i.e., w/o ECC) page writes (i.e., w/o OOB), we may 
> not have initialized and filled the chip->oob_poi buffer. 
> This can end up writing junk to the flash if we're not 
> careful. Say, for example, we use `nandwrite -n' (without 
> OOB). Then nand_do_write_ops calls
> chip->write_page, which writes OOB data with some previous, junk data.
> 
> This fixes a bug with this commit (from l2-mtd-2.6.git):
> 
>   commit a8ee364bbf14861d5d0af39c4da06c30441895fb
>   mtd: nand_base: always initialise oob_poi before writing OOB data

That's annoying that I missed that. :( Thanks for spotting said issue and providing a fix.

Regards

Adam

Patch

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d2ee68a..273e6a5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2227,6 +2227,9 @@  static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 			size_t len = min(oobwritelen, oobmaxlen);
 			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,