bin/108990: incorrect error handling in tar(1) for files that change size

[Andrew]

>Number:         108990
>Category:       bin
>Synopsis:       incorrect error handling in tar(1) for files that change size
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Feb 09 20:40:27 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Andrew
>Release:        RELENG_6  as of 20070202
>Organization:
Critical Path, Inc
>Environment:
FreeBSD orac.supernews.net 6.2-20070202 FreeBSD 6.2-20070202 #0: Fri Feb  2 16:29:10 UTC 2007     root at supernews.net:/usr/obj/usr/src/sys/SUPERNEWS  i386

>Description:
tar aborts early with a meaningless error message if any file being archived grows in size between the initial stat() call and completing the archive write.

The error message is "tar: (Empty error message)"

The problem is caused by incorrect handling of the return value from archive_write_data called in write_file_data in write.c. This error was not visible until this commit:

http://www.freebsd.org/cgi/cvsweb.cgi/src/usr.bin/tar/write.c.diff?r1=1.47&r2=1.48

which adds an exit(1) call in the event of the incorrectly handled condition.


>How-To-Repeat:
Arrange for a file to grow continuously (assuming the use of an sh-type shell here):

$ (while true; do echo a; done) >>testfile &

Then try and archive it:

$ tar cf test.tar testfile
tar: (Empty error message)


>Fix:
This code in write_file_data (in write.c) appears to be incorrect:

bytes_written = archive_write_data(a, buff, bytes_read);
if (bytes_written <= 0) {
    /* Write failed; this is bad */
    bsdtar_warnc(bsdtar, 0, "%s", archive_error_string(a));
    return (-1);
}

archive_write_data will correctly return 0 if the original size of the file to be archived has already been reached. Unless I'm badly misunderstanding the libarchive code, this call will return < 0, and not == 0, in the event of a real error in writing to the output archive. My suggested fix is therefore simply:

if (bytes_written < 0) {

we are testing this fix locally, and so far it seems to be correct.

>Release-Note:
>Audit-Trail:
>Unformatted: