[PATCH v2 12/15] ovl: change ovl_create_real() to get a new lock when re-opening created file.
Amir Goldstein
amir73il at gmail.com
Mon Feb 23 09:39:39 UTC 2026
On Mon, Feb 23, 2026 at 2:14 AM NeilBrown <neilb at ownmail.net> wrote:
>
> From: NeilBrown <neil at brown.name>
>
> When ovl_create_real() is used to create a file on the upper filesystem
> it needs to return the resulting dentry - positive and hashed.
> It is usually the case the that dentry passed to the create function
> (e.g. vfs_create()) will be suitable but this is not guaranteed. The
> filesystem may unhash that dentry forcing a repeat lookup next time the
> name is wanted.
>
> So ovl_create_real() must be (and is) aware of this and prepared to
> perform that lookup to get a hash positive dentry.
>
> This is currently done under that same directory lock that provided
> exclusion for the create. Proposed changes to locking will make this
> not possible - as the name, rather than the directory, will be locked.
> The new APIs provided for lookup and locking do not and cannot support
> this pattern.
>
> The lock isn't needed. ovl_create_real() can drop the lock and then get
> a new lock for the lookup - then check that the lookup returned the
> correct inode. In a well-behaved configuration where the upper
> filesystem is not being modified by a third party, this will always work
> reliably, and if there are separate modification it will fail cleanly.
>
> So change ovl_create_real() to drop the lock and call
> ovl_start_creating_upper() to find the correct dentry. Note that
> start_creating doesn't fail if the name already exists.
>
> The lookup previously used the name from newdentry which was guaranteed
> to be stable because the parent directory was locked. As we now drop
> the lock we lose that guarantee. As newdentry is unhashed it is
> unlikely for the name to change, but safest not to depend on that. So
> the expected name is now passed in to ovl_create_real() and that is
> used.
>
> This removes the only remaining use of ovl_lookup_upper, so it is
> removed.
>
> Signed-off-by: NeilBrown <neil at brown.name>
Reviewed-by: Amir Goldstein <amir73il at gmail.com>
> ---
> fs/overlayfs/dir.c | 36 ++++++++++++++++++++++++------------
> fs/overlayfs/overlayfs.h | 8 +-------
> fs/overlayfs/super.c | 1 +
> 3 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index c4feb89ad1e3..6285069ccc59 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -159,7 +159,8 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
> }
>
> struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent,
> - struct dentry *newdentry, struct ovl_cattr *attr)
> + struct dentry *newdentry, struct qstr *qname,
> + struct ovl_cattr *attr)
> {
> struct inode *dir = parent->d_inode;
> int err;
> @@ -221,19 +222,29 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent,
> struct dentry *d;
> /*
> * Some filesystems (i.e. casefolded) may return an unhashed
> - * negative dentry from the ovl_lookup_upper() call before
> + * negative dentry from the ovl_start_creating_upper() call before
> * ovl_create_real().
> * In that case, lookup again after making the newdentry
> * positive, so ovl_create_upper() always returns a hashed
> - * positive dentry.
> + * positive dentry. We lookup using qname which should be
> + * the same name as newentry, but is certain not to change.
> + * As we have to drop the lock before the lookup a race
> + * could result in a lookup failure. In that case we return
> + * an error.
> */
> - d = ovl_lookup_upper(ofs, newdentry->d_name.name, parent,
> - newdentry->d_name.len);
> - dput(newdentry);
> - if (IS_ERR_OR_NULL(d))
> + end_creating_keep(newdentry);
> + d = ovl_start_creating_upper(ofs, parent, qname);
> +
> + if (IS_ERR_OR_NULL(d)) {
> err = d ? PTR_ERR(d) : -ENOENT;
> - else
> + } else if (d->d_inode != newdentry->d_inode) {
> + err = -EIO;
> + dput(newdentry);
> + } else {
> + dput(newdentry);
> return d;
> + }
> + return ERR_PTR(err);
> }
> out:
> if (err) {
> @@ -252,7 +263,7 @@ struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir,
> ret = ovl_start_creating_temp(ofs, workdir, name);
> if (IS_ERR(ret))
> return ret;
> - ret = ovl_create_real(ofs, workdir, ret, attr);
> + ret = ovl_create_real(ofs, workdir, ret, &QSTR(name), attr);
> return end_creating_keep(ret);
> }
>
> @@ -352,14 +363,15 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
> struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
> struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
> struct dentry *newdentry;
> + struct qstr qname = QSTR_LEN(dentry->d_name.name,
> + dentry->d_name.len);
> int err;
>
> newdentry = ovl_start_creating_upper(ofs, upperdir,
> - &QSTR_LEN(dentry->d_name.name,
> - dentry->d_name.len));
> + &qname);
> if (IS_ERR(newdentry))
> return PTR_ERR(newdentry);
> - newdentry = ovl_create_real(ofs, upperdir, newdentry, attr);
> + newdentry = ovl_create_real(ofs, upperdir, newdentry, &qname, attr);
> if (IS_ERR(newdentry))
> return PTR_ERR(newdentry);
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index cad2055ebf18..714a1cec3709 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -406,13 +406,6 @@ static inline struct file *ovl_do_tmpfile(struct ovl_fs *ofs,
> return file;
> }
>
> -static inline struct dentry *ovl_lookup_upper(struct ovl_fs *ofs,
> - const char *name,
> - struct dentry *base, int len)
> -{
> - 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,
> @@ -888,6 +881,7 @@ struct ovl_cattr {
>
> struct dentry *ovl_create_real(struct ovl_fs *ofs,
> struct dentry *parent, struct dentry *newdentry,
> + struct qstr *qname,
> struct ovl_cattr *attr);
> int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry);
> #define OVL_TEMPNAME_SIZE 20
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index d4c12feec039..109643930b9f 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -634,6 +634,7 @@ static struct dentry *ovl_lookup_or_create(struct ovl_fs *ofs,
> if (!IS_ERR(child)) {
> if (!child->d_inode)
> child = ovl_create_real(ofs, parent, child,
> + &QSTR(name),
> OVL_CATTR(mode));
> end_creating_keep(child);
> }
> --
> 2.50.0.107.gf914562f5916.dirty
>
More information about the Linux-security-module-archive
mailing list