tar --use-compress-program fatal blocking bug

[Mark W. Eichin]

First the fix, then the explanation.
===================================================================
RCS file: tar-1.13.25/src/RCS/buffer.c,v
retrieving revision 1.1
diff -uwp -C 5 -r1.1 tar-1.13.25/src/buffer.c
/usr/bin/diff: conflicting specifications of output style
*** tar-1.13.25/src/buffer.c    2002/02/01 06:36:23     1.1
--- tar-1.13.25/src/buffer.c    2002/02/01 06:36:33
*************** child_open_for_compress (void)
*** 417,428 ****
           length < record_size;
           length += status, cursor += status)
        {
          size_t size = record_size - length;
  
-         if (size < BLOCKSIZE)
-           size = BLOCKSIZE;
          status = safe_read (STDIN_FILENO, cursor, size);
          if (status <= 0)
            break;
        }
  
--- 417,426 ----
===================================================================

I was investigating debian bug #126274, 

http://bugs.debian.org/cgi-bin/bugreport.cgi?archive=no&bug=126274

wherein "tar --use-compress-program=/bin/lzop" sometimes produced
corrupted archives.  Investigation showed that it was data dependent,
but that setting a small blocksize (-b 1) triggered it reliably, and
in a form that made it clear that tar was the faulty program - namely,
it called read() with arguments that caused an EFAULT.

It turns out that since the "grandchild" program (lzop in this case,
but while gzip and bzip2 [which buffer their output] don't trigger it,
other compression filters easily could) does writes of various sizes
to the "child" tar, the loop shown above could easily lead to a
partial safe_read, then continuing around the loop with a shorter
size... except that then the conditional would raise size to more than
the remainder of the buffer, causing the read to fail.

Simple confirmation of the problem comes from adding a non-optimized
reference to cursor[size-1] just before the safe_read, and linking
with Electric Fence; this causes it to blow up quite promptly.  (The
reference is necessary because the read() detects the problem and
returns EFAULT, rather than signalling.)

                        _Mark_ <address@hidden>
                        The Herd of Kittens
                        Debian Package Maintainer