[3/4] mtd: mxc_nand: fix truncate of unaligned oob copying
diff mbox

Message ID 1f8e85fd8e75170e1005473c075942cb3d4479fe.1430034929.git.baruch@tkos.co.il
State New, archived
Headers show

Commit Message

Baruch Siach April 26, 2015, 8:16 a.m. UTC
Copy to/from oob io area might not be aligned to 4 bytes. When 8 bit ECC is
used, the buffer size is 26. Make sure we pass 4 bytes aligned buffer size to
memcpy32_{to,from}io to avoid truncating the buffer.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/mtd/nand/mxc_nand.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Uwe Kleine-König April 26, 2015, 7:52 p.m. UTC | #1
Hello Baruch,

On Sun, Apr 26, 2015 at 11:16:50AM +0300, Baruch Siach wrote:
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index c650f0950b20..c05f5e8fef17 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -840,22 +840,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
>  		for (i = 0; i < num_chunks - 1; i++)
>  			memcpy32_fromio(d + i * oob_chunk_size,
>  					s + i * sparebuf_size,
> -					oob_chunk_size);
> +					ALIGN(oob_chunk_size, 4));
If oob_chunk_size isn't 32-bit-aligned, d + i * oob_chunk_size isn't
either for uneven i. That's not nice. I suggest to use memcpy16_fromio.

Best regards
Uwe
Baruch Siach April 27, 2015, 4:46 a.m. UTC | #2
Hi Uwe,

On Sun, Apr 26, 2015 at 09:52:11PM +0200, Uwe Kleine-König wrote:
> On Sun, Apr 26, 2015 at 11:16:50AM +0300, Baruch Siach wrote:
> > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> > index c650f0950b20..c05f5e8fef17 100644
> > --- a/drivers/mtd/nand/mxc_nand.c
> > +++ b/drivers/mtd/nand/mxc_nand.c
> > @@ -840,22 +840,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
> >  		for (i = 0; i < num_chunks - 1; i++)
> >  			memcpy32_fromio(d + i * oob_chunk_size,
> >  					s + i * sparebuf_size,
> > -					oob_chunk_size);
> > +					ALIGN(oob_chunk_size, 4));
> If oob_chunk_size isn't 32-bit-aligned, d + i * oob_chunk_size isn't
> either for uneven i. That's not nice. I suggest to use memcpy16_fromio.

memcpy32_fromio() was introduced by Sascha in 096bcc231fd2 (mtd: mxc_nand: use 
32bit copy functions, 2012-05-29), replacing memcpy_fromio() to force 32bit io 
access. Are you sure the hypothetical memcpy16_fromio() would work?

baruch
Uwe Kleine-König April 27, 2015, 6:56 a.m. UTC | #3
Hello Baruch,

On Mon, Apr 27, 2015 at 07:46:18AM +0300, Baruch Siach wrote:
> On Sun, Apr 26, 2015 at 09:52:11PM +0200, Uwe Kleine-König wrote:
> > On Sun, Apr 26, 2015 at 11:16:50AM +0300, Baruch Siach wrote:
> > > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> > > index c650f0950b20..c05f5e8fef17 100644
> > > --- a/drivers/mtd/nand/mxc_nand.c
> > > +++ b/drivers/mtd/nand/mxc_nand.c
> > > @@ -840,22 +840,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
> > >  		for (i = 0; i < num_chunks - 1; i++)
> > >  			memcpy32_fromio(d + i * oob_chunk_size,
> > >  					s + i * sparebuf_size,
> > > -					oob_chunk_size);
> > > +					ALIGN(oob_chunk_size, 4));
> > If oob_chunk_size isn't 32-bit-aligned, d + i * oob_chunk_size isn't
> > either for uneven i. That's not nice. I suggest to use memcpy16_fromio.
> 
> memcpy32_fromio() was introduced by Sascha in 096bcc231fd2 (mtd: mxc_nand: use 
> 32bit copy functions, 2012-05-29), replacing memcpy_fromio() to force 32bit io 
> access. Are you sure the hypothetical memcpy16_fromio() would work?
according to the reference manual you can use 16-bit or 32-bit accesses.
And giving that in some cases the amount of valid data (in bytes) is ? 2
(mod 4) 16-bit access seem to be the obvious thing. Also Sascha only
wrote about byte accesses being problematic in 096bcc231fd2. So I'm
expecting that 16-bit operators should be fine.

Best regards
Uwe

Patch
diff mbox

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index c650f0950b20..c05f5e8fef17 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -840,22 +840,22 @@  static void copy_spare(struct mtd_info *mtd, bool bfrom)
 		for (i = 0; i < num_chunks - 1; i++)
 			memcpy32_fromio(d + i * oob_chunk_size,
 					s + i * sparebuf_size,
-					oob_chunk_size);
+					ALIGN(oob_chunk_size, 4));
 
 		/* the last chunk */
 		memcpy32_fromio(d + i * oob_chunk_size,
 				s + i * sparebuf_size,
-				used_oobsize - i * oob_chunk_size);
+				ALIGN(used_oobsize - i * oob_chunk_size, 4));
 	} else {
 		for (i = 0; i < num_chunks - 1; i++)
 			memcpy32_toio(&s[i * sparebuf_size],
 				      &d[i * oob_chunk_size],
-				      oob_chunk_size);
+				      ALIGN(oob_chunk_size, 4));
 
 		/* the last chunk */
 		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
 			      &d[i * oob_chunk_size],
-			      used_oobsize - i * oob_chunk_size);
+			      ALIGN(used_oobsize - i * oob_chunk_size, 4));
 	}
 }