[coreboot] write/verify bug

Paul Fox pgf at foxharp.boston.ma.us
Thu Jun 11 21:17:22 CEST 2009


[responding to earlier private mail from carl-daniel, when i initially
sent the patch directly to him.]

carl-daniel wrote:
 > Hi Paul,
 > 
 > could you include coreboot at coreboot.org in your reply? That will allow
 > the other developers to chime in. Thanks.
 > 
 > On 11.06.2009 15:46, Paul Fox wrote:
 > > 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.
 > >   
 > 
 > Heh. I heard from people who expected first write, then erase. But I
 > admit your interpretation follows the principle of least surprise.

it also matches the behavior of every chip-level programmer i've ever
used.

 > 
 > 
 > > this doesn't currently work, and in fact, will zero the chip
 > > without telling you, and tell you that the operation succeeded. 
 > > much hilarity ensues.  :-)
 > >   
 > 
 > Ouch. This is a long-standing user interface bug. I'm arguing that
 > 1. flashrom should auto-verify all operations by default.
 > 2. read/erase and read/write and probably even erase/write should be
 > rejected by flashrom because it is not clear in which order the user
 > wants to run them.

well, i'll have to allow as how given the primary usage of
flashrom, i.e., re-flashing one's expensive new motherboard, that
one might want the option parsing to be as bullet- and idiot-proof
as possible.  but speaking as someone who programs flash chips for
a living (at least, it seems like that sometimes), i'd like to reduce
the length of my edit/compile/erase/write/verify/debug/edit cycle
as much as possible.  i can barely imagine using a programmer in
anything but erase/write/verify mode, and i certainly want to
invoke them all at once.

 > 
 > 
 > > because of the else-if at line 767, specifying -E along -w used
 > > to bypass the file open and read, leaving buf[] empty.  the chip
 > > would then be programmed with nulls.  worse, if -v was also used,
 > > it would verify correctly.
 > >   
 > 
 > That needs to be fixed.
 > 
 > 
 > > with this patch (which 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..
 > >   
 > 
 > Hm true. Then again, marking the buffer const except in the read-file
 > routine should be a pretty good protection against buffer writing,
 > considering that we tell gcc to error out on any warning.

not really good enough.  the code in the ft2232 programmer
currently memcpys the user's write data at least once, maybe
twice, before it reaches the programmer.  those buffers aren't
const, of course.  (it might be possible to rewrite so the
original data never moves, but i think my point remains.)
re-reading the file (or, re-copying from an initial "read-only"
buffer) should be pretty cheap.

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




More information about the coreboot mailing list