Date: Tue, 20 Jan 2009 22:59:28 +0200 From: Giorgos Keramidas <keramida@FreeBSD.org> To: Tim Kientzle <kientzle@FreeBSD.org> Cc: freebsd-current@FreeBSD.org Subject: [PATCH] bsdcpio core dump Message-ID: <87skndg0cv.fsf@kobe.laptop>
next in thread | raw e-mail | index | archive | help
Hi Tim & -CURRENT,
I think I stumbled upon a minor bsdcpio bug. When bsdcpio tries to
write to a directory it doesn't have permissions it core dumps:
# mkdir /tmp/koko
# chmod 0700 /tmp/koko
% cd /ws/keramida/src/usr.in/cpio
% env DEBUG_FLAGS='-ggdb' CFLAGS='' make bsdcpio
...
% echo bsdcpio | ./bsdcpio -p -dmvu
bsdcpio: /tmp/koko/bsdcpio: Can't create '/tmp/koko/bsdcpio': Permission denied: Permission denied
INTERNAL ERROR: Function 'archive_write_data' invoked with archive structure in state 'header', should be in state 'data'
Segmentation fault: 11 (core dumped)
% gdb bsdcpio bsdcpio.core
#0 0x280b9ad7 in diediedie () at archive_check_magic.c:55
55 *(char *)0 = 1; /* Deliberately segfault and force a coredump. */
(gdb) bt
#0 0x280b9ad7 in diediedie () at archive_check_magic.c:55
#1 0x280b9caf in __archive_check_magic (a=0x28202140, magic=3221336261, state=4,
function=0x280bc960 "archive_write_data") at archive_check_magic.c:116
#2 0x2809f95d in _archive_write_data (_a=0x28202140, buff=0x804f360, size=16384) at archive_write_disk.c:625
#3 0x280a2e35 in archive_write_data (a=0x28202140, buff=0x804f360, s=16384) at archive_virtual.c:74
#4 0x0804b534 in entry_to_archive (cpio=0xbfbfe6ec, entry=0x282231c0) at cpio.c:637
#5 0x0804b219 in file_to_archive (cpio=0xbfbfe6ec, srcpath=0x2821e000 "bsdcpio") at cpio.c:542
#6 0x0804c1d1 in mode_pass (cpio=0xbfbfe6ec, destdir=0xbfbfe94e "/tmp/koko") at cpio.c:939
#7 0x0804a899 in main (argc=4, argv=0xbfbfe7c8) at cpio.c:291
(gdb)
It seems that cpio tries in entry_to_archive() to handle errors in calls
of archive_write_header() but it should be a bit more strict about it.
I picked up ARCHIVE_RETRY as the smallest error code we 'accept' when
archive_write_header() fails, but the choice seems pretty arbitrary.
The patch does solve the core dump I was seeing though. Does it look
good enough for cpio? Should we handle archive_write_header() errors
differently?
%%%
diff -r cb9a95f8dfb3 usr.bin/cpio/cpio.c
--- a/usr.bin/cpio/cpio.c Tue Jan 20 21:45:52 2009 +0200
+++ b/usr.bin/cpio/cpio.c Tue Jan 20 22:56:48 2009 +0200
@@ -623,12 +623,12 @@
r = archive_write_header(cpio->archive, entry);
if (r != ARCHIVE_OK)
- cpio_warnc(archive_errno(cpio->archive),
+ cpio_warnc(0,
"%s: %s",
- destpath,
+ srcpath,
archive_error_string(cpio->archive));
- if (r == ARCHIVE_FATAL)
+ if (r < ARCHIVE_RETRY)
exit(1);
if (r >= ARCHIVE_WARN && fd >= 0) {
%%%
While here, I also changed destpath to srcpath in the warning and
changed the 'code' argument of cpio_warnc() to zero, to avoid a
duplicate strerror() like output. The message returned by
archive_error_string() contains the destination path and strerror() text
already, so this changes the error from:
bsdcpio: /tmp/koko/bsdcpio: Can't create '/tmp/koko/bsdcpio': Permission denied: Permission denied
to:
bsdcpio: ./bsdcpio: Can't create '/tmp/koko/bsdcpio': Permission denied
The cosmetic changes can probably go in a separate commit, but we should
probably fix & backport the core dump when archive_write_header() fails.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?87skndg0cv.fsf>