On Mon, Feb 23, 2026 at 2:14 AM NeilBrown <[email protected]> wrote: > > From: NeilBrown <[email protected]> > > 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 <[email protected]>
Reviewed-by: Amir Goldstein <[email protected]> > --- > 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 >
