ovl: Call ovl_create_temp() without lock held.

ovl currently locks a directory or two and then performs multiple actions
in one or both directories.  This is incompatible with proposed changes
which will lock just the dentry of objects being acted on.

This patch moves calls to ovl_create_temp() out of the locked regions and
has it take and release the relevant lock itself.

The lock that was taken before this function was called is now taken
after.  This means that any code between where the lock was taken and
ovl_create_temp() is now unlocked.  This necessitates the use of
ovl_cleanup_unlocked() and the creation of ovl_lookup_upper_unlocked().
These will be used more widely in future patches.

Now that the file is created before the lock is taken for rename, we
need to ensure the parent wasn't changed before the lock was gained.
ovl_lock_rename_workdir() is changed to optionally receive the dentries
that will be involved in the rename.  If either is present but has the
wrong parent, an error is returned.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://lore.kernel.org/20250716004725.1206467-4-neil@brown.name
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index d485bd7edd8ff640a0cf4acb9165f3f52957a1ed..fef873d18b2da92d728e5c1da68340fcf6c3d065 100644 (file)
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -523,7 +523,6 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 {
        struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
        struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
-       struct inode *dir = d_inode(indexdir);
        struct dentry *index = NULL;
        struct dentry *temp = NULL;
        struct qstr name = { };
@@ -548,9 +547,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
        if (err)
                return err;
 
-       inode_lock(dir);
        temp = ovl_create_temp(ofs, indexdir, OVL_CATTR(S_IFDIR | 0));
-       inode_unlock(dir);
        err = PTR_ERR(temp);
        if (IS_ERR(temp))
                goto free_name;
@@ -766,7 +763,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 {
        struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
        struct inode *inode;
-       struct inode *wdir = d_inode(c->workdir);
        struct path path = { .mnt = ovl_upper_mnt(ofs) };
        struct dentry *temp, *upper, *trap;
        struct ovl_cu_creds cc;
@@ -783,9 +779,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
                return err;
 
        ovl_start_write(c->dentry);
-       inode_lock(wdir);
        temp = ovl_create_temp(ofs, c->workdir, &cattr);
-       inode_unlock(wdir);
        ovl_end_write(c->dentry);
        ovl_revert_cu_creds(&cc);
 

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 67cad3dba8ad639e86cadb57da5c3e39a1ae79c2..373335e420fd6770e90513929e361cdf7f656652 100644 (file)
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -214,8 +214,12 @@ out:
 struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir,
                               struct ovl_cattr *attr)
 {
-       return ovl_create_real(ofs, d_inode(workdir),
-                              ovl_lookup_temp(ofs, workdir), attr);
+       struct dentry *ret;
+       inode_lock(workdir->d_inode);
+       ret = ovl_create_real(ofs, d_inode(workdir),
+                             ovl_lookup_temp(ofs, workdir), attr);
+       inode_unlock(workdir->d_inode);
+       return ret;
 }
 
 static int ovl_set_opaque_xerr(struct dentry *dentry, struct dentry *upper,
@@ -353,7 +357,6 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
        struct dentry *workdir = ovl_workdir(dentry);
        struct inode *wdir = workdir->d_inode;
        struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
-       struct inode *udir = upperdir->d_inode;
        struct path upperpath;
        struct dentry *upper;
        struct dentry *opaquedir;
@@ -363,27 +366,25 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
        if (WARN_ON(!workdir))
                return ERR_PTR(-EROFS);
 
-       err = ovl_lock_rename_workdir(workdir, upperdir);
-       if (err)
-               goto out;
-
        ovl_path_upper(dentry, &upperpath);
        err = vfs_getattr(&upperpath, &stat,
                          STATX_BASIC_STATS, AT_STATX_SYNC_AS_STAT);
        if (err)
-               goto out_unlock;
+               goto out;
 
        err = -ESTALE;
        if (!S_ISDIR(stat.mode))
-               goto out_unlock;
+               goto out;
        upper = upperpath.dentry;
-       if (upper->d_parent->d_inode != udir)
-               goto out_unlock;
 
        opaquedir = ovl_create_temp(ofs, workdir, OVL_CATTR(stat.mode));
        err = PTR_ERR(opaquedir);
        if (IS_ERR(opaquedir))
-               goto out_unlock;
+               goto out;
+
+       err = ovl_lock_rename_workdir(workdir, opaquedir, upperdir, upper);
+       if (err)
+               goto out_cleanup_unlocked;
 
        err = ovl_copy_xattr(dentry->d_sb, &upperpath, opaquedir);
        if (err)
@@ -413,10 +414,10 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
        return opaquedir;
 
 out_cleanup:
-       ovl_cleanup(ofs, wdir, opaquedir);
-       dput(opaquedir);
-out_unlock:
        unlock_rename(workdir, upperdir);
+out_cleanup_unlocked:
+       ovl_cleanup_unlocked(ofs, workdir, opaquedir);
+       dput(opaquedir);
 out:
        return ERR_PTR(err);
 }
@@ -454,15 +455,11 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
                        return err;
        }
 
-       err = ovl_lock_rename_workdir(workdir, upperdir);
-       if (err)
-               goto out;
-
-       upper = ovl_lookup_upper(ofs, dentry->d_name.name, upperdir,
-                                dentry->d_name.len);
+       upper = ovl_lookup_upper_unlocked(ofs, dentry->d_name.name, upperdir,
+                                         dentry->d_name.len);
        err = PTR_ERR(upper);
        if (IS_ERR(upper))
-               goto out_unlock;
+               goto out;
 
        err = -ESTALE;
        if (d_is_negative(upper) || !ovl_upper_is_whiteout(ofs, upper))
@@ -473,6 +470,10 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
        if (IS_ERR(newdentry))
                goto out_dput;
 
+       err = ovl_lock_rename_workdir(workdir, newdentry, upperdir, upper);
+       if (err)
+               goto out_cleanup_unlocked;
+
        /*
         * mode could have been mutilated due to umask (e.g. sgid directory)
         */
@@ -523,10 +524,9 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
                ovl_cleanup(ofs, udir, newdentry);
                dput(newdentry);
        }
+       unlock_rename(workdir, upperdir);
 out_dput:
        dput(upper);
-out_unlock:
-       unlock_rename(workdir, upperdir);
 out:
        if (!hardlink) {
                posix_acl_release(acl);
@@ -535,7 +535,9 @@ out:
        return err;
 
 out_cleanup:
-       ovl_cleanup(ofs, wdir, newdentry);
+       unlock_rename(workdir, upperdir);
+out_cleanup_unlocked:
+       ovl_cleanup_unlocked(ofs, workdir, newdentry);
        dput(newdentry);
        goto out_dput;
 }
@@ -772,7 +774,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
                        goto out;
        }
 
-       err = ovl_lock_rename_workdir(workdir, upperdir);
+       err = ovl_lock_rename_workdir(workdir, NULL, upperdir, NULL);
        if (err)
                goto out_dput;
 

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index a8f18a06a19e0330b14e100c38111b7418f1a244..002019b40af3720e38d95c5e2a53d6bdd4a0e49a 100644 (file)
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -405,6 +405,15 @@ static inline struct dentry *ovl_lookup_upper(struct ovl_fs *ofs,
        return lookup_one(ovl_upper_mnt_idmap(ofs), &QSTR_LEN(name, len), base);
 }
 
+static inline struct dentry *ovl_lookup_upper_unlocked(struct ovl_fs *ofs,
+                                                      const char *name,
+                                                      struct dentry *base,
+                                                      int len)
+{
+       return lookup_one_unlocked(ovl_upper_mnt_idmap(ofs),
+                                  &QSTR_LEN(name, len), base);
+}
+
 static inline bool ovl_open_flags_need_copy_up(int flags)
 {
        if (!flags)
@@ -544,7 +553,8 @@ bool ovl_is_inuse(struct dentry *dentry);
 bool ovl_need_index(struct dentry *dentry);
 int ovl_nlink_start(struct dentry *dentry);
 void ovl_nlink_end(struct dentry *dentry);
-int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir);
+int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
+                           struct dentry *upperdir, struct dentry *upper);
 int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path,
                             struct ovl_metacopy *data);
 int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index cf99b276fdfbfaa47e40b2ac12c144df6883e41e..2e6b25bde83f6cd2a83d746e27b7865bdbf6bcf9 100644 (file)
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -564,13 +564,16 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
        struct name_snapshot name;
        int err;
 
-       inode_lock_nested(dir, I_MUTEX_PARENT);
-
        temp = ovl_create_temp(ofs, workdir, OVL_CATTR(S_IFREG | 0));
        err = PTR_ERR(temp);
        if (IS_ERR(temp))
-               goto out_unlock;
+               return err;
 
+       err = ovl_parent_lock(workdir, temp);
+       if (err) {
+               dput(temp);
+               return err;
+       }
        dest = ovl_lookup_temp(ofs, workdir);
        err = PTR_ERR(dest);
        if (IS_ERR(dest)) {
@@ -606,7 +609,7 @@ cleanup_temp:
        dput(dest);
 
 out_unlock:
-       inode_unlock(dir);
+       ovl_parent_unlock(workdir);
 
        return err;
 }

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 130aba055ffe54099b89298bfadff3267f323a05..c25c86e0f4da8aa06d793c93e89e1dc684c53378 100644 (file)
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1227,7 +1227,8 @@ void ovl_nlink_end(struct dentry *dentry)
        ovl_inode_unlock(inode);
 }
 
-int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir)
+int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
+                           struct dentry *upperdir, struct dentry *upper)
 {
        struct dentry *trap;
 
@@ -1241,6 +1242,10 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir)
                goto err;
        if (trap)
                goto err_unlock;
+       if (work && work->d_parent != workdir)
+               goto err_unlock;
+       if (upper && upper->d_parent != upperdir)
+               goto err_unlock;
 
        return 0;