On Tue, 13 May 2014 12:51:18 +0400 Shire <[email protected]> wrote:
> Thanks to The Raven and Stefan Tauner for bug report. > I fix patch for correct write buffer calculation. > Correct version of patch is attached. Yes, that one looks correct. I came to the same conclusion like you: 1) the loop condition is wrong and 2) the offset used within the read buffer should start at 0 and not at the chip offset. I really wonder how this could have ever worked in my tests :) My patch to solve the issue is a bit different though and I think it is slightly better readable. I'd be glad if you could test/review it. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
>From 03ef3045da5c7fb5d40c9415141241978a70effe Mon Sep 17 00:00:00 2001 From: Stefan Tauner <[email protected]> Date: Fri, 16 May 2014 14:17:54 +0200 Subject: [PATCH] AT45DB: fix length calculations in read functions. This fixes segfaults on reads (implicit reads on writes too), ouch. Thanks to The Raven for reporting the problem and to Alexander Irenkov for providing a workable fix for it additionally. There were actually two problems: 1) The loop conditions were bogus which could lead to read errors (e.g. on implicit erase verifications). 2) The offset used within the read buffers provided to spi_nbyte_read() and memcpy() were not starting at 0 but the offset of the block within the flash chip (which has nothing to do with read buffer in most cases). This patch works similarly to Alexander's but is intended to be more readable. Signed-off-by: Stefan Tauner <[email protected]> --- at45db.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/at45db.c b/at45db.c index b1a81ef..41fc497 100644 --- a/at45db.c +++ b/at45db.c @@ -254,14 +254,16 @@ int spi_read_at45db(struct flashctx *flash, uint8_t *buf, unsigned int addr, uns * chunks can cross page boundaries. */ const unsigned int max_data_read = flash->pgm->spi.max_data_read; const unsigned int max_chunk = (max_data_read > 0) ? max_data_read : page_size; - while (addr < len) { + while (len > 0) { unsigned int chunk = min(max_chunk, len); - int ret = spi_nbyte_read(flash, at45db_convert_addr(addr, page_size), buf + addr, chunk); + int ret = spi_nbyte_read(flash, at45db_convert_addr(addr, page_size), buf, chunk); if (ret) { msg_cerr("%s: error sending read command!\n", __func__); return ret; } addr += chunk; + buf += chunk; + len -= chunk; } return 0; @@ -283,7 +285,7 @@ int spi_read_at45db_e8(struct flashctx *flash, uint8_t *buf, unsigned int addr, * chunks can cross page boundaries. */ const unsigned int max_data_read = flash->pgm->spi.max_data_read; const unsigned int max_chunk = (max_data_read > 0) ? max_data_read : page_size; - while (addr < len) { + while (len > 0) { const unsigned int addr_at45 = at45db_convert_addr(addr, page_size); const unsigned char cmd[] = { AT45DB_READ_ARRAY, @@ -300,8 +302,10 @@ int spi_read_at45db_e8(struct flashctx *flash, uint8_t *buf, unsigned int addr, return ret; } /* Copy result without dummy bytes into buf and advance address counter respectively. */ - memcpy(buf + addr, tmp + 4, chunk - 4); + memcpy(buf, tmp + 4, chunk - 4); addr += chunk - 4; + buf += chunk - 4; + len -= chunk - 4; } return 0; } -- Kind regards, Stefan Tauner
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
