[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[Martin Krause]

The function find_sector() does not take into account if the flash bank
has changed since the last call. This could lead to illegal accesses inside
and beyond the flash_info_t info strcture. For example if the current
flash bank has less sectors than the last used flash bank.

This patch adds two cheks. One that insures, that the current sector does
not exceed the allowed maximum (which is always a good idea). And one that
checks if the current access is to the same flash bank as the last access.
If not, the search loop will start with sector 0.

Signed-off-by: Martin Krause <martin.krause@tqs.de>
---
 drivers/mtd/cfi_flash.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index dd394a8..0909fe7 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -744,8 +744,12 @@ static void flash_add_byte (flash_info_t * info, cfiword_t * cword, uchar c)
 static flash_sect_t find_sector (flash_info_t * info, ulong addr)
 {
 	static flash_sect_t saved_sector = 0; /* previously found sector */
+	static flash_info_t *saved_info = 0; /* previously used flash bank */
 	flash_sect_t sector = saved_sector;
 
+	if ((info != saved_info) || (sector >= info->sector_count))
+		sector = 0;
+
 	while ((info->start[sector] < addr)
 			&& (sector < info->sector_count - 1))
 		sector++;
@@ -757,6 +761,7 @@ static flash_sect_t find_sector (flash_info_t * info, ulong addr)
 		sector--;
 
 	saved_sector = sector;
+	saved_info = info;
 	return sector;
 }
 
-- 
1.5.4.3

[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[Detlev Zundel]

Martin Krause <martin.krause@tqs.de> writes:

> The function find_sector() does not take into account if the flash bank
> has changed since the last call. This could lead to illegal accesses inside
> and beyond the flash_info_t info strcture. For example if the current
> flash bank has less sectors than the last used flash bank.
>
> This patch adds two cheks. One that insures, that the current sector does
> not exceed the allowed maximum (which is always a good idea). And one that
> checks if the current access is to the same flash bank as the last access.
> If not, the search loop will start with sector 0.
>
> Signed-off-by: Martin Krause <martin.krause@tqs.de>
> ---
>  drivers/mtd/cfi_flash.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index dd394a8..0909fe7 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -744,8 +744,12 @@ static void flash_add_byte (flash_info_t * info, cfiword_t * cword, uchar c)
>  static flash_sect_t find_sector (flash_info_t * info, ulong addr)
>  {
>  	static flash_sect_t saved_sector = 0; /* previously found sector */
> +	static flash_info_t *saved_info = 0; /* previously used flash bank */
>  	flash_sect_t sector = saved_sector;
>  
> +	if ((info != saved_info) || (sector >= info->sector_count))
> +		sector = 0;
> +
>  	while ((info->start[sector] < addr)
>  			&& (sector < info->sector_count - 1))
>  		sector++;
> @@ -757,6 +761,7 @@ static flash_sect_t find_sector (flash_info_t * info, ulong addr)
>  		sector--;
>  
>  	saved_sector = sector;
> +	saved_info = info;
>  	return sector;
>  }

-- 
The structure of a system reflects the structure of the organization
that built it.
                                        -- Richard E. Fairley
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[Detlev Zundel]

Hello Stefan,

> The function find_sector() does not take into account if the flash bank
> has changed since the last call. This could lead to illegal accesses inside
> and beyond the flash_info_t info strcture. For example if the current
> flash bank has less sectors than the last used flash bank.
>
> This patch adds two cheks. One that insures, that the current sector does
> not exceed the allowed maximum (which is always a good idea). And one that
> checks if the current access is to the same flash bank as the last access.
> If not, the search loop will start with sector 0.
>
> Signed-off-by: Martin Krause <martin.krause@tqs.de>

Can you please comment on Martins fix?  Thanks!
  Detlev

-- 
There are three kinds of people in the world; those who can count and
those who can't.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[Stefan Roese]

Hi Detlev,

On Monday 28 March 2011 17:55:03 Detlev Zundel wrote:
> > The function find_sector() does not take into account if the flash bank
> > has changed since the last call. This could lead to illegal accesses
> > inside and beyond the flash_info_t info strcture. For example if the
> > current flash bank has less sectors than the last used flash bank.
> > 
> > This patch adds two cheks. One that insures, that the current sector does
> > not exceed the allowed maximum (which is always a good idea). And one
> > that checks if the current access is to the same flash bank as the last
> > access. If not, the search loop will start with sector 0.
> > 
> > Signed-off-by: Martin Krause <martin.krause@tqs.de>
> 
> Can you please comment on Martins fix?  Thanks!

I already did and asked for a non line-wrapped patch version:

http://lists.denx.de/pipermail/u-boot/2011-March/088950.html

Still no answer though. Martin, how is your schedule here? Will you find the 
time to send an updated patch shortly?

Thanks.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[Detlev Zundel]

Hi Stefan,

> Hi Detlev,
>
> On Monday 28 March 2011 17:55:03 Detlev Zundel wrote:
>> > The function find_sector() does not take into account if the flash bank
>> > has changed since the last call. This could lead to illegal accesses
>> > inside and beyond the flash_info_t info strcture. For example if the
>> > current flash bank has less sectors than the last used flash bank.
>> > 
>> > This patch adds two cheks. One that insures, that the current sector does
>> > not exceed the allowed maximum (which is always a good idea). And one
>> > that checks if the current access is to the same flash bank as the last
>> > access. If not, the search loop will start with sector 0.
>> > 
>> > Signed-off-by: Martin Krause <martin.krause@tqs.de>
>> 
>> Can you please comment on Martins fix?  Thanks!
>
> I already did and asked for a non line-wrapped patch version:
>
> http://lists.denx.de/pipermail/u-boot/2011-March/088950.html
>
> Still no answer though. Martin, how is your schedule here? Will you find the 
> time to send an updated patch shortly?

Well actually I followed up to a patch version that Martin sent out
pretty much immediately and which I can apply without problems.  Can you
please try again if the patch works for you also?

Thanks!
  Detlev

-- 
I have always observed that the pretensions of all people are in
exact inverse ratio to their merits; this is one of the axioms of
morals.                            -- Joseph Lagrange
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[Stefan Roese]

Hi Detlev,

On Monday 28 March 2011 18:11:37 Detlev Zundel wrote:
> >> Can you please comment on Martins fix?  Thanks!
> > 
> > I already did and asked for a non line-wrapped patch version:
> > 
> > http://lists.denx.de/pipermail/u-boot/2011-March/088950.html
> > 
> > Still no answer though. Martin, how is your schedule here? Will you find
> > the time to send an updated patch shortly?
> 
> Well actually I followed up to a patch version that Martin sent out
> pretty much immediately and which I can apply without problems.  Can you
> please try again if the patch works for you also?

Ahh, I have missed this one somehow. Thanks for reminding.

I'll check again and will push it if no problems occur.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] [PATCH] cfi_flash: fix bug with flash banks with different sector numbers

[Stefan Roese]

On Monday 21 March 2011 18:07:56 Martin Krause wrote:
> The function find_sector() does not take into account if the flash bank
> has changed since the last call. This could lead to illegal accesses inside
> and beyond the flash_info_t info strcture. For example if the current
> flash bank has less sectors than the last used flash bank.
> 
> This patch adds two cheks. One that insures, that the current sector does
> not exceed the allowed maximum (which is always a good idea). And one that
> checks if the current access is to the same flash bank as the last access.
> If not, the search loop will start with sector 0.

Applied to u-boot-cfi-flash/master. Thanks.
 
Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de