From: Amir Goldstein <amir73il@gmail.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Ruiwen Zhao <ruiwen@google.com>,
linux-unionfs@vger.kernel.org,
Sergey Kanzhelev <skanzhelev@google.com>,
Michael Sheinin <msheinin@google.com>,
Theodore Tso <tytso@mit.edu>,
Christian Brauner <brauner@kernel.org>,
"Darrick J. Wong" <djwong@kernel.org>
Subject: Re: [Bug report] overlayfs: cannot rename symlink if lower filesystem is FUSE/NFS
Date: Sun, 3 Sep 2023 20:32:23 +0300 [thread overview]
Message-ID: <CAOQ4uxhJ5jw+mY_VJeEOXR7-xauFSY+GkTNnrr+N0nqY8dFPZQ@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxjHob=nDUghkGHpcfoAYSUNV_MZB5YHEkTUXB+bOuUBoA@mail.gmail.com>
On Fri, Sep 1, 2023 at 8:57 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Sep 1, 2023 at 1:14 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Fri, 1 Sept 2023 at 12:08, Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > On Fri, Sep 1, 2023 at 12:59 AM Ruiwen Zhao <ruiwen@google.com> wrote:
> > > >
> > > > Thanks for the reply Amir! Really helps. I will try the easy fix (i.e. ignoring ENXIO) and test it. Meanwhile I have two questions:
> > > >
> > > > 1. Since ENXIO comes from ovl_security_fileattr() trying to open the symlink, I was trying to find who returns ENXIO in the first place. I did some code search on libfuse (https://github.com/libfuse/libfuse), but cannot find ENXIO anywhere. Could it be from the kernel side? (I am trying to use this as a justification of the easy fix.)
> > > >
> > >
> > > I think ENXIO comes from no_open() default ->open() method.
> > >
> > > > 2. Miklos's commit message says "The reason is that ovl_copy_fileattr() is triggered due to S_NOATIME being
> > > > set on all inodes (by fuse) regardless of fileattr." Does that mean `ovl_copy_fileattr` should not be triggered on symlink files but it is, and therefore it is getting the errors like ENXIO and ENOTTY?
> > > >
> > >
> > > Miklos' comment explains why ovl_copy_fileattr() passes the gate:
> > >
> > > if (inode->i_flags & OVL_COPY_I_FLAGS_MASK) {
> > >
> > > S_NOATIME is an indication that the file MAY have fileattr FS_NOATIME_FL,
> > > but in the case of FUSE and NFS, S_NOATIME is there for another unrelated
> > > reason.
> > >
> > > In any case, S_NOATIME on a symlink is never an indication of fileattr,
> > > so I think the correct solution is to add the conditions to the gate:
> > >
> > > if (inode->i_flags & OVL_COPY_I_FLAGS_MASK &&
> > > ((S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode))) {
> > >
> >
> > I don't think this is correct: symlink's atime is updated on readlink
> > or following.
>
> I am not saying that symlink cannot have S_NOATIME, but
> i_flags of the symlink have already been copied to ovl inode.
> I don't think that symlink can have fileattr, because symlink
> cannot be opened for the FS_IOC_SETFLAGS ioctl, so there
> is never a reason to call ovl_copy_fileattr() for anything other
> than dir or regular file. Right?
>
Well, not exactly right.
Apparently, in ext*/btrfs/f2fs/... and recently also tmpfs, the
FS_NOATIME_FL | FS_NODUMP_FL flags are inherited
from parent to all file types, so they actually can exist on symlinks
and ext4 symlinks created inside FS_NOATIME_FL parent do indeed
exhibit S_NOATIME behavior.
This is not the case with xfs, which takes care not to copy any flags
to special files and symlinks.
The thing is that for those fs that inherit NOATIME to symlink:
1. lsattr cannot be used to query the NOATIME flag on symlink
2. chattr cannot be used to remove the NOATIME flag on symlink
3. overlayfs fails to copy up those symlinks
While technically, overlayfs can query and set flags directly without
opening the symlink, I don't think we should bother with this at all
and I think it should be fine if overlayfs only ever copied up
fileattr flags for directories and regular files.
Here is an xfstest for the regression [1].
It only fails when running fstests with ext4 or one of the other
mentioned fs as base fs.
Thanks,
Amir.
[1] https://github.com/amir73il/xfstests/blob/overlayfs-devel/tests/overlay/082
prev parent reply other threads:[~2023-09-03 17:32 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAKd=y5EeKfC6vBXh1xqTfeW6OQZiNWaZ04J1SNWxyEjY4QxhHw@mail.gmail.com>
[not found] ` <CAKd=y5HZ0nJJ9XN9i6vnyhzM=COijmuSzgqJPAPFn6dguQyFQA@mail.gmail.com>
2023-08-31 19:54 ` [Bug report] overlayfs: cannot rename symlink if lower filesystem is FUSE/NFS Amir Goldstein
[not found] ` <CAKd=y5Gsz1z1xSmHGyoEs+SckC=M0T7nrP7t5mvvuoWkCWkDLg@mail.gmail.com>
2023-09-01 10:08 ` Amir Goldstein
2023-09-01 10:14 ` Miklos Szeredi
2023-09-01 17:57 ` Amir Goldstein
[not found] ` <CAKd=y5FZ+imzR_bWK+g-xhBNvxhV2OpuHYLtm3dd4y+k0pMyNw@mail.gmail.com>
2023-09-01 20:01 ` Ruiwen Zhao
2023-09-02 9:52 ` Amir Goldstein
2023-09-03 17:32 ` Amir Goldstein [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=CAOQ4uxhJ5jw+mY_VJeEOXR7-xauFSY+GkTNnrr+N0nqY8dFPZQ@mail.gmail.com \
--to=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=djwong@kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=msheinin@google.com \
--cc=ruiwen@google.com \
--cc=skanzhelev@google.com \
--cc=tytso@mit.edu \
/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).