On Mon, Sep 4, 2023 at 11:57 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Mon, Sep 4, 2023 at 9:40 PM Ruiwen Zhao <ruiwen@xxxxxxxxxx> wrote: > > > > On Mon, Sep 4, 2023 at 6:44 AM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > > > > > On Mon, 4 Sept 2023 at 15:24, Amir Goldstein <amir73il@xxxxxxxxx> 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@xxxxxxxxxx> > > > > Link: https://lore.kernel.org/r/CAKd=y5Hpg7J2gxrFT02F94o=FM9QvGp=kcH1Grctx8HzFYvpiA@xxxxxxxxxxxxxx/ > > > > Fixes: 72db82115d2b ("ovl: copy up sync/noatime fileattr flags") > > > > Cc: <stable@xxxxxxxxxxxxxxx> # v5.15 > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > > > --- > > > > > > > > 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@xxxxxxxxxx> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217850 > Fixes: 72db82115d2b ("ovl: copy up sync/noatime fileattr flags") > Cc: <stable@xxxxxxxxxxxxxxx> # 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
