diff mbox

[08/10] mtd-utils: nandwrite: prevent 32-bit overflow

Message ID 1288772847-8120-8-git-send-email-computersforpeace@gmail.com
State New, archived
Headers show

Commit Message

Brian Norris Nov. 3, 2010, 8:27 a.m. UTC
For large block- and page-sizes, the multiplication of ebsize_aligned
and pagelen can overflow a 32-bit integer.  This overflow can be
prevented by a simple change in order of operations (i.e., do division
first).

Since ebsize_aligned is always a multiple of mtd.min_io_size, this
produces no change in results.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Mike Frysinger Nov. 9, 2010, 9:48 a.m. UTC | #1
On Wed, Nov 3, 2010 at 04:27, Brian Norris wrote:
> For large block- and page-sizes, the multiplication of ebsize_aligned
> and pagelen can overflow a 32-bit integer.  This overflow can be
> prevented by a simple change in order of operations (i.e., do division
> first).
>
> -       filebuf_max = pagelen * ebsize_aligned / mtd.min_io_size;
> +       filebuf_max = ebsize_aligned / mtd.min_io_size * pagelen;

i'd insert a comment noting that order of operations matters to avoid
32bit overflow so someone doesnt switch it back by accident in the
future
-mike
Artem Bityutskiy Nov. 9, 2010, 12:20 p.m. UTC | #2
On Wed, 2010-11-03 at 01:27 -0700, Brian Norris wrote:
> For large block- and page-sizes, the multiplication of ebsize_aligned
> and pagelen can overflow a 32-bit integer.  This overflow can be
> prevented by a simple change in order of operations (i.e., do division
> first).
> 
> Since ebsize_aligned is always a multiple of mtd.min_io_size, this
> produces no change in results.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Brian, increasingly have less and less time for mtd, but I remember
about your patches - I'll pick them as soon as I have some time. You can
meanwhile address Mikes comments.
diff mbox

Patch

diff --git a/nandwrite.c b/nandwrite.c
index 8ec5afe..364acdf 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -441,7 +441,7 @@  int main(int argc, char * const argv[])
 	}
 
 	// Allocate a buffer big enough to contain all the data (OOB included) for one eraseblock
-	filebuf_max = pagelen * ebsize_aligned / mtd.min_io_size;
+	filebuf_max = ebsize_aligned / mtd.min_io_size * pagelen;
 	filebuf = xmalloc(filebuf_max);
 	erase_buffer(filebuf, filebuf_max);