From: Tim Kientzle <kientzle@gmail.com>
Date: Sun, 8 Nov 2009 07:06:02 +0000 (-0500)
Subject: Thanks to Xavier for pointing out that the security.capability xattr
X-Git-Tag: v2.8.0~225
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=26f63f459a9f03ac1389f12510e292339e23c93d;p=thirdparty%2Flibarchive.git

Thanks to Xavier for pointing out that the security.capability xattr
on Linux must be restored last.  Let's try moving xattr restore to
after file data and see if that works any better; we may end up having
to do a full xattr restore both before and after the file data.

This also changes the behavior of xattr restore failures; they now
cause warnings when the entry is finished (which can be as late as
archive_write_close()) rather than when the header is written.

SVN-Revision: 1590
---

diff --git a/libarchive/archive_write_disk.c b/libarchive/archive_write_disk.c
index 772219657..b824cc9e7 100644
--- a/libarchive/archive_write_disk.c
+++ b/libarchive/archive_write_disk.c
@@ -442,15 +442,14 @@ _archive_write_header(struct archive *_a, struct archive_entry *entry)
 	ret = restore_entry(a);
 
 	/*
-	 * On the GNU tar mailing list, some people working with new
-	 * Linux filesystems observed that system xattrs used as
-	 * layout hints need to be restored before the file contents
-	 * are written, so this can't be done at file close.
+	 * TODO: There are rumours that some extended attributes must
+	 * be restored before file data is written.  If this is true,
+	 * then we either need to write all extended attributes both
+	 * before and after restoring the data, or find some rule for
+	 * determining which must go first and which last.  Due to the
+	 * many ways people are using xattrs, this may prove to be an
+	 * intractable problem.
 	 */
-	if (a->todo & TODO_XATTR) {
-		int r2 = set_xattrs(a);
-		if (r2 < ret) ret = r2;
-	}
 
 #ifdef HAVE_FCHDIR
 	/* If we changed directory above, restore it here. */
@@ -754,6 +753,17 @@ _archive_write_finish_entry(struct archive *_a)
 		int r2 = set_acls(a);
 		if (r2 < ret) ret = r2;
 	}
+
+	/*
+	 * Security-related extended attributes (such as
+	 * security.capability on Linux) have to be restored last,
+	 * since they're implicitly removed by other file changes.
+	 */
+	if (a->todo & TODO_XATTR) {
+		int r2 = set_xattrs(a);
+		if (r2 < ret) ret = r2;
+	}
+
 	/*
 	 * Some flags prevent file modification; they must be restored after
 	 * file contents are written.
diff --git a/libarchive/test/test_extattr_freebsd.c b/libarchive/test/test_extattr_freebsd.c
index 8b8cc7e6e..de3ae2a58 100644
--- a/libarchive/test/test_extattr_freebsd.c
+++ b/libarchive/test/test_extattr_freebsd.c
@@ -114,17 +114,15 @@ DEFINE_TEST(test_extattr_freebsd)
 	archive_entry_set_size(ae, 0);
 	archive_entry_set_mode(ae, 0);
 	archive_entry_xattr_add_entry(ae, "user.bar", "123456", 6);
-	if (extattr_privilege_bug)
-		/* If the bug is here, write_header will return warning. */
-		assertEqualIntA(a, ARCHIVE_WARN,
-		    archive_write_header(a, ae));
-	else
-		assertEqualIntA(a, ARCHIVE_OK,
-		    archive_write_header(a, ae));
+	assertEqualIntA(a, ARCHIVE_OK, archive_write_header(a, ae));
 	archive_entry_free(ae);
 
 	/* Close the archive. */
-	assertEqualIntA(a, ARCHIVE_OK, archive_write_close(a));
+	if (extattr_privilege_bug)
+		/* If the bug is here, write_close will return warning. */
+		assertEqualIntA(a, ARCHIVE_WARN, archive_write_close(a));
+	else
+		assertEqualIntA(a, ARCHIVE_OK, archive_write_close(a));
 	assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
 
 	/* Verify the data on disk. */