From owner-freebsd-current@FreeBSD.ORG Tue Jan 20 21:16:37 2009 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A3397106564A; Tue, 20 Jan 2009 21:16:37 +0000 (UTC) (envelope-from keramida@FreeBSD.org) Received: from igloo.linux.gr (igloo.linux.gr [62.1.205.36]) by mx1.freebsd.org (Postfix) with ESMTP id 089D38FC13; Tue, 20 Jan 2009 21:16:36 +0000 (UTC) (envelope-from keramida@FreeBSD.org) Received: from kobe.laptop (adsl189-242.kln.forthnet.gr [79.103.2.242]) (authenticated bits=128) by igloo.linux.gr (8.14.3/8.14.3/Debian-5) with ESMTP id n0KKxUom011264 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 20 Jan 2009 22:59:36 +0200 Received: from kobe.laptop (kobe.laptop [127.0.0.1]) by kobe.laptop (8.14.3/8.14.3) with ESMTP id n0KKxTjR098892; Tue, 20 Jan 2009 22:59:29 +0200 (EET) (envelope-from keramida@freebsd.org) Received: (from keramida@localhost) by kobe.laptop (8.14.3/8.14.3/Submit) id n0KKxTkK098891; Tue, 20 Jan 2009 22:59:29 +0200 (EET) (envelope-from keramida@freebsd.org) From: Giorgos Keramidas To: Tim Kientzle Date: Tue, 20 Jan 2009 22:59:28 +0200 Message-ID: <87skndg0cv.fsf@kobe.laptop> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-MailScanner-ID: n0KKxUom011264 X-Hellug-MailScanner: Found to be clean X-Hellug-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-4.149, required 5, autolearn=not spam, ALL_TRUSTED -1.80, AWL 0.25, BAYES_00 -2.60) X-Hellug-MailScanner-From: keramida@freebsd.org X-Spam-Status: No Cc: freebsd-current@FreeBSD.org Subject: [PATCH] bsdcpio core dump X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jan 2009 21:16:38 -0000 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.