[Bug 501] New: rx does not strip EOF bytes at the end of the last block, leading to incorrect file reception

bugzilla at busybox.net bugzilla at busybox.net
Wed Jul 29 19:53:00 UTC 2009


https://bugs.busybox.net/show_bug.cgi?id=501

           Summary: rx does not strip EOF bytes at the end of the last
                    block, leading to incorrect file reception
           Product: Busybox
           Version: 1.14.x
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: major
          Priority: P5
         Component: Standard Compliance
        AssignedTo: unassigned at busybox.net
        ReportedBy: thomas.petazzoni at free-electrons.com
                CC: busybox-cvs at busybox.net
   Estimated Hours: 0.0


The xmodem specification states that the last block of data might be padded
with 0x1A bytes, up to the 128 bytes block size. And indeed, when sending data
using Minicom Xmodem file upload feature, if the file size is not a multiple of
128 bytes, 0x1A are sent.

The issue is that the 'rx' applet doesn't strip those bytes. So the received
file on the target is in fact different from the one sent from the host. Its
size is rounded up to the next 128 bytes boundary, and this space is padded
with 0x1A bytes. Obvisouly the MD5 of the received and sent files do not match.

U-Boot does the following to "detect" such a case, but I'm not sure it's 100%
correct :

/* 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--;
    }
}

So I've cooked a patch against the 'rx' applet:

Index: busybox-1.14.2/miscutils/rx.c
===================================================================
--- busybox-1.14.2.orig/miscutils/rx.c
+++ busybox-1.14.2/miscutils/rx.c
@@ -26,6 +26,7 @@
 #define ACK 0x06
 #define NAK 0x15
 #define BS  0x08
+#define PAD 0x1A

 /*
 Cf:
@@ -173,6 +174,13 @@
                        goto error;
                }

+               for (i = blockLength - 1; i >= 0; i--) {
+                       if (blockBuf[i] == PAD)
+                               blockLength--;
+                       else
+                               break;
+               }
+
                wantBlockNo++;
                length += blockLength;

But this patch is obvisouly wrong, since it removes all the 0x1A at the end of
each block. I don't know how to detect:
 1. That we are handling the last block
 2. More importantly, that the 0x1A at the end of the block are actually real
data or padding

Unfortunately, due to this bug, the 'rx' applet is currently not usable.


-- 
Configure bugmail: https://bugs.busybox.net/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


More information about the busybox-cvs mailing list