Performance: If the dir mode requested is "reasonable", then just

restore it directly and skip chmod() during the post-extract fixup.
In particular, bsdtar -xm now completely skips the post-extract fixup
for directories, which produces a noticable speedup in that case.

1 files changed, 89 insertions, 49 deletions

diff --git a/lib/libarchive/archive_read_extract.c b/lib/libarchive/archive_read_extract.c
index badf4eb..f9e180f 100644
--- a/lib/libarchive/archive_read_extract.c
+++ b/lib/libarchive/archive_read_extract.c
@@ -85,7 +85,8 @@ struct bucket {
 
 struct extract {
 	mode_t			 umask;
-	mode_t			 default_dir_mode;
+	mode_t			 default_dir_mode_initial;
+	mode_t			 default_dir_mode_final;
 	struct archive_string	 create_parent_dir;
 	struct fixup_entry	*fixup_list;
 	struct fixup_entry	*current_fixup;
@@ -108,9 +109,11 @@ struct extract {
  * Mode to use for newly-created dirs during extraction; the correct
  * mode will be set at the end of the extraction.
  */
-#define SECURE_DIR_MODE 0700
+#define MINIMUM_DIR_MODE 0700
+#define MAXIMUM_DIR_MODE 0775
 
 static int	archive_extract_cleanup(struct archive *);
+static int	create_extract(struct archive *a);
 static int	extract_block_device(struct archive *,
 		    struct archive_entry *, int);
 static int	extract_char_device(struct archive *,
@@ -169,17 +172,11 @@ archive_read_extract(struct archive *a, struct archive_entry *entry, int flags)
 	char *original_filename;
 
 	if (a->extract == NULL) {
-		a->extract = malloc(sizeof(*a->extract));
-		if (a->extract == NULL) {
-			archive_set_error(a, ENOMEM, "Can't extract");
-			return (ARCHIVE_FATAL);
-		}
-		a->cleanup_archive_extract = archive_extract_cleanup;
-		memset(a->extract, 0, sizeof(*a->extract));
+		ret = create_extract(a);
+		if (ret)
+			return (ret);
 	}
 	extract = a->extract;
-	umask(extract->umask = umask(0)); /* Read the current umask. */
-	extract->default_dir_mode = DEFAULT_DIR_MODE & ~extract->umask;
 	extract->pst = NULL;
 	extract->current_fixup = NULL;
 	restore_pwd = -1;
@@ -293,6 +290,35 @@ cleanup:
 	return (ret);
 }
 
+
+static int
+create_extract(struct archive *a)
+{
+	struct extract *extract;
+
+	extract = malloc(sizeof(*extract));
+	if (extract == NULL) {
+		archive_set_error(a, ENOMEM, "Can't extract");
+		return (ARCHIVE_FATAL);
+	}
+	a->cleanup_archive_extract = archive_extract_cleanup;
+	memset(extract, 0, sizeof(*extract));
+	umask(extract->umask = umask(0)); /* Read the current umask. */
+	/* Final permission for default dirs. */
+	extract->default_dir_mode_final
+	    = DEFAULT_DIR_MODE & ~extract->umask;
+	/* Temporary permission for default dirs during extract. */
+	extract->default_dir_mode_initial
+	    = extract->default_dir_mode_final;
+	extract->default_dir_mode_initial |= MINIMUM_DIR_MODE;
+	extract->default_dir_mode_initial &= MAXIMUM_DIR_MODE;
+	/* If the two permissions above are different, then
+	 * the "final" permissions will be applied in the
+	 * post-extract fixup pass. */
+	a->extract = extract;
+	return (ARCHIVE_OK);
+}
+
 /*
  * Cleanup function for archive_extract.  Mostly, this involves processing
  * the fixup list, which is used to address a number of problems:
@@ -508,6 +534,7 @@ extract_dir(struct archive *a, struct archive_entry *entry, int flags)
 	struct extract *extract;
 	struct fixup_entry *fe;
 	char *path, *p;
+	mode_t restore_mode, final_mode;
 
 	extract = a->extract;
 	extract->pst = NULL; /* Invalidate cached stat data. */
@@ -554,7 +581,16 @@ extract_dir(struct archive *a, struct archive_entry *entry, int flags)
 		break;
 	}
 
-	if (mkdir(path, SECURE_DIR_MODE) == 0)
+	final_mode = archive_entry_mode(entry) &
+	    (S_ISUID | S_ISGID | S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO);
+	if ((flags & ARCHIVE_EXTRACT_PERM) == 0)
+		final_mode &= ~extract->umask;
+	/* Constrain the permissions in effect during the restore. */
+	restore_mode = final_mode;
+	restore_mode |= MINIMUM_DIR_MODE;
+	restore_mode &= MAXIMUM_DIR_MODE;
+
+	if (mkdir(path, restore_mode) == 0)
 		goto success;
 
 	if (extract->pst == NULL && stat(path, &extract->st) == 0)
@@ -575,27 +611,26 @@ extract_dir(struct archive *a, struct archive_entry *entry, int flags)
 	}
 
 	/* One final attempt to create the dir. */
-	if (mkdir(path, SECURE_DIR_MODE) != 0) {
+	if (mkdir(path, restore_mode) != 0) {
 		archive_set_error(a, errno, "Can't create directory");
 		return (ARCHIVE_WARN);
 	}
 
 success:
 	/* Add this dir to the fixup list. */
-	fe = current_fixup(a, path);
-	fe->fixup |= FIXUP_MODE;
-	fe->mode = archive_entry_mode(entry);
-	if ((flags & ARCHIVE_EXTRACT_PERM) == 0)
-		fe->mode &= ~extract->umask;
+	if (final_mode != restore_mode) {
+		fe = current_fixup(a, path);
+		fe->fixup |= FIXUP_MODE;
+		fe->mode = final_mode;
+	}
 	if (flags & ARCHIVE_EXTRACT_TIME) {
+		fe = current_fixup(a, path);
 		fe->fixup |= FIXUP_TIMES;
 		fe->mtime = archive_entry_mtime(entry);
 		fe->mtime_nanos = archive_entry_mtime_nsec(entry);
 		fe->atime = archive_entry_atime(entry);
 		fe->atime_nanos = archive_entry_atime_nsec(entry);
 	}
-	/* For now, set the mode to SECURE_DIR_MODE. */
-	archive_entry_set_mode(entry, SECURE_DIR_MODE);
 	return (restore_metadata(a, -1, entry, flags));
 }
 
@@ -656,12 +691,9 @@ create_parent_dir_mutable(struct archive *a, char *path, int flags)
 static int
 create_dir_mutable(struct archive *a, char *path, int flags)
 {
-	mode_t old_umask;
 	int r;
 
-	old_umask = umask(~SECURE_DIR_MODE);
 	r = create_dir_recursive(a, path, flags);
-	umask(old_umask);
 	return (r);
 }
 
@@ -735,10 +767,13 @@ create_dir_recursive(struct archive *a, char *path, int flags)
 			return (r);
 	}
 
-	if (mkdir(path, SECURE_DIR_MODE) == 0) {
-		le = new_fixup(a, path);
-		le->fixup |= FIXUP_MODE;
-		le->mode = extract->default_dir_mode;
+	if (mkdir(path, extract->default_dir_mode_initial) == 0) {
+		if (extract->default_dir_mode_initial
+		    != extract->default_dir_mode_final) {
+			le = new_fixup(a, path);
+			le->fixup |= FIXUP_MODE;
+			le->mode = extract->default_dir_mode_final;
+		}
 		return (ARCHIVE_OK);
 	}
 
@@ -1052,30 +1087,12 @@ set_perm(struct archive *a, int fd, struct archive_entry *entry,
 			mode &= ~ S_ISGID;
 	}
 
-	/*
-	 * Ensure we change permissions on the object we extracted,
-	 * and not any incidental symlink that might have gotten in
-	 * the way.
-	 */
-	if (!S_ISLNK(archive_entry_mode(entry))) {
-#ifdef HAVE_FCHMOD
-		if (fd >= 0) {
-			if (fchmod(fd, mode) != 0) {
-				archive_set_error(a, errno,
-				    "Can't set permissions");
-				return (ARCHIVE_WARN);
-			}
-		} else
-#endif
-		if (chmod(name, mode) != 0) {
-			archive_set_error(a, errno, "Can't set permissions");
-			return (ARCHIVE_WARN);
-		}
+	if (S_ISLNK(archive_entry_mode(entry))) {
 #ifdef HAVE_LCHMOD
-	} else {
 		/*
-		 * If lchmod() isn't supported, it's no big deal.
-		 * Permissions on symlinks are actually ignored on
+		 * If this is a symlink, use lchmod().  If the
+		 * platform doesn't support lchmod(), just skip it as
+		 * permissions on symlinks are actually ignored on
 		 * most platforms.
 		 */
 		if (lchmod(name, mode) != 0) {
@@ -1083,6 +1100,29 @@ set_perm(struct archive *a, int fd, struct archive_entry *entry,
 			return (ARCHIVE_WARN);
 		}
 #endif
+	} else if (!S_ISDIR(archive_entry_mode(entry))) {
+		/*
+		 * If it's not a symlink and not a dir, then use
+		 * fchmod() or chmod(), depending on whether we have
+		 * an fd.  Dirs get their perms set during the
+		 * post-extract fixup, which is handled elsewhere.
+		 */
+#ifdef HAVE_FCHMOD
+		if (fd >= 0) {
+			if (fchmod(fd, mode) != 0) {
+				archive_set_error(a, errno,
+				    "Can't set permissions");
+				return (ARCHIVE_WARN);
+			}
+		} else
+#endif
+			/* If this platform lacks fchmod(), then
+			 * we'll just use chmod(). */
+			if (chmod(name, mode) != 0) {
+				archive_set_error(a, errno,
+				    "Can't set permissions");
+				return (ARCHIVE_WARN);
+			}
 	}
 
 	if (flags & ARCHIVE_EXTRACT_ACL) {