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. This removes the only remaining use of ovl_lookup_upper, so it is removed. Signed-off-by: NeilBrown <[email protected]> --- fs/overlayfs/dir.c | 24 ++++++++++++++++++------ fs/overlayfs/overlayfs.h | 7 ------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index ff3dbd1ca61f..ec08904d084d 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -219,21 +219,33 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent, err = -EIO; } else if (d_unhashed(newdentry)) { struct dentry *d; + struct name_snapshot name; /* * 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. + * 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)) + take_dentry_name_snapshot(&name, newdentry); + end_creating_keep(newdentry); + d = ovl_start_creating_upper(ofs, parent, &name.name); + release_dentry_name_snapshot(&name); + + 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) { diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 315882a360ce..4fb4750a83e4 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, -- 2.50.0.107.gf914562f5916.dirty
