[coreboot] write/verify bug

Paul Fox pgf at foxharp.boston.ma.us
Thu Jun 11 20:39:11 CEST 2009


hi --

i'm working on a spi programmer based on the ft4232h
USB-to-{serial/spi/jtag} converter from FTDI.  i'll be sending an
initial implementation to the list later this afternoon.

i scratched my head for a while yesterday when my programmer was
suddenly bulk-clearing the flash part, when it had been
programming successfully just hours earlier.  it turned out (to
my relief :-) that it was an existing bug in flashrom.

given the way the options are parsed, i expected to be able to
run "flashrom -Ewv myfile.img" and have the chip erased, written,
and verified in one pass. 

this doesn't currently work, and in fact, will zero the chip
without telling you, and then tell you that the operation
succeeded.  much hilarity ensues.  :-)

because of the else-if at line 767, specifying -E along -w
bypasses the file open and read, leaving buf[] empty.  the chip
will then be programmed with nulls.  worse, if -v is also used,
it will verify correctly.

with this patch (which also makes the usage message match the
actual arg parsing) the above command invocation now works
correctly.

(for real robustness, the verify step should probably re-read the
file, in case one of the many worker routines accidentally writes
to the buffer.  but that's a different patch..)

paul
=---------------------
 paul fox, pgf at foxharp.boston.ma.us (arlington, ma, where it's 53.4 degrees)


Signed-off-by: Paul Fox <pgf at foxharp.boston.ma.us>

Index: flashrom.c
===================================================================
--- flashrom.c	(revision 581)
+++ flashrom.c	(working copy)
@@ -471,7 +471,7 @@
 
 void usage(const char *name)
 {
-	printf("usage: %s [-EVfLhR] [-r file] [-w file] [-v file] [-c chipname] [-s addr]\n"
+	printf("usage: %s [-EVfLhR] [-r] [-w] [-v] [-c chipname] [-s addr]\n"
 	       "       [-e addr] [-m [vendor:]part] [-l file] [-i image] [-p programmer] [file]\n\n",
 	       name);
 
@@ -498,6 +498,7 @@
 	     "                                     (internal, dummy, nic3com, satasii, it87spi)\n"
 	     "   -h | --help:                      print this help text\n"
 	     "   -R | --version:                   print the version (release)\n"
+	     "\nThe specified file is used by -r, -w, and -v."
 	     "\nIf no file is specified, then all that happens"
 	     " is that flash info is dumped.\n\n");
 	exit(1);
@@ -764,7 +765,8 @@
 	if (erase_it) {
 		if (erase_flash(flash))
 			return 1;
-	} else if (read_it) {
+	} 
+	if (read_it) {
 		if (read_flash(flash, filename, exclude_start_position, exclude_end_position))
 			return 1;
 	} else {






More information about the coreboot mailing list