This is the mail archive of the ecos-discuss@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RedBoot xyModem bug


Rutger Hofman wrote:
Is *really* *nobody* interested in bugs in the RedBoot upload implementation?

Rutger

Rutger Hofman wrote on Jan 11, 2008:
Hello list,

I ran into a bug in RedBoot xyModem.c. It is in lines 420..434 in the current version (cvs 1.21):

    while (!xyz.at_eof && (size > 0)) {
... code for each block in the xyzModem protocols: ...

#if defined(xyzModem_zmodem) || defined(USE_YMODEM_LENGTH)
if (xyz.mode == xyzModem_xmodem || xyz.file_length == 0) {
#else
if (1) {
#endif
// Data blocks can be padded with ^Z (EOF) characters
// This code tries to detect and remove them
if ((xyz.bufp[xyz.len-1] == EOF) &&
(xyz.bufp[xyz.len-2] == EOF) &&
(xyz.bufp[xyz.len-3] == EOF)) {
while (xyz.len && (xyz.bufp[xyz.len-1] == EOF)) {
xyz.len--;
}
}
}


What this does, is inspect each uploaded block for trailing ^Z = 0x1a bytes, and strip them. Rationale: xmodem or ymodem may signal end-of-file with ^Z a.k.a. 'CP/M EOF', and may optionally pad the block with more ^Z chars. This is live behaviour: sx and sy (0.12.20) under Linux do ^Z padding in the last block. The xyModem.c code above is wrong. ^Z chars are stripped at the end of *each block* in stead of at the end of the file.

This problem surfaced when I enabled zlib in eCos. In one of its tables, there is a sequence of bytes 26 = 0x1a = ^Z (in packages/services/compress/zlib/current/src/trees.h, lines 90, 91, 112-114), which may trigger the test above. Depending on the offset in the file, these 0x1a's hit a block boundary or not, and the file is telescope-truncated or not. (I noticed that my executable crashed or not depending on code size, even depending on a number of 'nop' assembly instructions that I inserted anywhere in the code; it was a beast of a bug to pin down...)

The fix is nonobvious to me. For ymodem, if #define USE_YMODEM_LENGTH is on and a file length is specified, any trailing characters are removed elsewhere: this works correctly. However, for xmodem or ymodem without file length, any trailing ^Z at the end-of-file must still be removed. The code above is inherently unaware whether this is the last block or not: xmodem sends end-of-file control info, but that is consumed only in the *next* block read. The solution might be to maintain one more read-ahead block buffer to see if the current block is the last normal block.

Some random remarks:

As a workaround (I dare not flash the RedBoot in my device, because we bought it ready-made; I use the code base for my eCos application), I tried disabling the table in zlib. That lead to another problem, which I will report separately.

Why not test using a RAM RedBoot configuration? This should be totally safe and won't "brick" your device.

I am in the dark what must be done if a file to be xmodem'd actually contains trailing ^Z characters. There is nothing that forbids that outside CP/M or MS-DOS, I'd say.

I also wondered how this bug can have lived for so long in RedBoot. Or doesn't anyone use xyModem nowadays?

Indeed, it does get used, but this problem has only been seen/reported by you. If you want to see things happen, send patches.

--
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]