Gently pinging on this again. This causes invalid extraction failures. I'm ready for the review cycles (patch attached :-)!
Pavel On Monday, February 15, 2021 10:00:20 AM CET Pavel Raiskup wrote: > Ping, I can see there's 1.34 release now, thank you! > > But the git isn't updated yet so I'm not sure this has been fixed or not. > > Pavel > > On Tuesday, February 9, 2021 5:06:31 PM CET Pavel Raiskup wrote: > > Gently ping on this. This is easily reproducible, so I am re-attaching > > the patch including a new test-case that fails with the current code: > > > > $ tar --xattrs -xf tar > > tar: setxattrat: Cannot set 'user.attr' extended attribute for file > > 'file': Permission denied > > tar: file: Cannot open: Permission denied > > tar: Exiting with failure status due to previous errors > > > > Pavel > > > > On Friday, October 9, 2020 1:39:11 PM CET Pavel Raiskup wrote: > > > We used to respect the target file mode when pre-creating files in > > > set_xattr, so we also pre-created read-only files that we were not able > > > to open later for writing. This is now fixed, and we always create the > > > file with S_IWUSR. > > > > > > Fixes the original bug report https://bugzilla.redhat.com/1886540 > > > > > > * src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created > > > files, to avoid openat failures later. > > > --- > > > src/extract.c | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/extract.c b/src/extract.c > > > index b73a591..2e650ad 100644 > > > --- a/src/extract.c > > > +++ b/src/extract.c > > > @@ -865,7 +865,13 @@ set_xattr (char const *file_name, struct > > > tar_stat_info const *st, > > > > > > for (;;) > > > { > > > - if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, > > > 0)) > > > + /* We'll open the file with O_WRONLY later by open_output_file, > > > + therefore we need to give us the S_IWUSR bit. If the file > > > was > > > + meant to be user-read-only, the permissions will be > > > corrected by > > > + the set_stat call. */ > > > + mode_t initial_mode = mode ^ invert_permissions | S_IWUSR; > > > + > > > + if (!mknodat (chdir_fd, file_name, initial_mode, 0)) > > > { > > > /* Successfully created file */ > > > xattrs_xattrs_set (st, file_name, typeflag, 0); > > > > > > > > > > > >
>From 5f9283cd00273115097920da7ece8e97ccf1902b Mon Sep 17 00:00:00 2001 From: Pavel Raiskup <prais...@redhat.com> Date: Fri, 9 Oct 2020 12:03:22 +0200 Subject: [PATCH] Don't pre-create read-only files with --extract --xattrs We used to respect the target file mode when pre-creating files in set_xattr, so we also pre-created read-only files that we were not able to open later for writing. This is now fixed, and we always create the file with S_IWUSR. Fixes the original bug report https://bugzilla.redhat.com/1886540 * src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created files, to avoid openat failures later. * tests/xattr08.at: New test-case file. --- src/extract.c | 8 +++++++- tests/xattr08.at | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 tests/xattr08.at diff --git a/src/extract.c b/src/extract.c index 850a08a2..1bfe36a6 100644 --- a/src/extract.c +++ b/src/extract.c @@ -860,7 +860,13 @@ set_xattr (char const *file_name, struct tar_stat_info const *st, for (;;) { - if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0)) + /* We'll open the file with O_WRONLY later by open_output_file, + therefore we need to give us the S_IWUSR bit. If the file was + meant to be user-read-only, the permissions will be corrected by + the set_stat call. */ + mode_t initial_mode = mode ^ invert_permissions | S_IWUSR; + + if (!mknodat (chdir_fd, file_name, initial_mode, 0)) { /* Successfully created file */ xattrs_xattrs_set (st, file_name, typeflag, 0); diff --git a/tests/xattr08.at b/tests/xattr08.at new file mode 100644 index 00000000..0dd3ed34 --- /dev/null +++ b/tests/xattr08.at @@ -0,0 +1,45 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- +# +# Test suite for GNU tar. +# Copyright 2021 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 for extracting read-only files with xattrs. +# +# Relevant mailing list thread: +# https://lists.gnu.org/archive/html/bug-tar/2020-10/msg00001.html + +AT_SETUP([xattrs: extract read-only files]) +AT_KEYWORDS([xattrs xattr07]) + +AT_TAR_CHECK([ +AT_XATTRS_PREREQ + +touch file +setfattr -n 'user.attr' -v value file +chmod 444 file +tar --xattrs -cf tar file +rm -f file +tar --xattrs -xf tar +getfattr -m user.attr file -d | grep user.attr +], +[0], +[user.attr="value" +], +[]) + +AT_CLEANUP -- 2.33.1
