All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: Scott Wood <oss@buserror.net>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
	Hector Palacios <hector.palacios@digi.com>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	richard@nod.at, stable@vger.kernel.org
Subject: Re: [PATCH] mtd: nand: fix bug writing 1 byte less than page size
Date: Tue, 19 Jul 2016 12:56:10 -0700	[thread overview]
Message-ID: <20160719195610.GL76613@google.com> (raw)
In-Reply-To: <1468881442.25630.8.camel@buserror.net>

On Mon, Jul 18, 2016 at 05:37:22PM -0500, Scott Wood wrote:
> On Mon, 2016-07-18 at 11:04 +0200, Boris Brezillon wrote:
> > On Mon, 18 Jul 2016 10:39:18 +0200
> > Hector Palacios <hector.palacios@digi.com> wrote:
> > 
> > > 
> > > nand_do_write_ops() determines if it is writing a partial page with the
> > > formula:
> > > 	part_pagewr = (column || writelen < (mtd->writesize - 1))
> > > 
> > > When 'writelen' is exactly 1 byte less than the NAND page size the formula
> > > equates to zero, so the code doesn't process it as a partial write,
> > > although it should.
> > > As a consequence the function remains in the while(1) loop with 'writelen'
> > > becoming 0xffffffff and iterating endlessly.
> > > 
> > > The bug may not be easy to reproduce in Linux since user space tools
> > > usually force the padding or round-up the write size to a page-size
> > > multiple.
> > > This was discovered in U-Boot where the issue can be reproduced by
> > > writing any size that is 1 byte less than a page-size multiple.
> > > For example, on a NAND with 2K page (0x800):
> > > 	=> nand erase.part <partition>
> > > 	=> nand write $loadaddr <partition> 7ff  
> > > 
> > > Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> > Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > 
> > Brian, can you take this patch in your tree.
> > 
> > As usual, I'm unsure whether we should Cc stable or not, but we
> > should at least add
> > 
> > Fixes: 66507c7bc8895 ("mtd: nand: Add support to use nand_base poi databuf
> > as bounce buffer")
> 
> That commit just moved the bad test; it was introduced in 29072b96078ffde3
> ("[MTD] NAND: add subpage write support").

Indeed. I've update the Fixes tag and added an additional comment in the
commit message.

Thanks,
Brian

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <computersforpeace@gmail.com>
To: Scott Wood <oss@buserror.net>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
	Hector Palacios <hector.palacios@digi.com>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	richard@nod.at, stable@vger.kernel.org
Subject: Re: [PATCH] mtd: nand: fix bug writing 1 byte less than page size
Date: Tue, 19 Jul 2016 12:56:10 -0700	[thread overview]
Message-ID: <20160719195610.GL76613@google.com> (raw)
In-Reply-To: <1468881442.25630.8.camel@buserror.net>

On Mon, Jul 18, 2016 at 05:37:22PM -0500, Scott Wood wrote:
> On Mon, 2016-07-18 at 11:04 +0200, Boris Brezillon wrote:
> > On Mon, 18 Jul 2016 10:39:18 +0200
> > Hector Palacios <hector.palacios@digi.com> wrote:
> > 
> > > 
> > > nand_do_write_ops() determines if it is writing a partial page with the
> > > formula:
> > > 	part_pagewr = (column || writelen < (mtd->writesize - 1))
> > > 
> > > When 'writelen' is exactly 1 byte less than the NAND page size the formula
> > > equates to zero, so the code doesn't process it as a partial write,
> > > although it should.
> > > As a consequence the function remains in the while(1) loop with 'writelen'
> > > becoming 0xffffffff and iterating endlessly.
> > > 
> > > The bug may not be easy to reproduce in Linux since user space tools
> > > usually force the padding or round-up the write size to a page-size
> > > multiple.
> > > This was discovered in U-Boot where the issue can be reproduced by
> > > writing any size that is 1 byte less than a page-size multiple.
> > > For example, on a NAND with 2K page (0x800):
> > > 	=> nand erase.part <partition>
> > > 	=> nand write $loadaddr <partition> 7ff��
> > > 
> > > Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> > Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > 
> > Brian, can you take this patch in your tree.
> > 
> > As usual, I'm unsure whether we should Cc stable or not, but we
> > should at least add
> > 
> > Fixes: 66507c7bc8895 ("mtd: nand: Add support to use nand_base poi databuf
> > as bounce buffer")
> 
> That commit just moved the bad test; it was introduced in�29072b96078ffde3
> ("[MTD] NAND: add subpage write support").

Indeed. I've update the Fixes tag and added an additional comment in the
commit message.

Thanks,
Brian

  reply	other threads:[~2016-07-19 19:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-18  8:39 [PATCH] mtd: nand: fix bug writing 1 byte less than page size Hector Palacios
2016-07-18  9:04 ` Boris Brezillon
2016-07-18 17:18   ` Brian Norris
2016-07-18 22:37   ` Scott Wood
2016-07-19 19:56     ` Brian Norris [this message]
2016-07-19 19:56       ` Brian Norris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160719195610.GL76613@google.com \
    --to=computersforpeace@gmail.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=hector.palacios@digi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=oss@buserror.net \
    --cc=richard@nod.at \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.