Bug#525818: tar: closes file stream before real EOF

[Carl Worth]

On Mon, 27 Apr 2009 12:02:04 +0200 Uwe Kleine-König 
 wrote:
> This is a regression from fixing http://bugs.debian.org/235820 plus (I
> think) the upstream change described in http://bugs.debian.org/522858.
> In tar-1.20 this doesn't show up.
> 
> Reverting the diff for src/list.c makes this problem go away.  It
> doesn't fix #522858, though.

Fortunately, #522858 has since been fixed independently by a change in
dpkg, so we shouldn't have to worry about that at all. Meanwhile, the
proposal to revert the diff for src/list.c would undo the fix for
#235820 (which was to simply avoid printing a confusing warning when tar
was actually successful).

I think we can resolve the current bug without undoing the fix for
#235820 by reading the header, but remaining silent about any lone zero
block.

Please examine the patch below and let me know if you find it
acceptable. (The patch moves the #if 0 down to exclude only the printing
of the warning, without otherwise changing tar's behavior. The diff
doesn't make that as obvious as one might like.)

-Carl

commit 37efd16782afb35738fcd67c85f134fef2cc409f
Author: Carl Worth 
Date:   Wed Jul 29 16:10:01 2009 -0700

Don't close file stream before EOF, closes #525818

To fix bug #235820 src/list.c was modified to avoid printing
a confusing warning message about a lone zero block. However,
the change also caused the input stream to be closed before
reading EOF which could cause a SIGPIPE to be sent to the
writing process.

In this change, the previous fix is made more narrow to exclude
only the printing of the warning message, but without the side
effect that was causing the SIGPIPE.

diff --git a/src/list.c b/src/list.c
index c650172..126a3c1 100644
--- a/src/list.c
+++ b/src/list.c
@@ -136,7 +136,11 @@ read_and (void (*do_something) (void))
 
  if (!ignore_zeros_option)
{
+ char buf[UINTMAX_STRSIZE_BOUND];
 
+ status = read_header (false);
+ if (status == HEADER_ZERO_BLOCK)
+   break;
  /* 
   * According to POSIX tar specs, this is wrong, but on the web
   * there are some tar specs that can trigger this, and some tar
@@ -144,11 +148,6 @@ read_and (void (*do_something) (void))
   * let's not be pedantic about issuing the warning.
   */
 #if 0 
- char buf[UINTMAX_STRSIZE_BOUND];
-
- status = read_header (false);
- if (status == HEADER_ZERO_BLOCK)
-   break;
  WARN ((0, 0, _("A lone zero block at %s"),
 STRINGIFY_BIGINT (current_block_ordinal (), buf)));
 #endif



signature.asc
Description: This is a digitally signed message part

Bug#525818: tar: closes file stream before real EOF

[Uwe Kleine-König]

Package: tar
Version: 1.22-1
Severity: normal
Tags: patch

Hello,

when listing the contents of module-init-tools-3.5.tar.bz2[1] as follows
(redirecting to /dev/null to keep the report nice and small):

$ bzcat module-init-tools-3.5.tar.bz2 | tar t > /dev/null
tar: Record size = 8 blocks

tar's stdin is closed before the stream ends:

$ echo ${pipestat...@]}
141 0

(I think PIPESTATUS is a bashism.)  141 = 128 + SIGPIPE

This upsets our build system, which is a bit picky about such errors.

Some debugging details:

- module-init-tools-3.5.tar has a length of 0x16d000
- the last 0x2b43 bytes are zeros only
- bzcat is killed by SIGPIPE at offset 0x16b000
- stracing bzcat ends with:

write(1, 
"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) 
= -1 EPIPE (Broken pipe)
--- SIGPIPE (Broken pipe) @ 0 (0) ---
+++ killed by SIGPIPE +++
- adding -i to the options of tar makes the problem go away

This is a regression from fixing http://bugs.debian.org/235820 plus (I
think) the upstream change described in http://bugs.debian.org/522858.
In tar-1.20 this doesn't show up.

Reverting the diff for src/list.c makes this problem go away.  It
doesn't fix #522858, though.

A suggested patch is attached.

Best regards
Uwe

PS: I wonder about the output to stderr above
("tar: Record size = 8 blocks").  Is this intended?

[1] available at
http://www.kernel.org/pub/linux/utils/kernel/module-init-tools/module-init-tools-3.5.tar.bz2

-- System Information:
Debian Release: 5.0.1
  APT prefers proposed-updates
  APT policy: (900, 'proposed-updates'), (900, 'stable'), (600, 
'testing-proposed-updates'), (600, 'testing'), (500, 
'oldstable-proposed-updates'), (500, 'oldstable'), (200, 'unstable'), (2, 
'experimental')
Architecture: i386 (i686)

Kernel: Linux 2.6.29-1-686 (SMP w/1 CPU core)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages tar depends on:
ii  libc6 2.9-7  GNU C Library: Shared libraries

tar recommends no packages.

Versions of packages tar suggests:
ii  bzip2 1.0.5-1high-quality block-sorting file co
ii  ncompress 4.2.4.2-1  Original Lempel-Ziv compress/uncom

-- no debconf information
diff --git a/src/list.c b/src/list.c
index c650172..64fd929 100644
--- a/src/list.c
+++ b/src/list.c
@@ -143,7 +143,7 @@ read_and (void (*do_something) (void))
 	   * implementations create tars according to that spec.  For now,
 	   * let's not be pedantic about issuing the warning.
 	   */
-#if 0	   
+#if 0
 	  char buf[UINTMAX_STRSIZE_BOUND];
 
 	  status = read_header (false);
@@ -151,6 +151,8 @@ read_and (void (*do_something) (void))
 		break;
 	  WARN ((0, 0, _("A lone zero block at %s"),
 		 STRINGIFY_BIGINT (current_block_ordinal (), buf)));
+#else
+	  read_header (false);
 #endif
 	  break;
 	}