From: Ruiwen Zhao <ruiwen@google.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: linux-unionfs@vger.kernel.org, Miklos Szeredi <miklos@szeredi.hu>
Subject: Re: [PATCH] ovl: fix failed copyup of fileattr on a symlink
Date: Tue, 5 Sep 2023 17:19:48 -0700 [thread overview]
Message-ID: <CAKd=y5Hm8c7cPmHq0hoWojTSiKEgrbRzB5wgV4wkaQ_M0VR0NQ@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxgkHCCT8prDpKPf2rgXSEwgF6yj8AeTE1qrY9V=2CaEYw@mail.gmail.com>
On Mon, Sep 4, 2023 at 11:57 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Sep 4, 2023 at 9:40 PM Ruiwen Zhao <ruiwen@google.com> wrote:
> >
> > On Mon, Sep 4, 2023 at 6:44 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Mon, 4 Sept 2023 at 15:24, Amir Goldstein <amir73il@gmail.com> wrote:
> > > >
> > > > Some local filesystems support setting persistent fileattr flags
> > > > (e.g. FS_NOATIME_FL) on directories and regular files via ioctl.
> > > > Some of those persistent fileattr flags are reflected to vfs as
> > > > in-memory inode flags (e.g. S_NOATIME).
> > > >
> > > > Overlayfs uses the in-memory inode flags (e.g. S_NOATIME) on a lower file
> > > > as an indication that a the lower file may have persistent inode fileattr
> > > > flags (e.g. FS_NOATIME_FL) that need to be copied to upper file.
> > > >
> > > > However, in some cases, the S_NOATIME in-memory flag could be a false
> > > > indication for persistent FS_NOATIME_FL fileattr. For example, with NFS
> > > > and FUSE lower fs, as was the case in the two bug reports, the S_NOATIME
> > > > flag is set unconditionally for all inodes.
> > > >
> > > > Users cannot set persistent fileattr flags on symlinks and special files,
> > > > but in some local fs, such as ext4/btrfs/tmpfs, the FS_NOATIME_FL fileattr
> > > > flag are inheritted to symlinks and special files from parent directory.
> > > >
> > > > In both cases described above, when lower symlink has the S_NOATIME flag,
> > > > overlayfs will try to copy the symlink's fileattrs and fail with error
> > > > ENOXIO, because it could not open the symlink for the ioctl security hook.
> > > >
> > > > To solve this failure, do not attempt to copyup fileattrs for anything
> > > > other than directories and regular files.
> > > >
> > > > Reported-by: Ruiwen Zhao <ruiwen@google.com>
> > > > Link: https://lore.kernel.org/r/CAKd=y5Hpg7J2gxrFT02F94o=FM9QvGp=kcH1Grctx8HzFYvpiA@mail.gmail.com/
> > > > Fixes: 72db82115d2b ("ovl: copy up sync/noatime fileattr flags")
> > > > Cc: <stable@vger.kernel.org> # v5.15
> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > ---
> > > >
> > > > Hi Miklos,
> > > >
> > > > Do you agree with this solution?
> > >
> > > It's good enough. Linux might add API's in the future that allow
> > > querying and setting fileattr on symlinks, but we can deal with that
> > > later.
> > >
> > > Thanks,
> > > Miklos
> >
> > Thanks Amir for sending the fix! The fix looks good but I am not sure
> > how to LGTM it. (Still new to the kernel review process)
> >
> > I believe the fix will be merged to the master branch first. Can we
> > backport it to 5.15, considering this is a regression and 5.15 is an
> > LTS version?
> >
>
> The commit, when it gets merged will include:
>
> Reported-by: Ruiwen Zhao <ruiwen@google.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217850
> Fixes: 72db82115d2b ("ovl: copy up sync/noatime fileattr flags")
> Cc: <stable@vger.kernel.org> # v5.15
>
> So it should be automatically picked for stable bots
> and by bugzilla I suppose.
>
> Please let me know if you tested my patch and I will change to
> Reported-and-tested-by:
>
> Thanks,
> Amir.
Thanks! I tested this patch with the same repro step mentioned before
and can confirm it fixed the issue. Also checked dmesg and can confirm
there is no error. I think we can merge the fix.
ruiwen@instance-1:/tmp # mv merged/home/ruiwen/foolink
merged/home/ruiwen/foolink2
ruiwen@instance-1:/tmp # ls -l merged/home/ruiwen/ -l
total 0
-rw-r--r-- 1 root root 0 Sep 6 00:07 foo
lrwxrwxrwx 1 root root 3 Sep 6 00:07 foolink2 -> foo
Thanks,
Ruiwen
prev parent reply other threads:[~2023-09-06 0:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-04 13:24 [PATCH] ovl: fix failed copyup of fileattr on a symlink Amir Goldstein
2023-09-04 13:43 ` Miklos Szeredi
2023-09-04 18:40 ` Ruiwen Zhao
2023-09-04 18:57 ` Amir Goldstein
2023-09-06 0:19 ` Ruiwen Zhao [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKd=y5Hm8c7cPmHq0hoWojTSiKEgrbRzB5wgV4wkaQ_M0VR0NQ@mail.gmail.com' \
--to=ruiwen@google.com \
--cc=amir73il@gmail.com \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).