From 02d70090e0e020eff440dbe51e93fe2fc94d9835 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 19 Nov 2023 20:55:00 +0200 Subject: ovl: remove redundant ofs->indexdir member When the index feature is disabled, ofs->indexdir is NULL. When the index feature is enabled, ofs->indexdir has the same value as ofs->workdir and takes an extra reference. This makes the code harder to understand when it is not always clear that ofs->indexdir in one function is the same dentry as ofs->workdir in another function. Remove this redundancy, by referencing ofs->workdir directly in index helpers and by using the ovl_indexdir() accessor in generic code. Signed-off-by: Amir Goldstein --- fs/overlayfs/export.c | 4 ++-- fs/overlayfs/namei.c | 4 ++-- fs/overlayfs/ovl_entry.h | 5 +---- fs/overlayfs/params.c | 2 -- fs/overlayfs/readdir.c | 2 +- fs/overlayfs/super.c | 19 ++++++++----------- fs/overlayfs/util.c | 2 +- 7 files changed, 15 insertions(+), 23 deletions(-) (limited to 'fs/overlayfs') diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c index 7e16bbcad95e..6909c4a5da56 100644 --- a/fs/overlayfs/export.c +++ b/fs/overlayfs/export.c @@ -460,7 +460,7 @@ static struct dentry *ovl_lookup_real_inode(struct super_block *sb, * For decoded lower dir file handle, lookup index by origin to check * if lower dir was copied up and and/or removed. */ - if (!this && layer->idx && ofs->indexdir && !WARN_ON(!d_is_dir(real))) { + if (!this && layer->idx && ovl_indexdir(sb) && !WARN_ON(!d_is_dir(real))) { index = ovl_lookup_index(ofs, NULL, real, false); if (IS_ERR(index)) return index; @@ -733,7 +733,7 @@ static struct dentry *ovl_lower_fh_to_d(struct super_block *sb, } /* Then lookup indexed upper/whiteout by origin fh */ - if (ofs->indexdir) { + if (ovl_indexdir(sb)) { index = ovl_get_index_fh(ofs, fh); err = PTR_ERR(index); if (IS_ERR(index)) { diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 03bc8d5dfa31..984ffdaeed6c 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -754,7 +754,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh) if (err) return ERR_PTR(err); - index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len); + index = lookup_positive_unlocked(name.name, ofs->workdir, name.len); kfree(name.name); if (IS_ERR(index)) { if (PTR_ERR(index) == -ENOENT) @@ -787,7 +787,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper, return ERR_PTR(err); index = lookup_one_positive_unlocked(ovl_upper_mnt_idmap(ofs), name.name, - ofs->indexdir, name.len); + ofs->workdir, name.len); if (IS_ERR(index)) { err = PTR_ERR(index); if (err == -ENOENT) { diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h index d82d2a043da2..5fa9c58af65f 100644 --- a/fs/overlayfs/ovl_entry.h +++ b/fs/overlayfs/ovl_entry.h @@ -63,10 +63,8 @@ struct ovl_fs { struct ovl_sb *fs; /* workbasedir is the path at workdir= mount option */ struct dentry *workbasedir; - /* workdir is the 'work' directory under workbasedir */ + /* workdir is the 'work' or 'index' directory under workbasedir */ struct dentry *workdir; - /* index directory listing overlay inodes by origin file handle */ - struct dentry *indexdir; long namelen; /* pathnames of lower and upper dirs, for show_options */ struct ovl_config config; @@ -81,7 +79,6 @@ struct ovl_fs { /* Traps in ovl inode cache */ struct inode *workbasedir_trap; struct inode *workdir_trap; - struct inode *indexdir_trap; /* -1: disabled, 0: same fs, 1..32: number of unused ino bits */ int xino_mode; /* For allocation of non-persistent inode numbers */ diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c index 3fe2dde1598f..112b4b12f825 100644 --- a/fs/overlayfs/params.c +++ b/fs/overlayfs/params.c @@ -743,10 +743,8 @@ void ovl_free_fs(struct ovl_fs *ofs) unsigned i; iput(ofs->workbasedir_trap); - iput(ofs->indexdir_trap); iput(ofs->workdir_trap); dput(ofs->whiteout); - dput(ofs->indexdir); dput(ofs->workdir); if (ofs->workdir_locked) ovl_inuse_unlock(ofs->workbasedir); diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index a490fc47c3e7..e71156baa7bc 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1169,7 +1169,7 @@ int ovl_workdir_cleanup(struct ovl_fs *ofs, struct inode *dir, int ovl_indexdir_cleanup(struct ovl_fs *ofs) { int err; - struct dentry *indexdir = ofs->indexdir; + struct dentry *indexdir = ofs->workdir; struct dentry *index = NULL; struct inode *dir = indexdir->d_inode; struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = indexdir }; diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index a0967bb25003..f78161cf8388 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -853,10 +853,8 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, if (IS_ERR(indexdir)) { err = PTR_ERR(indexdir); } else if (indexdir) { - ofs->indexdir = indexdir; - ofs->workdir = dget(indexdir); - - err = ovl_setup_trap(sb, ofs->indexdir, &ofs->indexdir_trap, + ofs->workdir = indexdir; + err = ovl_setup_trap(sb, indexdir, &ofs->workdir_trap, "indexdir"); if (err) goto out; @@ -869,16 +867,15 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, * ".overlay.upper" to indicate that index may have * directory entries. */ - if (ovl_check_origin_xattr(ofs, ofs->indexdir)) { - err = ovl_verify_origin_xattr(ofs, ofs->indexdir, + if (ovl_check_origin_xattr(ofs, indexdir)) { + err = ovl_verify_origin_xattr(ofs, indexdir, OVL_XATTR_ORIGIN, upperpath->dentry, true, false); if (err) pr_err("failed to verify index dir 'origin' xattr\n"); } - err = ovl_verify_upper(ofs, ofs->indexdir, upperpath->dentry, - true); + err = ovl_verify_upper(ofs, indexdir, upperpath->dentry, true); if (err) pr_err("failed to verify index dir 'upper' xattr\n"); @@ -886,7 +883,7 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, if (!err) err = ovl_indexdir_cleanup(ofs); } - if (err || !ofs->indexdir) + if (err || !indexdir) pr_warn("try deleting index dir or mounting with '-o index=off' to disable inodes index.\n"); out: @@ -1406,7 +1403,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc) goto out_free_oe; /* Force r/o mount with no index dir */ - if (!ofs->indexdir) + if (!ofs->workdir) sb->s_flags |= SB_RDONLY; } @@ -1415,7 +1412,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc) goto out_free_oe; /* Show index=off in /proc/mounts for forced r/o mount */ - if (!ofs->indexdir) { + if (!ofs->workdir) { ofs->config.index = false; if (ovl_upper_mnt(ofs) && ofs->config.nfs_export) { pr_warn("NFS export requires an index dir, falling back to nfs_export=off.\n"); diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index c3f020ca13a8..22b519763267 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -91,7 +91,7 @@ struct dentry *ovl_indexdir(struct super_block *sb) { struct ovl_fs *ofs = OVL_FS(sb); - return ofs->indexdir; + return ofs->config.index ? ofs->workdir : NULL; } /* Index all files on copy up. For now only enabled for NFS export */ -- cgit v1.2.3 From 2c3ef4f89ced1a9d3a6fd4f6457be5c440c89362 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Mon, 20 Nov 2023 15:43:34 +0200 Subject: ovl: initialize ovl_copy_up_ctx.destname inside ovl_do_copy_up() The ->destname member of struct ovl_copy_up_ctx is initialized inside ovl_copy_up_one() to ->d_name of the overlayfs dentry being copied up and then it may be overridden by index name inside ovl_do_copy_up(). ovl_inode_lock() in ovl_copy_up_start() and ovl_copy_up() in ovl_rename() effectively stabilze ->d_name of the overlayfs dentry being copied up, but ovl_inode_lock() is not held when ->d_name is being read. It is not a correctness bug, because if ovl_do_copy_up() races with ovl_rename() and ctx.destname is freed, we will not end up calling ovl_do_copy_up() with the dead name reference. The code becomes much easier to understand and to document if the initialization of c->destname is always done inside ovl_do_copy_up(), either to the index entry name, or to the overlay dentry ->d_name. Signed-off-by: Amir Goldstein --- fs/overlayfs/copy_up.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'fs/overlayfs') diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 4382881b0709..500c555792ff 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -929,6 +929,13 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) err = -EIO; goto out_free_fh; } else { + /* + * c->dentry->d_name is stabilzed by ovl_copy_up_start(), + * because if we got here, it means that c->dentry has no upper + * alias and changing ->d_name means going through ovl_rename() + * that will call ovl_copy_up() on source and target dentry. + */ + c->destname = c->dentry->d_name; /* * Mark parent "impure" because it may now contain non-pure * upper @@ -1109,7 +1116,6 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, if (parent) { ovl_path_upper(parent, &parentpath); ctx.destdir = parentpath.dentry; - ctx.destname = dentry->d_name; err = vfs_getattr(&parentpath, &ctx.pstat, STATX_ATIME | STATX_MTIME, -- cgit v1.2.3