[U-Boot] [PATCH] sf: Set current flash bank to 0 in clean_bar()

[Marek Vasut]

On 05/24/2018 09:58 PM, Marek Vasut wrote:
> The clean_bar() function resets the SPI NOR BAR register to 0, but
> does not set the flash->curr_bar to 0 , therefore those two can get
> out of sync, which could ultimatelly result in corrupted flash content.
> 
> The simplest test case is this:
> 
>   => mw 0x10000000 0x1234abcd 0x4000
>   => sf probe
>   => sf erase 0x1000000 0x10000
>   => sf write 0x10000000 0x1000000 0x10000
> 
>   => sf probe ; sf read 0x12000000 0 0x10000 ; md 0x12000000
> 
> That is, erase a sector above the 16 MiB boundary and write it with
> random pre-configured data. What will actually happen without this
> patch is the sector will be erased, but the data will be written to
> BAR 0 offset 0x0 in the flash.
> 
> This is because the erase command will call write_bar()+clean_bar(),
> which will leave flash->bank_curr = 1 while the hardware BAR registers
> will be set to 0 through clean_bar(). The subsequent write will also
> trigger write_bar()+clean_bar(), but write_bar checks if the target
> bank == flash->bank_curr and if so, does NOT reconfigure the BAR in
> the SPI NOR. Since flash->bank_curr is still 1 and out of sync with
> the HW, the condition matches, BAR programming is skipped and write
> ends up at address 0x0, thus corrupting flash content.
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Jagan Teki <jagan at openedev.com>
> Cc: Tom Rini <trini at konsulko.com>

This is a critical fix and it's been broken for multiple releases, so bump.

> ---
>  drivers/mtd/spi/spi_flash.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 2911729b28..1b61095859 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -128,6 +128,7 @@ static int clean_bar(struct spi_flash *flash)
>  	if (flash->bank_curr == 0)
>  		return 0;
>  	cmd = flash->bank_write_cmd;
> +	flash->bank_curr = 0;
>  
>  	return spi_flash_write_common(flash, &cmd, 1, &bank_sel, 1);
>  }
> 


-- 
Best regards,
Marek Vasut