[PATCH] bsdcpio core dump

From: Giorgos Keramidas <keramida_at_FreeBSD.org>
Date: Tue, 20 Jan 2009 22:59:28 +0200
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
_at__at_ -623,12 +623,12 _at__at_
 	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.
Received on Tue Jan 20 2009 - 20:16:37 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:40 UTC