[PATCH] mtd: nand: fix MTD_MODE_RAW writes

[PATCH] mtd: nand: fix MTD_MODE_RAW writes

[Jon Povey]

RAW writes were broken by 782ce79a45b3b850b108896fcf7da26754061c8f
which introduced a check of ops->ooboffs in nand_do_write_ops().

When writing in RAW mode this is called with an ops struct on the stack
of mtdchar.c:mtd_write() which does not initialise ops->ooboffs, so it
is garbage and fails this test.

This test does not make sense if ops->oobbuf is NULL, which it is in the
RAW write path, so include that in the test.

Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>
CC: Maxim Levitsky <maximlevitsky@gmail.com>
CC: David Woodhouse <David.Woodhouse@intel.com>
---
I am not sure this is the right way to fix this, as I don't really understand
what the test was trying to fix/prevent.

This is really just me showing my shotgun debugging solution that seems to
work for me, and see what people have to say about it / how to fix properly.

This was an unpleasant regression for me: I use MTD_MODE_RAW to write a
bootloader, and this got close to being a field update bricker..

 drivers/mtd/nand/nand_base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 044be4c..b3469a7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2219,7 +2219,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 		memset(chip->oob_poi, 0xff, mtd->oobsize);
 
 	/* Don't allow multipage oob writes with offset */
-	if (ops->ooboffs && (ops->ooboffs + ops->ooblen > oobmaxlen))
+	if (oob && ops->ooboffs && (ops->ooboffs + ops->ooblen > oobmaxlen))
 		return -EINVAL;
 
 	while(1) {
-- 
1.6.3.3

Re: [PATCH] mtd: nand: fix MTD_MODE_RAW writes

[Artem Bityutskiy]

On Thu, 2010-09-30 at 20:41 +0900, Jon Povey wrote:
> RAW writes were broken by 782ce79a45b3b850b108896fcf7da26754061c8f
> which introduced a check of ops->ooboffs in nand_do_write_ops().
> 
> When writing in RAW mode this is called with an ops struct on the stack
> of mtdchar.c:mtd_write() which does not initialise ops->ooboffs, so it
> is garbage and fails this test.
> 
> This test does not make sense if ops->oobbuf is NULL, which it is in the
> RAW write path, so include that in the test.
> 
> Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>
> CC: Maxim Levitsky <maximlevitsky@gmail.com>
> CC: David Woodhouse <David.Woodhouse@intel.com>
> ---
> I am not sure this is the right way to fix this, as I don't really understand
> what the test was trying to fix/prevent.
> 
> This is really just me showing my shotgun debugging solution that seems to
> work for me, and see what people have to say about it / how to fix properly.
> 
> This was an unpleasant regression for me: I use MTD_MODE_RAW to write a
> bootloader, and this got close to being a field update bricker..

I already have something similar in my l2-mtd-2.6.git / for-2.6.36 and I
was bugging David to send it upstream.

But I've pushed your patch to the same branch and added Cc: stable
<stable@kernel.org>

But I think ideally these 2 patches should be folded into one patch.
I'll leave this for David to do and possibly decide who would be the
author of the resulting patch :-)

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)