Thank you for the fix in 35d9845d5d41023 and 0b74885e81b90d6ab4. The
patches simplify the code a lot. I just think two in-code doc fixes are
worth it, patch attached. Per https://github.com/praiskup/tar/pull/2
Pavel
On středa 24. listopadu 2021 15:56:05 CEST Pavel Raiskup wrote:
> 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 4fc376ec43c8a235306ef719db3e157a67969598 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Thu, 18 May 2023 14:30:08 +0200
Subject: [PATCH] Comment a bit on the xattr extraction logic
* src/extract.c (extract_file): Document why we pre-create with S_IWUSR.
(set_xattr): Drop the INVERT_PERMISSIONS doc leftover.
---
src/extract.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/extract.c b/src/extract.c
index aec5de69..7adc7aff 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -902,11 +902,10 @@ maybe_recoverable (char *file_name, bool regular, bool *interdir_made)
(e.g. on Lustre distributed parallel filesystem - setting info about how many
servers is this file striped over, stripe size, mirror copies, etc.
in advance dramatically improves the following performance of reading and
- writing a file). If not restoring permissions, invert the INVERT_PERMISSIONS
- bits from the file's current permissions. TYPEFLAG specifies the type of the
- file. Return a negative number (setting errno) on failure, zero if
- successful but FILE_NAME was not created (e.g., xattrs not
- available), and a positive number if FILE_NAME was created. */
+ writing a file). TYPEFLAG specifies the type of the file. Return a negative
+ number (setting errno) on failure, zero if successful but FILE_NAME was not
+ created (e.g., xattrs not available), and a positive number if FILE_NAME was
+ created. */
static int
set_xattr (char const *file_name, struct tar_stat_info const *st,
mode_t mode, char typeflag)
@@ -1271,6 +1270,10 @@ extract_file (char *file_name, int typeflag)
else
{
int file_created;
+ /* Either we pre-create the file in set_xattr(), or we just directly open
+ the file in open_output_file() with O_CREAT. If pre-creating, we need
+ to use S_IWUSR so we can open the file O_WRONLY in open_output_file().
+ The additional mode bit is cleared later by set_stat()->set_mode(). */
while (((file_created = set_xattr (file_name, ¤t_stat_info,
mode | S_IWUSR, typeflag))
< 0)
--
2.40.1
