Fix bug with -x --xattr read-only files

Problem reported by Kevin Raymond in:
https://bugzilla.redhat.com/show_bug.cgi?id=1886540
* src/extract.c (open_output_file): If we already created the
empty file, do not open with O_EXCL, or with O_CREAT or O_TRUNC
for that matter.  Instead, use only O_NOFOLLOW to avoid some
races.  When estimating current mode, use openflag & O_EXCL rather
than overwriting_old_files.
(extract_file): Also invert S_IWUSR if it’s not set.
* tests/xattr08.at: New test.
* tests/Makefile.am, tests/testsuite.at: Add it.

diff --git a/src/extract.c b/src/extract.c
index 2813c961e4093541a2de24ea5ec86ee1fa22e11c..c1f5731bad2ebd9a1232f25fb8da2a71b52c9077 100644 (file)
--- a/src/extract.c
+++ b/src/extract.c
@@ -1189,14 +1189,12 @@ open_output_file (char const *file_name, int typeflag, mode_t mode,
   int fd;
   bool overwriting_old_files = old_files_option == OVERWRITE_OLD_FILES;
   int openflag = (O_WRONLY | O_BINARY | O_CLOEXEC | O_NOCTTY | O_NONBLOCK
-                 | O_CREAT
-                 | (overwriting_old_files
-                    ? O_TRUNC | (dereference_option ? 0 : O_NOFOLLOW)
-                    : O_EXCL));
-
-  /* File might be created in set_xattr. So clear O_EXCL to avoid open() fail */
-  if (file_created)
-    openflag = openflag & ~O_EXCL;
+                 | (file_created
+                    ? O_NOFOLLOW
+                    : (O_CREAT
+                       | (overwriting_old_files
+                          ? O_TRUNC | (dereference_option ? 0 : O_NOFOLLOW)
+                          : O_EXCL))));
 
   if (typeflag == CONTTYPE)
     {
@@ -1227,7 +1225,12 @@ open_output_file (char const *file_name, int typeflag, mode_t mode,
   fd = openat (chdir_fd, file_name, openflag, mode);
   if (0 <= fd)
     {
-      if (overwriting_old_files)
+      if (openflag & O_EXCL)
+       {
+         *current_mode = mode & ~ current_umask;
+         *current_mode_mask = MODE_RWX;
+       }
+      else
        {
          struct stat st;
          if (fstat (fd, &st) != 0)
@@ -1246,11 +1249,6 @@ open_output_file (char const *file_name, int typeflag, mode_t mode,
          *current_mode = st.st_mode;
          *current_mode_mask = ALL_MODE_BITS;
        }
-      else
-       {
-         *current_mode = mode & ~ current_umask;
-         *current_mode_mask = MODE_RWX;
-       }
     }
 
   return fd;
@@ -1268,8 +1266,9 @@ extract_file (char *file_name, int typeflag)
   bool interdir_made = false;
   mode_t mode = (current_stat_info.stat.st_mode & MODE_RWX
                 & ~ (0 < same_owner_option ? S_IRWXG | S_IRWXO : 0));
-  mode_t invert_permissions = 0 < same_owner_option ? mode & (S_IRWXG | S_IRWXO)
-                                                    : 0;
+  mode_t invert_permissions
+    = ((0 < same_owner_option ? mode & (S_IRWXG | S_IRWXO) : 0)
+       | ((mode & S_IWUSR) ^ S_IWUSR));
   mode_t current_mode = 0;
   mode_t current_mode_mask = 0;
 

diff --git a/tests/Makefile.am b/tests/Makefile.am
index eff8a3bf15ab0b00e509fd4a6fc5b4ae8a82a972..a0ce690b2b541fdfea404c730d4e84bd3ef0766a 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -275,6 +275,7 @@ TESTSUITE_AT = \
  xattr05.at\
  xattr06.at\
  xattr07.at\
+ xattr08.at\
  acls01.at\
  acls02.at\
  acls03.at\

diff --git a/tests/testsuite.at b/tests/testsuite.at
index 0769e71bb6578bc69d3e3d8aad9bc30bc96c6ef6..a99cdeee18ce6d3540f167b559c47f505cb143c7 100644 (file)
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -502,6 +502,7 @@ m4_include([xattr04.at])
 m4_include([xattr05.at])
 m4_include([xattr06.at])
 m4_include([xattr07.at])
+m4_include([xattr08.at])
 
 m4_include([acls01.at])
 m4_include([acls02.at])

diff --git a/tests/xattr08.at b/tests/xattr08.at
new file mode 100644 (file)
index 0000000..2beef23
--- /dev/null
+++ b/tests/xattr08.at
@@ -0,0 +1,41 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This file is part of GNU tar.
+
+# GNU tar is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# GNU tar is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# Test description:
+# Test read-only files can be extracted with --xattr.
+# Per report:
+# https://lists.gnu.org/r/bug-tar/2020-10/msg00001.html
+
+AT_SETUP([xattrs: xattrs and read-only files])
+AT_KEYWORDS([xattrs xattr08])
+
+AT_TAR_CHECK([
+AT_XATTRS_PREREQ
+mkdir dir dir2
+genfile --file dir/file
+
+setfattr -n user.test -v OurDirValue dir/file
+chmod a-w dir/file
+
+tar --xattrs -C dir -cf archive.tar file
+tar --xattrs -C dir2 -xf archive.tar
+])
+
+AT_CLEANUP