* [ECOS] RedBoot xyModem bug
@ 2008-01-11 15:24 Rutger Hofman
2008-01-22 16:21 ` Rutger Hofman
0 siblings, 1 reply; 4+ messages in thread
From: Rutger Hofman @ 2008-01-11 15:24 UTC (permalink / raw)
To: ecos-discuss
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.
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?
Rutger Hofman
VU Amsterdam
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ECOS] RedBoot xyModem bug
2008-01-11 15:24 [ECOS] RedBoot xyModem bug Rutger Hofman
@ 2008-01-22 16:21 ` Rutger Hofman
2008-01-22 23:47 ` Gary Thomas
2008-01-24 7:26 ` Jürgen Lambrecht
0 siblings, 2 replies; 4+ messages in thread
From: Rutger Hofman @ 2008-01-22 16:21 UTC (permalink / raw)
Cc: ecos-discuss
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.
>
> 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?
>
> Rutger Hofman
> VU Amsterdam
>
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ECOS] RedBoot xyModem bug
2008-01-22 16:21 ` Rutger Hofman
@ 2008-01-22 23:47 ` Gary Thomas
2008-01-24 7:26 ` Jürgen Lambrecht
1 sibling, 0 replies; 4+ messages in thread
From: Gary Thomas @ 2008-01-22 23:47 UTC (permalink / raw)
To: Rutger Hofman; +Cc: ecos-discuss
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ECOS] RedBoot xyModem bug
2008-01-22 16:21 ` Rutger Hofman
2008-01-22 23:47 ` Gary Thomas
@ 2008-01-24 7:26 ` Jürgen Lambrecht
1 sibling, 0 replies; 4+ messages in thread
From: Jürgen Lambrecht @ 2008-01-24 7:26 UTC (permalink / raw)
To: Rutger Hofman; +Cc: ecos-discuss
>>
>> I also wondered how this bug can have lived for so long in RedBoot.
>> Or doesn't anyone use xyModem nowadays?
>>
I used xyModem in the beginning, but it only worked for 19600 baud, and
not for higher baud rates. Also others have had problems with xyModem.
So I wrote my Ethernet driver and started using tftp instead...
Regards,
Jürgen
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-01-24 7:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-11 15:24 [ECOS] RedBoot xyModem bug Rutger Hofman
2008-01-22 16:21 ` Rutger Hofman
2008-01-22 23:47 ` Gary Thomas
2008-01-24 7:26 ` Jürgen Lambrecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).