[PATCH 1/2] m25p80: add additional sanity checks for erasure

[PATCH 1/2] m25p80: add additional sanity checks for erasure

[Paul Fertser]

This guards for the cases where the initial offset or byte count is
not aligned with regard to erase block size, thus making it impossible
for erase to do any harm to the nearby sectors.

Signed-off-by: Paul Fertser <fercerpav@gmail.com>
---
 drivers/nor/m25p80.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/nor/m25p80.c b/drivers/nor/m25p80.c
index e6fe75e..6f650a5 100644
--- a/drivers/nor/m25p80.c
+++ b/drivers/nor/m25p80.c
@@ -206,7 +206,9 @@ static ssize_t m25p80_erase(struct cdev *cdev, size_t count, unsigned long offse
 		__func__, "at", (long long)offset, (long long)count);
 
 	/* sanity checks */
-	if (offset + count > flash->size)
+	if ((offset + count > flash->size) ||
+	    (offset % flash->erasesize) ||
+	    ((offset+count) % flash->erasesize))
 		return -EINVAL;
 
 	addr = offset;
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] m25p80: use proper erasesize for SECT_4K devices

[Paul Fertser]

This fixes a bug that causes only the first 4K out of every 64K to be
erased on SECT_4K devices.

Signed-off-by: Paul Fertser <fercerpav@gmail.com>
---
 drivers/nor/m25p80.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/nor/m25p80.c b/drivers/nor/m25p80.c
index 6f650a5..31193cf 100644
--- a/drivers/nor/m25p80.c
+++ b/drivers/nor/m25p80.c
@@ -760,7 +760,10 @@ static int m25p_probe(struct device_d *dev)
 	flash->name = (char *)id->name;
 	flash->info = info;
 	flash->size = info->sector_size * info->n_sectors;
-	flash->erasesize = info->sector_size;
+	if (info->flags & SECT_4K)
+		flash->erasesize = 4096;
+	else
+		flash->erasesize = info->sector_size;
 	flash->cdev.size = info->sector_size * info->n_sectors;
 	flash->cdev.dev = dev;
 	flash->cdev.ops = &m25p80_ops;
-- 
1.7.2.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] m25p80: add additional sanity checks for erasure

[Sascha Hauer]

On Thu, Sep 15, 2011 at 03:27:36PM +0400, Paul Fertser wrote:
> This guards for the cases where the initial offset or byte count is
> not aligned with regard to erase block size, thus making it impossible
> for erase to do any harm to the nearby sectors.

I'm unsure about this one. The other flash drivers allow to erase areas
which are not eraseblock aligned. Maybe we should instead add a
cdev->erasesize field. Then we can add this check in the generic code
and fix this for all drivers. (I know this won't work for some nor
flashes which have eraseblocks of different size, but this special
case could still be checked in the driver)

Sascha


> 
> Signed-off-by: Paul Fertser <fercerpav@gmail.com>
> ---
>  drivers/nor/m25p80.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/nor/m25p80.c b/drivers/nor/m25p80.c
> index e6fe75e..6f650a5 100644
> --- a/drivers/nor/m25p80.c
> +++ b/drivers/nor/m25p80.c
> @@ -206,7 +206,9 @@ static ssize_t m25p80_erase(struct cdev *cdev, size_t count, unsigned long offse
>  		__func__, "at", (long long)offset, (long long)count);
>  
>  	/* sanity checks */
> -	if (offset + count > flash->size)
> +	if ((offset + count > flash->size) ||
> +	    (offset % flash->erasesize) ||
> +	    ((offset+count) % flash->erasesize))
>  		return -EINVAL;
>  
>  	addr = offset;
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] m25p80: add additional sanity checks for erasure

[Paul Fertser]

On Tue, Sep 20, 2011 at 09:31:21PM +0200, Sascha Hauer wrote:
> On Thu, Sep 15, 2011 at 03:27:36PM +0400, Paul Fertser wrote:
> > This guards for the cases where the initial offset or byte count is
> > not aligned with regard to erase block size, thus making it impossible
> > for erase to do any harm to the nearby sectors.
> 
> I'm unsure about this one. The other flash drivers allow to erase areas
> which are not eraseblock aligned. Maybe we should instead add a
> cdev->erasesize field. Then we can add this check in the generic code
> and fix this for all drivers.

I guess i was too surprised by seeing an endless loop when i tried to
erase an area of the wrong size (the counter underflowed) to think
about it in a less narrow-minded way, sorry, you're right of course.

Talking about generalisation, shouldn't the m25p80 driver be hooked
into the mtd subsystem to allow running ubi on top of it? By the
cursory look it seems to be doable without much effort.

Another question: should i repost this early (questionable) series you
seem to have missed[1]?

[1] http://lists.infradead.org/pipermail/barebox/2011-August/004483.html
-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] m25p80: add additional sanity checks for erasure

[Sascha Hauer]

Hi Paul,

Sorry for the delay.

On Wed, Sep 21, 2011 at 12:29:01AM +0400, Paul Fertser wrote:
> On Tue, Sep 20, 2011 at 09:31:21PM +0200, Sascha Hauer wrote:
> > On Thu, Sep 15, 2011 at 03:27:36PM +0400, Paul Fertser wrote:
> > > This guards for the cases where the initial offset or byte count is
> > > not aligned with regard to erase block size, thus making it impossible
> > > for erase to do any harm to the nearby sectors.
> > 
> > I'm unsure about this one. The other flash drivers allow to erase areas
> > which are not eraseblock aligned. Maybe we should instead add a
> > cdev->erasesize field. Then we can add this check in the generic code
> > and fix this for all drivers.
> 
> I guess i was too surprised by seeing an endless loop when i tried to
> erase an area of the wrong size (the counter underflowed) to think
> about it in a less narrow-minded way, sorry, you're right of course.
> 
> Talking about generalisation, shouldn't the m25p80 driver be hooked
> into the mtd subsystem to allow running ubi on top of it? By the
> cursory look it seems to be doable without much effort.

Yes, it probably should. The cfi driver also is not a mtd driver but
has some mtd glue code. Maybe the same approach (even better the same
code) can be used here.

> 
> Another question: should i repost this early (questionable) series you
> seem to have missed[1]?
> 
> [1] http://lists.infradead.org/pipermail/barebox/2011-August/004483.html

Not necessary, I just applied it.

Thanks
 Sascha



-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox