usr.bin/tar/ ignores error codes from read() silently pads with nulls, corrupting copied data without warning ! Log of how I discovered problem + diffs + copy of send-pr: http://berklix.com/~jhs/src/bsd/fixes/FreeBSD/src/gen/usr.bin/tar/ (various files have .ignore extension to avoid my customise script). Comparison with gtar: gtar also delivers false nulls unfortunately, gtar warns users it is getting device errors. Comparison with cp: cp warns of errors, cp delivers no false nulls, truncates at read fail. Fix: My patches fix code to avoid unwarned damaged data. As it's Tim K.'s recently written code, I presume he will want to fix it himself, so my patch lines as well as fixing, are also appended with edit mark // JHS for easy searching. write.c & util.c were bad in 8.1-RELEASE (& the critical bit in ./-current/src/usr/bin/write.c ^write_file_data() the final bytes_read = read(fd, bsdtar->buff, FILEDATABUFLEN); is also bad) Various return values silently ignored. All invocations of [f]read() & [f]write() & all invocations of *_read_* & *_write_* etc should be checked whether return value is properly used. Later fixes if any will be in http://berklix.com/~jhs/src/bsd/fixes/FreeBSD/src/gen/usr.bin/tar/ A copy appended. How-To-Repeat: - Get a normal data CD or DVD you have created ) (not commercial recorded hollywood deliberately sector crippled media), - Damage the media so it will have some read errors, mount /dev/acd0 /cdrom cd /tmp mkdir -p cp tar/cdrom cp -R /cdrom cp (cd /cdrom ; tar cf - . ) | ( cd tar/cdrom ; tar xf - ) cd tar/cdrom # http://berklix.com/~jhs/src/bsd/jhs/bin/public/cmpd/ find . -type f -exex cmpd -d {} ../../cp/cdrom \; # http://berklix.com/~jhs/src/bsd/jhs/bin/public/8f/ foreach i ( `find . -type f | xargs 8f -n 2048 -b 0 -l` ) echo $i xargs od -c $i | yail end
Responsible Changed From-To: freebsd-bugs->kientzle Over to maintainer.
Author: kientzle Date: Sat Jun 25 16:27:49 2011 New Revision: 223541 URL: http://svn.freebsd.org/changeset/base/223541 Log: If there is a read error reading Y/N confirmation from the keyboard, exit immediately with an error. If there is an error opening or reading a file to put into the archive, set the return value for a deferred error exit. PR: bin/154407 Modified: head/usr.bin/tar/util.c head/usr.bin/tar/write.c Modified: head/usr.bin/tar/util.c ============================================================================== --- head/usr.bin/tar/util.c Sat Jun 25 16:13:56 2011 (r223540) +++ head/usr.bin/tar/util.c Sat Jun 25 16:27:49 2011 (r223541) @@ -226,7 +226,11 @@ yes(const char *fmt, ...) fflush(stderr); l = read(2, buff, sizeof(buff) - 1); - if (l <= 0) + if (l < 0) { + fprintf(stderr, "Keyboard read failed\n"); + exit(1); + } + if (l == 0) return (0); buff[l] = 0; Modified: head/usr.bin/tar/write.c ============================================================================== --- head/usr.bin/tar/write.c Sat Jun 25 16:13:56 2011 (r223540) +++ head/usr.bin/tar/write.c Sat Jun 25 16:27:49 2011 (r223541) @@ -919,6 +919,7 @@ write_entry_backend(struct bsdtar *bsdta const char *pathname = archive_entry_sourcepath(entry); fd = open(pathname, O_RDONLY | O_BINARY); if (fd == -1) { + bsdtar->return_value = 1; if (!bsdtar->verbose) bsdtar_warnc(errno, "%s: could not open file", pathname); @@ -1020,6 +1021,12 @@ write_file_data(struct bsdtar *bsdtar, s progress += bytes_written; bytes_read = read(fd, bsdtar->buff, FILEDATABUFLEN); } + if (bytes_read < 0) { + bsdtar_warnc(errno, + "%s: Read error", + archive_entry_pathname(entry)); + bsdtar->return_value = 1; + } return 0; } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Please take a look at r223541, which I believe fixes this problem in = -CURRENT. My solution is a little different from yours; read errors will not abort = archiving immediately but will instead report the error and set a deferred exit value for tar. Tim
State Changed From-To: open->patched r223541 fixes this in trunk
State Changed From-To: patched->closed This appears to have been fixed in all supported release some time ago.