From 1f0cb8bcc7f96bcd816c80618eb0a7a361c70fbd Mon Sep 17 00:00:00 2001 From: Sargun Dhillon Date: Sun, 29 Nov 2020 19:00:39 -0800 Subject: ovl: plumb through flush method Filesystems can implement their own flush method that release resources, or manipulate caches. Currently if one of these filesystems is used with overlayfs, the flush method is not called. [Amir: fix fd leak in ovl_flush()] Signed-off-by: Sargun Dhillon Signed-off-by: Miklos Szeredi --- fs/overlayfs/file.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index dbfb35fb0ff7..6e454a294046 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -686,6 +686,26 @@ static loff_t ovl_remap_file_range(struct file *file_in, loff_t pos_in, remap_flags, op); } +static int ovl_flush(struct file *file, fl_owner_t id) +{ + struct fd real; + const struct cred *old_cred; + int err; + + err = ovl_real_fdget(file, &real); + if (err) + return err; + + if (real.file->f_op->flush) { + old_cred = ovl_override_creds(file_inode(file)->i_sb); + err = real.file->f_op->flush(real.file, id); + revert_creds(old_cred); + } + fdput(real); + + return err; +} + const struct file_operations ovl_file_operations = { .open = ovl_open, .release = ovl_release, @@ -697,6 +717,7 @@ const struct file_operations ovl_file_operations = { .fallocate = ovl_fallocate, .fadvise = ovl_fadvise, .unlocked_ioctl = ovl_ioctl, + .flush = ovl_flush, #ifdef CONFIG_COMPAT .compat_ioctl = ovl_compat_ioctl, #endif -- cgit From e21a6c57e3905313664aa012727346a0067effd5 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Thu, 8 Apr 2021 14:30:20 +0300 Subject: ovl: check that upperdir path is not on a read-only mount So far we only checked that sb is not read-only. Suggested-by: Christian Brauner Signed-off-by: Amir Goldstein Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index fdd72f1a9c5e..8d8366350093 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1185,8 +1185,8 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs, if (err) goto out; - /* Upper fs should not be r/o */ - if (sb_rdonly(upperpath->mnt->mnt_sb)) { + /* Upperdir path should not be r/o */ + if (__mnt_is_readonly(upperpath->mnt)) { pr_err("upper fs is r/o, try multi-lower layers mount\n"); err = -EINVAL; goto out; -- cgit From b0e0f69731cde2de09a45c9a7a881378e7dbc4ba Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Tue, 9 Mar 2021 18:26:54 +0200 Subject: ovl: restrict lower null uuid for "xino=auto" Commit a888db310195 ("ovl: fix regression with re-formatted lower squashfs") attempted to fix a regression with existing setups that use a practice that we are trying to discourage. The discourage part was described this way in the commit message: "To avoid the reported regression while still allowing the new features with single lower squashfs, do not allow decoding origin with lower null uuid unless user opted-in to one of the new features that require following the lower inode of non-dir upper (index, xino, metacopy)." The three mentioned features are disabled by default in Kconfig, so it was assumed that if they are enabled, the user opted-in for them. Apparently, distros started to configure CONFIG_OVERLAY_FS_XINO_AUTO=y some time ago, so users upgrading their kernels can still be affected by said regression even though they never opted-in for any new feature. To fix this, treat "xino=on" as "user opted-in", but not "xino=auto". Since we are changing the behavior of "xino=auto" to no longer follow to lower origin with null uuid, take this one step further and disable xino in that corner case. To be consistent, disable xino also in cases of lower fs without file handle support and upper fs without xattr support. Update documentation w.r.t the new "xino=auto" behavior and fix the out dated bits of documentation regarding "xino" and regarding offline modifications to lower layers. Link: https://lore.kernel.org/linux-unionfs/b36a429d7c563730c28d763d4d57a6fc30508a4f.1615216996.git.kevin@kevinlocke.name/ Signed-off-by: Amir Goldstein Signed-off-by: Miklos Szeredi --- Documentation/filesystems/overlayfs.rst | 26 ++++++++++----------- fs/overlayfs/super.c | 41 ++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst index 78240e29b0bb..455ca86eb4fc 100644 --- a/Documentation/filesystems/overlayfs.rst +++ b/Documentation/filesystems/overlayfs.rst @@ -40,17 +40,17 @@ On 64bit systems, even if all overlay layers are not on the same underlying filesystem, the same compliant behavior could be achieved with the "xino" feature. The "xino" feature composes a unique object identifier from the real object st_ino and an underlying fsid index. - -If all underlying filesystems support NFS file handles and export file -handles with 32bit inode number encoding (e.g. ext4), overlay filesystem -will use the high inode number bits for fsid. Even when the underlying -filesystem uses 64bit inode numbers, users can still enable the "xino" -feature with the "-o xino=on" overlay mount option. That is useful for the -case of underlying filesystems like xfs and tmpfs, which use 64bit inode -numbers, but are very unlikely to use the high inode number bits. In case +The "xino" feature uses the high inode number bits for fsid, because the +underlying filesystems rarely use the high inode number bits. In case the underlying inode number does overflow into the high xino bits, overlay filesystem will fall back to the non xino behavior for that inode. +The "xino" feature can be enabled with the "-o xino=on" overlay mount option. +If all underlying filesystems support NFS file handles, the value of st_ino +for overlay filesystem objects is not only unique, but also persistent over +the lifetime of the filesystem. The "-o xino=auto" overlay mount option +enables the "xino" feature only if the persistent st_ino requirement is met. + The following table summarizes what can be expected in different overlay configurations. @@ -66,14 +66,13 @@ Inode properties | All layers | Y | Y | Y | Y | Y | Y | Y | Y | | on same fs | | | | | | | | | +--------------+-----+------+-----+------+--------+--------+--------+-------+ -| Layers not | N | Y | Y | N | N | Y | N | Y | +| Layers not | N | N | Y | N | N | Y | N | Y | | on same fs, | | | | | | | | | | xino=off | | | | | | | | | +--------------+-----+------+-----+------+--------+--------+--------+-------+ | xino=on/auto | Y | Y | Y | Y | Y | Y | Y | Y | -| | | | | | | | | | +--------------+-----+------+-----+------+--------+--------+--------+-------+ -| xino=on/auto,| N | Y | Y | N | N | Y | N | Y | +| xino=on/auto,| N | N | Y | N | N | Y | N | Y | | ino overflow | | | | | | | | | +--------------+-----+------+-----+------+--------+--------+--------+-------+ @@ -81,7 +80,6 @@ Inode properties /proc files, such as /proc/locks and /proc/self/fdinfo/ of an inotify file descriptor. - Upper and Lower --------------- @@ -461,7 +459,7 @@ enough free bits in the inode number, then overlayfs will not be able to guarantee that the values of st_ino and st_dev returned by stat(2) and the value of d_ino returned by readdir(3) will act like on a normal filesystem. E.g. the value of st_dev may be different for two objects in the same -overlay filesystem and the value of st_ino for directory objects may not be +overlay filesystem and the value of st_ino for filesystem objects may not be persistent and could change even while the overlay filesystem is mounted, as summarized in the `Inode properties`_ table above. @@ -476,7 +474,7 @@ a crash or deadlock. Offline changes, when the overlay is not mounted, are allowed to the upper tree. Offline changes to the lower tree are only allowed if the -"metadata only copy up", "inode index", and "redirect_dir" features +"metadata only copy up", "inode index", "xino" and "redirect_dir" features have not been used. If the lower tree is modified and any of these features has been used, the behavior of the overlay is undefined, though it will not result in a crash or deadlock. diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 8d8366350093..5fcaf3acc350 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -945,6 +945,16 @@ static int ovl_lower_dir(const char *name, struct path *path, pr_warn("fs on '%s' does not support file handles, falling back to index=off,nfs_export=off.\n", name); } + /* + * Decoding origin file handle is required for persistent st_ino. + * Without persistent st_ino, xino=auto falls back to xino=off. + */ + if (ofs->config.xino == OVL_XINO_AUTO && + ofs->config.upperdir && !fh_type) { + ofs->config.xino = OVL_XINO_OFF; + pr_warn("fs on '%s' does not support file handles, falling back to xino=off.\n", + name); + } /* Check if lower fs has 32bit inode numbers */ if (fh_type != FILEID_INO32_GEN) @@ -1401,9 +1411,19 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs, err = ovl_do_setxattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE, "0", 1); if (err) { ofs->noxattr = true; - ofs->config.index = false; - ofs->config.metacopy = false; - pr_warn("upper fs does not support xattr, falling back to index=off and metacopy=off.\n"); + if (ofs->config.index || ofs->config.metacopy) { + ofs->config.index = false; + ofs->config.metacopy = false; + pr_warn("upper fs does not support xattr, falling back to index=off,metacopy=off.\n"); + } + /* + * xattr support is required for persistent st_ino. + * Without persistent st_ino, xino=auto falls back to xino=off. + */ + if (ofs->config.xino == OVL_XINO_AUTO) { + ofs->config.xino = OVL_XINO_OFF; + pr_warn("upper fs does not support xattr, falling back to xino=off.\n"); + } err = 0; } else { ovl_do_removexattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE); @@ -1580,7 +1600,8 @@ static bool ovl_lower_uuid_ok(struct ovl_fs *ofs, const uuid_t *uuid) * user opted-in to one of the new features that require following the * lower inode of non-dir upper. */ - if (!ofs->config.index && !ofs->config.metacopy && !ofs->config.xino && + if (!ofs->config.index && !ofs->config.metacopy && + ofs->config.xino != OVL_XINO_ON && uuid_is_null(uuid)) return false; @@ -1609,6 +1630,7 @@ static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path) dev_t dev; int err; bool bad_uuid = false; + bool warn = false; for (i = 0; i < ofs->numfs; i++) { if (ofs->fs[i].sb == sb) @@ -1617,13 +1639,20 @@ static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path) if (!ovl_lower_uuid_ok(ofs, &sb->s_uuid)) { bad_uuid = true; + if (ofs->config.xino == OVL_XINO_AUTO) { + ofs->config.xino = OVL_XINO_OFF; + warn = true; + } if (ofs->config.index || ofs->config.nfs_export) { ofs->config.index = false; ofs->config.nfs_export = false; - pr_warn("%s uuid detected in lower fs '%pd2', falling back to index=off,nfs_export=off.\n", + warn = true; + } + if (warn) { + pr_warn("%s uuid detected in lower fs '%pd2', falling back to xino=%s,index=off,nfs_export=off.\n", uuid_is_null(&sb->s_uuid) ? "null" : "conflicting", - path->dentry); + path->dentry, ovl_xino_str[ofs->config.xino]); } } -- cgit From eaab1d45cdb4bb0c846bd23c3d666d5b90af7b41 Mon Sep 17 00:00:00 2001 From: Mickaël Salaün Date: Mon, 29 Mar 2021 18:49:07 +0200 Subject: ovl: fix leaked dentry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()"), overlayfs doesn't put temporary dentry when there is a metacopy error, which leads to dentry leaks when shutting down the related superblock: overlayfs: refusing to follow metacopy origin for (/file0) ... BUG: Dentry (____ptrval____){i=3f33,n=file3} still in use (1) [unmount of overlay overlay] ... WARNING: CPU: 1 PID: 432 at umount_check.cold+0x107/0x14d CPU: 1 PID: 432 Comm: unmount-overlay Not tainted 5.12.0-rc5 #1 ... RIP: 0010:umount_check.cold+0x107/0x14d ... Call Trace: d_walk+0x28c/0x950 ? dentry_lru_isolate+0x2b0/0x2b0 ? __kasan_slab_free+0x12/0x20 do_one_tree+0x33/0x60 shrink_dcache_for_umount+0x78/0x1d0 generic_shutdown_super+0x70/0x440 kill_anon_super+0x3e/0x70 deactivate_locked_super+0xc4/0x160 deactivate_super+0xfa/0x140 cleanup_mnt+0x22e/0x370 __cleanup_mnt+0x1a/0x30 task_work_run+0x139/0x210 do_exit+0xb0c/0x2820 ? __kasan_check_read+0x1d/0x30 ? find_held_lock+0x35/0x160 ? lock_release+0x1b6/0x660 ? mm_update_next_owner+0xa20/0xa20 ? reacquire_held_locks+0x3f0/0x3f0 ? __sanitizer_cov_trace_const_cmp4+0x22/0x30 do_group_exit+0x135/0x380 __do_sys_exit_group.isra.0+0x20/0x20 __x64_sys_exit_group+0x3c/0x50 do_syscall_64+0x45/0x70 entry_SYSCALL_64_after_hwframe+0x44/0xae ... VFS: Busy inodes after unmount of overlay. Self-destruct in 5 seconds. Have a nice day... This fix has been tested with a syzkaller reproducer. Cc: Amir Goldstein Cc: # v5.8+ Reported-by: syzbot Fixes: 6815f479ca90 ("ovl: use only uppermetacopy state in ovl_lookup()") Signed-off-by: Mickaël Salaün Link: https://lore.kernel.org/r/20210329164907.2133175-1-mic@digikod.net Reviewed-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/namei.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 3fe05fb5d145..71e264e2f16b 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -919,6 +919,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, continue; if ((uppermetacopy || d.metacopy) && !ofs->config.metacopy) { + dput(this); err = -EPERM; pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", dentry); goto out_put; -- cgit From 7b279bbfd2b230c7a210ff8f405799c7e46bbf48 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 23 Mar 2021 16:19:35 +0300 Subject: ovl: fix missing revert_creds() on error path Smatch complains about missing that the ovl_override_creds() doesn't have a matching revert_creds() if the dentry is disconnected. Fix this by moving the ovl_override_creds() until after the disconnected check. Fixes: aa3ff3c152ff ("ovl: copy up of disconnected dentries") Signed-off-by: Dan Carpenter Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 0b2891c6c71e..2846b943e80c 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -932,7 +932,7 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, static int ovl_copy_up_flags(struct dentry *dentry, int flags) { int err = 0; - const struct cred *old_cred = ovl_override_creds(dentry->d_sb); + const struct cred *old_cred; bool disconnected = (dentry->d_flags & DCACHE_DISCONNECTED); /* @@ -943,6 +943,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags) if (WARN_ON(disconnected && d_is_dir(dentry))) return -EIO; + old_cred = ovl_override_creds(dentry->d_sb); while (!err) { struct dentry *next; struct dentry *parent = NULL; -- cgit From d7b49b10d5a92f22333a3800dfae89ea0822751b Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 1 Mar 2021 14:19:30 +0800 Subject: ovl: fix error for ovl_fill_super() There are some places should return -EINVAL instead of -ENOMEM in ovl_fill_super(). [Amir] Consistently set error before checking the error condition. Signed-off-by: Chengguang Xu Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 5fcaf3acc350..3f6333b32797 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1981,6 +1981,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) if (!ofs) goto out; + err = -ENOMEM; ofs->creator_cred = cred = prepare_creds(); if (!cred) goto out_err; @@ -2009,6 +2010,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) if (!splitlower) goto out_err; + err = -EINVAL; numlower = ovl_split_lowerdirs(splitlower); if (numlower > OVL_MAX_STACK) { pr_err("too many lower directories, limit is %d\n", @@ -2016,6 +2018,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) goto out_err; } + err = -ENOMEM; layers = kcalloc(numlower + 1, sizeof(struct ovl_layer), GFP_KERNEL); if (!layers) goto out_err; @@ -2042,6 +2045,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) if (ofs->config.upperdir) { struct super_block *upper_sb; + err = -EINVAL; if (!ofs->config.workdir) { pr_err("missing 'workdir'\n"); goto out_err; -- cgit From c68e7ec53a53903aff4cb94ba35c58486008983d Mon Sep 17 00:00:00 2001 From: youngjun Date: Mon, 22 Feb 2021 08:59:21 -0800 Subject: ovl: remove ovl_map_dev_ino() return value ovl_map_dev_ino() always returns success. Remove unnecessary return value. Signed-off-by: youngjun Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 003cf83bf78a..8f3c79a0ab3c 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -95,7 +95,7 @@ out: return err; } -static int ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid) +static void ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid) { bool samefs = ovl_same_fs(dentry->d_sb); unsigned int xinobits = ovl_xino_bits(dentry->d_sb); @@ -108,7 +108,7 @@ static int ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid) * which is friendly to du -x. */ stat->dev = dentry->d_sb->s_dev; - return 0; + return; } else if (xinobits) { /* * All inode numbers of underlying fs should not be using the @@ -122,7 +122,7 @@ static int ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid) if (likely(!(stat->ino >> xinoshift))) { stat->ino |= ((u64)fsid) << (xinoshift + 1); stat->dev = dentry->d_sb->s_dev; - return 0; + return; } else if (ovl_xino_warn(dentry->d_sb)) { pr_warn_ratelimited("inode number too big (%pd2, ino=%llu, xinobits=%d)\n", dentry, stat->ino, xinobits); @@ -151,8 +151,6 @@ static int ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid) */ stat->dev = OVL_FS(dentry->d_sb)->fs[fsid].pseudo_dev; } - - return 0; } int ovl_getattr(struct user_namespace *mnt_userns, const struct path *path, @@ -251,9 +249,7 @@ int ovl_getattr(struct user_namespace *mnt_userns, const struct path *path, } } - err = ovl_map_dev_ino(dentry, stat, fsid); - if (err) - goto out; + ovl_map_dev_ino(dentry, stat, fsid); /* * It's probably not worth it to count subdirs to get the -- cgit From 568edee485a4b4f138eb8cea9e532b3fb6fdd5fe Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Fri, 26 Feb 2021 17:24:17 +0800 Subject: ovl: do not copy attr several times In ovl_xattr_set() we have already copied attr of real inode so no need to copy it again in ovl_posix_acl_xattr_set(). Signed-off-by: Chengguang Xu Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 3f6333b32797..4a60bcc0eea3 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1052,9 +1052,6 @@ ovl_posix_acl_xattr_set(const struct xattr_handler *handler, } err = ovl_xattr_set(dentry, inode, handler->name, value, size, flags); - if (!err) - ovl_copyattr(ovl_inode_real(inode), inode); - return err; out_acl_release: -- cgit From 597534e7bcfa6af175264885b8e044d4a1ed8d57 Mon Sep 17 00:00:00 2001 From: Xiong Zhenwu Date: Thu, 18 Mar 2021 04:30:27 -0700 Subject: ovl: fix misspellings using codespell tool A typo is found out by codespell tool: $ codespell ./fs/overlayfs/ ./fs/overlayfs/util.c:217: dependig ==> depending Fix a typo found by codespell. Signed-off-by: Xiong Zhenwu Signed-off-by: Miklos Szeredi --- fs/overlayfs/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 7f5a01a11f97..2544694d40b9 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -214,7 +214,7 @@ const struct ovl_layer *ovl_layer_lower(struct dentry *dentry) /* * ovl_dentry_lower() could return either a data dentry or metacopy dentry - * dependig on what is stored in lowerstack[0]. At times we need to find + * depending on what is stored in lowerstack[0]. At times we need to find * lower dentry which has data (and not metacopy dentry). This helper * returns the lower data dentry. */ -- cgit From f48bbfb20e1f96f6ada1fe8c62fb9072fb4c6c88 Mon Sep 17 00:00:00 2001 From: Bhaskar Chowdhury Date: Sat, 13 Mar 2021 09:00:23 +0530 Subject: ovl: trivial typo fixes in the file inode.c s/peresistent/persistent/ s/xatts/xattrs/ s/annotaion/annotation/ Signed-off-by: Bhaskar Chowdhury Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 8f3c79a0ab3c..28c71978eb2e 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -115,7 +115,7 @@ static void ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid) * high xinobits, so we use high xinobits to partition the * overlay st_ino address space. The high bits holds the fsid * (upper fsid is 0). The lowest xinobit is reserved for mapping - * the non-peresistent inode numbers range in case of overflow. + * the non-persistent inode numbers range in case of overflow. * This way all overlay inode numbers are unique and use the * overlay st_dev. */ @@ -404,7 +404,7 @@ static bool ovl_can_list(struct super_block *sb, const char *s) if (ovl_is_private_xattr(sb, s)) return false; - /* List all non-trusted xatts */ + /* List all non-trusted xattrs */ if (strncmp(s, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) != 0) return true; @@ -534,7 +534,7 @@ static const struct address_space_operations ovl_aops = { * stackable i_mutex locks according to stack level of the super * block instance. An overlayfs instance can never be in stack * depth 0 (there is always a real fs below it). An overlayfs - * inode lock will use the lockdep annotaion ovl_i_mutex_key[depth]. + * inode lock will use the lockdep annotation ovl_i_mutex_key[depth]. * * For example, here is a snip from /proc/lockdep_chains after * dir_iterate of nested overlayfs: -- cgit From 321b46b904816241044e177c1d6282ad20f17416 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 4 Mar 2021 17:45:15 +0100 Subject: ovl: show "userxattr" in the mount data This was missed when adding the option. Signed-off-by: Giuseppe Scrivano Reviewed-by: Vivek Goyal Fixes: 2d2f2d7322ff ("ovl: user xattr") Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 4a60bcc0eea3..a33b31bf7e05 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -380,6 +380,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry) ofs->config.metacopy ? "on" : "off"); if (ofs->config.ovl_volatile) seq_puts(m, ",volatile"); + if (ofs->config.userxattr) + seq_puts(m, ",userxattr"); return 0; } -- cgit From 708fa01597fa002599756bf56a96d0de1677375c Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Mon, 12 Apr 2021 12:00:37 +0200 Subject: ovl: allow upperdir inside lowerdir Commit 146d62e5a586 ("ovl: detect overlapping layers") made sure we don't have overlapping layers, but it also broke the arguably valid use case of mount -olowerdir=/,upperdir=/subdir,.. where upperdir overlaps lowerdir on the same filesystem. This has been causing regressions. Revert the check, but only for the specific case where upperdir and/or workdir are subdirectories of lowerdir. Any other overlap (e.g. lowerdir is subdirectory of upperdir, etc) case is crazy, so leave the check in place for those. Overlaps are detected at lookup time too, so reverting the mount time check should be safe. Fixes: 146d62e5a586 ("ovl: detect overlapping layers") Cc: # v5.2 Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index a33b31bf7e05..b01d4147520d 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1854,7 +1854,8 @@ out_err: * - upper/work dir of any overlayfs instance */ static int ovl_check_layer(struct super_block *sb, struct ovl_fs *ofs, - struct dentry *dentry, const char *name) + struct dentry *dentry, const char *name, + bool is_lower) { struct dentry *next = dentry, *parent; int err = 0; @@ -1866,7 +1867,7 @@ static int ovl_check_layer(struct super_block *sb, struct ovl_fs *ofs, /* Walk back ancestors to root (inclusive) looking for traps */ while (!err && parent != next) { - if (ovl_lookup_trap_inode(sb, parent)) { + if (is_lower && ovl_lookup_trap_inode(sb, parent)) { err = -ELOOP; pr_err("overlapping %s path\n", name); } else if (ovl_is_inuse(parent)) { @@ -1892,7 +1893,7 @@ static int ovl_check_overlapping_layers(struct super_block *sb, if (ovl_upper_mnt(ofs)) { err = ovl_check_layer(sb, ofs, ovl_upper_mnt(ofs)->mnt_root, - "upperdir"); + "upperdir", false); if (err) return err; @@ -1903,7 +1904,8 @@ static int ovl_check_overlapping_layers(struct super_block *sb, * workbasedir. In that case, we already have their traps in * inode cache and we will catch that case on lookup. */ - err = ovl_check_layer(sb, ofs, ofs->workbasedir, "workdir"); + err = ovl_check_layer(sb, ofs, ofs->workbasedir, "workdir", + false); if (err) return err; } @@ -1911,7 +1913,7 @@ static int ovl_check_overlapping_layers(struct super_block *sb, for (i = 1; i < ofs->numlayer; i++) { err = ovl_check_layer(sb, ofs, ofs->layers[i].mnt->mnt_root, - "lowerdir"); + "lowerdir", true); if (err) return err; } -- cgit From 65cd913ec9d9d71529665924c81015b7ab7d9381 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 11 Apr 2021 12:22:23 +0300 Subject: ovl: invalidate readdir cache on changes to dir with origin The test in ovl_dentry_version_inc() was out-dated and did not include the case where readdir cache is used on a non-merge dir that has origin xattr, indicating that it may contain leftover whiteouts. To make the code more robust, use the same helper ovl_dir_is_real() to determine if readdir cache should be used and if readdir cache should be invalidated. Fixes: b79e05aaa166 ("ovl: no direct iteration for dir with origin xattr") Link: https://lore.kernel.org/linux-unionfs/CAOQ4uxht70nODhNHNwGFMSqDyOKLXOKrY0H6g849os4BQ7cokA@mail.gmail.com/ Cc: Chris Murphy Signed-off-by: Amir Goldstein Signed-off-by: Miklos Szeredi --- fs/overlayfs/overlayfs.h | 30 +++++++++++++++++++++++++++--- fs/overlayfs/readdir.c | 12 ------------ fs/overlayfs/util.c | 31 +++++++++---------------------- 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 95cff83786a5..2322f854533c 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -319,9 +319,6 @@ int ovl_check_setxattr(struct dentry *dentry, struct dentry *upperdentry, enum ovl_xattr ox, const void *value, size_t size, int xerr); int ovl_set_impure(struct dentry *dentry, struct dentry *upperdentry); -void ovl_set_flag(unsigned long flag, struct inode *inode); -void ovl_clear_flag(unsigned long flag, struct inode *inode); -bool ovl_test_flag(unsigned long flag, struct inode *inode); bool ovl_inuse_trylock(struct dentry *dentry); void ovl_inuse_unlock(struct dentry *dentry); bool ovl_is_inuse(struct dentry *dentry); @@ -335,6 +332,21 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry, int padding); int ovl_sync_status(struct ovl_fs *ofs); +static inline void ovl_set_flag(unsigned long flag, struct inode *inode) +{ + set_bit(flag, &OVL_I(inode)->flags); +} + +static inline void ovl_clear_flag(unsigned long flag, struct inode *inode) +{ + clear_bit(flag, &OVL_I(inode)->flags); +} + +static inline bool ovl_test_flag(unsigned long flag, struct inode *inode) +{ + return test_bit(flag, &OVL_I(inode)->flags); +} + static inline bool ovl_is_impuredir(struct super_block *sb, struct dentry *dentry) { @@ -439,6 +451,18 @@ int ovl_workdir_cleanup(struct inode *dir, struct vfsmount *mnt, struct dentry *dentry, int level); int ovl_indexdir_cleanup(struct ovl_fs *ofs); +/* + * Can we iterate real dir directly? + * + * Non-merge dir may contain whiteouts from a time it was a merge upper, before + * lower dir was removed under it and possibly before it was rotated from upper + * to lower layer. + */ +static inline bool ovl_dir_is_real(struct dentry *dir) +{ + return !ovl_test_flag(OVL_WHITEOUTS, d_inode(dir)); +} + /* inode.c */ int ovl_set_nlink_upper(struct dentry *dentry); int ovl_set_nlink_lower(struct dentry *dentry); diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index f404a78e6b60..cc1e80257064 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -319,18 +319,6 @@ static inline int ovl_dir_read(struct path *realpath, return err; } -/* - * Can we iterate real dir directly? - * - * Non-merge dir may contain whiteouts from a time it was a merge upper, before - * lower dir was removed under it and possibly before it was rotated from upper - * to lower layer. - */ -static bool ovl_dir_is_real(struct dentry *dir) -{ - return !ovl_test_flag(OVL_WHITEOUTS, d_inode(dir)); -} - static void ovl_dir_reset(struct file *file) { struct ovl_dir_file *od = file->private_data; diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 2544694d40b9..b9d03627f364 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -422,18 +422,20 @@ void ovl_inode_update(struct inode *inode, struct dentry *upperdentry) } } -static void ovl_dentry_version_inc(struct dentry *dentry, bool impurity) +static void ovl_dir_version_inc(struct dentry *dentry, bool impurity) { struct inode *inode = d_inode(dentry); WARN_ON(!inode_is_locked(inode)); + WARN_ON(!d_is_dir(dentry)); /* - * Version is used by readdir code to keep cache consistent. For merge - * dirs all changes need to be noted. For non-merge dirs, cache only - * contains impure (ones which have been copied up and have origins) - * entries, so only need to note changes to impure entries. + * Version is used by readdir code to keep cache consistent. + * For merge dirs (or dirs with origin) all changes need to be noted. + * For non-merge dirs, cache contains only impure entries (i.e. ones + * which have been copied up and have origins), so only need to note + * changes to impure entries. */ - if (OVL_TYPE_MERGE(ovl_path_type(dentry)) || impurity) + if (!ovl_dir_is_real(dentry) || impurity) OVL_I(inode)->version++; } @@ -442,7 +444,7 @@ void ovl_dir_modified(struct dentry *dentry, bool impurity) /* Copy mtime/ctime */ ovl_copyattr(d_inode(ovl_dentry_upper(dentry)), d_inode(dentry)); - ovl_dentry_version_inc(dentry, impurity); + ovl_dir_version_inc(dentry, impurity); } u64 ovl_dentry_version_get(struct dentry *dentry) @@ -638,21 +640,6 @@ int ovl_set_impure(struct dentry *dentry, struct dentry *upperdentry) return err; } -void ovl_set_flag(unsigned long flag, struct inode *inode) -{ - set_bit(flag, &OVL_I(inode)->flags); -} - -void ovl_clear_flag(unsigned long flag, struct inode *inode) -{ - clear_bit(flag, &OVL_I(inode)->flags); -} - -bool ovl_test_flag(unsigned long flag, struct inode *inode) -{ - return test_bit(flag, &OVL_I(inode)->flags); -} - /** * Caller must hold a reference to inode to prevent it from being freed while * it is marked inuse. -- cgit From 5e717c6fa41ff9b9b0c1e5959ccf5d8ef42f804b Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sat, 10 Apr 2021 12:17:50 +0300 Subject: ovl: add debug print to ovl_do_getxattr() It was the only ovl_do helper missing it. Signed-off-by: Amir Goldstein Signed-off-by: Miklos Szeredi --- fs/overlayfs/overlayfs.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 2322f854533c..d1e08d804207 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -186,7 +186,12 @@ static inline ssize_t ovl_do_getxattr(struct ovl_fs *ofs, struct dentry *dentry, size_t size) { const char *name = ovl_xattr(ofs, ox); - return vfs_getxattr(&init_user_ns, dentry, name, value, size); + int err = vfs_getxattr(&init_user_ns, dentry, name, value, size); + int len = (value && err > 0) ? err : 0; + + pr_debug("getxattr(%pd2, \"%s\", \"%*pE\", %zu, 0) = %i\n", + dentry, name, min(len, 48), value, size, err); + return err; } static inline int ovl_do_setxattr(struct ovl_fs *ofs, struct dentry *dentry, -- cgit