Skip to content

Commit 8afa0a7

Browse files
neilbrownbrauner
authored andcommitted
ovl: narrow locking in ovl_whiteout()
ovl_whiteout() relies on the workdir i_rwsem to provide exclusive access to ofs->whiteout which it manipulates. Rather than depending on this, add a new mutex, "whiteout_lock" to explicitly provide the required locking. Use guard(mutex) for this so that we can return without needing to explicitly unlock. Then take the lock on workdir only when needed - to lookup the temp name and to do the whiteout or link. Signed-off-by: NeilBrown <[email protected]> Link: https://lore.kernel.org/[email protected] Reviewed-by: Amir Goldstein <[email protected]> Signed-off-by: Christian Brauner <[email protected]>
1 parent 2fa14cf commit 8afa0a7

File tree

3 files changed

+27
-20
lines changed

3 files changed

+27
-20
lines changed

fs/overlayfs/dir.c

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -84,41 +84,45 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
8484
struct dentry *workdir = ofs->workdir;
8585
struct inode *wdir = workdir->d_inode;
8686

87-
inode_lock_nested(wdir, I_MUTEX_PARENT);
87+
guard(mutex)(&ofs->whiteout_lock);
88+
8889
if (!ofs->whiteout) {
90+
inode_lock_nested(wdir, I_MUTEX_PARENT);
8991
whiteout = ovl_lookup_temp(ofs, workdir);
90-
if (IS_ERR(whiteout))
91-
goto out;
92-
93-
err = ovl_do_whiteout(ofs, wdir, whiteout);
94-
if (err) {
95-
dput(whiteout);
96-
whiteout = ERR_PTR(err);
97-
goto out;
92+
if (!IS_ERR(whiteout)) {
93+
err = ovl_do_whiteout(ofs, wdir, whiteout);
94+
if (err) {
95+
dput(whiteout);
96+
whiteout = ERR_PTR(err);
97+
}
9898
}
99+
inode_unlock(wdir);
100+
if (IS_ERR(whiteout))
101+
return whiteout;
99102
ofs->whiteout = whiteout;
100103
}
101104

102105
if (!ofs->no_shared_whiteout) {
106+
inode_lock_nested(wdir, I_MUTEX_PARENT);
103107
whiteout = ovl_lookup_temp(ofs, workdir);
104-
if (IS_ERR(whiteout))
105-
goto out;
106-
107-
err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout);
108-
if (!err)
109-
goto out;
110-
111-
if (err != -EMLINK) {
108+
if (!IS_ERR(whiteout)) {
109+
err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout);
110+
if (err) {
111+
dput(whiteout);
112+
whiteout = ERR_PTR(err);
113+
}
114+
}
115+
inode_unlock(wdir);
116+
if (!IS_ERR(whiteout))
117+
return whiteout;
118+
if (PTR_ERR(whiteout) != -EMLINK) {
112119
pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n",
113120
ofs->whiteout->d_inode->i_nlink, err);
114121
ofs->no_shared_whiteout = true;
115122
}
116-
dput(whiteout);
117123
}
118124
whiteout = ofs->whiteout;
119125
ofs->whiteout = NULL;
120-
out:
121-
inode_unlock(wdir);
122126
return whiteout;
123127
}
124128

fs/overlayfs/ovl_entry.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ struct ovl_fs {
8888
/* Shared whiteout cache */
8989
struct dentry *whiteout;
9090
bool no_shared_whiteout;
91+
struct mutex whiteout_lock;
9192
/* r/o snapshot of upperdir sb's only taken on volatile mounts */
9293
errseq_t errseq;
9394
};

fs/overlayfs/params.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,8 @@ int ovl_init_fs_context(struct fs_context *fc)
795795
fc->s_fs_info = ofs;
796796
fc->fs_private = ctx;
797797
fc->ops = &ovl_context_ops;
798+
799+
mutex_init(&ofs->whiteout_lock);
798800
return 0;
799801

800802
out_err:

0 commit comments

Comments
 (0)