Re: [PATCH] Don't pre-create read-only files with --extract --xattrs

[Pavel Raiskup]

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);
> > > > 
> > > 
> > > 
> > 
> > 
> > 
> > 
> > 
> 
>

0001-Comment-a-bit-on-the-xattr-extraction-logic.patch
Description: Text Data