From 5b0a414d06c3ed2097e32ef7944a4abb644b89bd Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Thu, 4 Nov 2021 14:04:52 +0100 Subject: ovl: fix filattr copy-up failure This regression can be reproduced with ntfs-3g and overlayfs: mkdir lower upper work overlay dd if=/dev/zero of=ntfs.raw bs=1M count=2 mkntfs -F ntfs.raw mount ntfs.raw lower touch lower/file.txt mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work - overlay mv overlay/file.txt overlay/file2.txt mv fails and (misleadingly) prints mv: cannot move 'overlay/file.txt' to a subdirectory of itself, 'overlay/file2.txt' The reason is that ovl_copy_fileattr() is triggered due to S_NOATIME being set on all inodes (by fuse) regardless of fileattr. ovl_copy_fileattr() tries to retrieve file attributes from lower file, but that fails because filesystem does not support this ioctl (this should fail with ENOTTY, but ntfs-3g return EINVAL instead). This failure is propagated to origial operation (in this case rename) that triggered the copy-up. The fix is to ignore ENOTTY and EINVAL errors from fileattr_get() in copy up. This also requires turning the internal ENOIOCTLCMD into ENOTTY. As a further measure to prevent unnecessary failures, only try the fileattr_get/set on upper if there are any flags to copy up. Side note: a number of filesystems set S_NOATIME (and sometimes other inode flags) irrespective of fileattr flags. This causes unnecessary calls during copy up, which might lead to a performance issue, especially if latency is high. To fix this, the kernel would need to differentiate between the two cases. E.g. introduce SB_NOATIME_UPDATE, a per-sb variant of S_NOATIME. SB_NOATIME doesn't work, because that's interpreted as "filesystem doesn't store an atime attribute" Reported-and-tested-by: Kevin Locke Fixes: 72db82115d2b ("ovl: copy up sync/noatime fileattr flags") Cc: # v5.15 Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'fs/overlayfs/copy_up.c') diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 4e7d5bfa2949..b193d08a3dc3 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -140,12 +140,14 @@ static int ovl_copy_fileattr(struct inode *inode, struct path *old, int err; err = ovl_real_fileattr_get(old, &oldfa); - if (err) - return err; - - err = ovl_real_fileattr_get(new, &newfa); - if (err) + if (err) { + /* Ntfs-3g returns -EINVAL for "no fileattr support" */ + if (err == -ENOTTY || err == -EINVAL) + return 0; + pr_warn("failed to retrieve lower fileattr (%pd2, err=%i)\n", + old, err); return err; + } /* * We cannot set immutable and append-only flags on upper inode, @@ -159,6 +161,17 @@ static int ovl_copy_fileattr(struct inode *inode, struct path *old, return err; } + /* Don't bother copying flags if none are set */ + if (!(oldfa.flags & OVL_COPY_FS_FLAGS_MASK)) + return 0; + + err = ovl_real_fileattr_get(new, &newfa); + if (err) { + pr_warn("failed to retrieve upper fileattr (%pd2, err=%i)\n", + new, err); + return err; + } + BUILD_BUG_ON(OVL_COPY_FS_FLAGS_MASK & ~FS_COMMON_FL); newfa.flags &= ~OVL_COPY_FS_FLAGS_MASK; newfa.flags |= (oldfa.flags & OVL_COPY_FS_FLAGS_MASK); -- cgit v1.2.3