start_removing_dentry() is similar to start_removing() but instead of providing a name for lookup, the target dentry is given. start_removing_dentry() checks that the dentry is still hashed and in the parent, and if so it locks and increases the refcount so that end_dirop() can be used to finish the operation. This is used in cachefiles, overlayfs, smb/server and apparmor. There will be other users including ecryptfs. Signed-off-by: NeilBrown <neil@xxxxxxxxxx> --- fs/cachefiles/interface.c | 14 +++++++++----- fs/cachefiles/namei.c | 22 ++++++++++++---------- fs/cachefiles/volume.c | 10 +++++++--- fs/namei.c | 29 +++++++++++++++++++++++++++++ fs/overlayfs/dir.c | 10 ++++------ fs/overlayfs/readdir.c | 8 ++++---- fs/smb/server/vfs.c | 27 ++++----------------------- include/linux/namei.h | 2 ++ security/apparmor/apparmorfs.c | 8 ++++---- 9 files changed, 75 insertions(+), 55 deletions(-) diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c index 3e63cfe15874..763d7d55b1f9 100644 --- a/fs/cachefiles/interface.c +++ b/fs/cachefiles/interface.c @@ -9,6 +9,7 @@ #include <linux/mount.h> #include <linux/xattr.h> #include <linux/file.h> +#include <linux/namei.h> #include <linux/falloc.h> #include <trace/events/fscache.h> #include "internal.h" @@ -428,11 +429,14 @@ static bool cachefiles_invalidate_cookie(struct fscache_cookie *cookie) if (!old_tmpfile) { struct cachefiles_volume *volume = object->volume; struct dentry *fan = volume->fanout[(u8)cookie->key_hash]; - - inode_lock_nested(d_inode(fan), I_MUTEX_PARENT); - cachefiles_bury_object(volume->cache, object, fan, - old_file->f_path.dentry, - FSCACHE_OBJECT_INVALIDATED); + struct dentry *obj; + + obj = start_removing_dentry(fan, old_file->f_path.dentry); + if (!IS_ERR(obj)) + cachefiles_bury_object(volume->cache, object, + fan, obj, + FSCACHE_OBJECT_INVALIDATED); + end_dirop(obj); } fput(old_file); } diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index ddced50afb66..cc6dccd606ea 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -424,13 +424,12 @@ int cachefiles_delete_object(struct cachefiles_object *object, _enter(",OBJ%x{%pD}", object->debug_id, object->file); - /* Stop the dentry being negated if it's only pinned by a file struct. */ - dget(dentry); - - inode_lock_nested(d_backing_inode(fan), I_MUTEX_PARENT); - ret = cachefiles_unlink(volume->cache, object, fan, dentry, why); - inode_unlock(d_backing_inode(fan)); - dput(dentry); + dentry = start_removing_dentry(fan, dentry); + if (IS_ERR(dentry)) + ret = PTR_ERR(dentry); + else + ret = cachefiles_unlink(volume->cache, object, fan, dentry, why); + end_dirop(dentry); return ret; } @@ -643,9 +642,12 @@ bool cachefiles_look_up_object(struct cachefiles_object *object) if (!d_is_reg(dentry)) { pr_err("%pd is not a file\n", dentry); - inode_lock_nested(d_inode(fan), I_MUTEX_PARENT); - ret = cachefiles_bury_object(volume->cache, object, fan, dentry, - FSCACHE_OBJECT_IS_WEIRD); + struct dentry *de = start_removing_dentry(fan, dentry); + if (!IS_ERR(de)) + ret = cachefiles_bury_object(volume->cache, object, + fan, de, + FSCACHE_OBJECT_IS_WEIRD); + end_dirop(de); dput(dentry); if (ret < 0) return false; diff --git a/fs/cachefiles/volume.c b/fs/cachefiles/volume.c index 781aac4ef274..8c29f3db3fae 100644 --- a/fs/cachefiles/volume.c +++ b/fs/cachefiles/volume.c @@ -7,6 +7,7 @@ #include <linux/fs.h> #include <linux/slab.h> +#include <linux/namei.h> #include "internal.h" #include <trace/events/fscache.h> @@ -58,9 +59,12 @@ void cachefiles_acquire_volume(struct fscache_volume *vcookie) if (ret < 0) { if (ret != -ESTALE) goto error_dir; - inode_lock_nested(d_inode(cache->store), I_MUTEX_PARENT); - cachefiles_bury_object(cache, NULL, cache->store, vdentry, - FSCACHE_VOLUME_IS_WEIRD); + vdentry = start_removing_dentry(cache->store, vdentry); + if (!IS_ERR(vdentry)) + cachefiles_bury_object(cache, NULL, cache->store, + vdentry, + FSCACHE_VOLUME_IS_WEIRD); + end_dirop(vdentry); cachefiles_put_directory(volume->dentry); cond_resched(); goto retry; diff --git a/fs/namei.c b/fs/namei.c index 34895487045e..af56bc39c4d5 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3261,6 +3261,35 @@ struct dentry *start_removing_noperm(struct dentry *parent, } EXPORT_SYMBOL(start_removing_noperm); +/** + * start_removing_dentry - prepare to remove a given dentry + * @parent - directory from which dentry should be removed + * @child - the dentry to be removed + * + * A lock is taken to protect the dentry again other dirops and + * the validity of the dentry is checked: correct parent and still hashed. + * + * If the dentry is valid a reference is taken and returned. If not + * an error is returned. + * + * end_dirop() should be called when removal is complete, or aborted. + * + * Returns: the valid dentry, or an error. + */ +struct dentry *start_removing_dentry(struct dentry *parent, + struct dentry *child) +{ + inode_lock_nested(parent->d_inode, I_MUTEX_PARENT); + if (unlikely(IS_DEADDIR(parent->d_inode) || + child->d_parent != parent || + d_unhashed(child))) { + inode_unlock(parent->d_inode); + return ERR_PTR(-EINVAL); + } + return dget(child); +} +EXPORT_SYMBOL(start_removing_dentry); + #ifdef CONFIG_UNIX98_PTYS int path_pts(struct path *path) { diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 70b8687dc45e..b8f0d409e841 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -47,14 +47,12 @@ static int ovl_cleanup_locked(struct ovl_fs *ofs, struct inode *wdir, int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *wdentry) { - int err; - - err = ovl_parent_lock(workdir, wdentry); - if (err) - return err; + wdentry = start_removing_dentry(workdir, wdentry); + if (IS_ERR(wdentry)) + return PTR_ERR(wdentry); ovl_cleanup_locked(ofs, workdir->d_inode, wdentry); - ovl_parent_unlock(workdir); + end_dirop(wdentry); return 0; } diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index b65cdfce31ce..20348be4b98f 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1158,11 +1158,11 @@ int ovl_workdir_cleanup(struct ovl_fs *ofs, struct dentry *parent, if (!d_is_dir(dentry) || level > 1) return ovl_cleanup(ofs, parent, dentry); - err = ovl_parent_lock(parent, dentry); - if (err) - return err; + dentry = start_removing_dentry(parent, dentry); + if (IS_ERR(dentry)) + return PTR_ERR(dentry); err = ovl_do_rmdir(ofs, parent->d_inode, dentry); - ovl_parent_unlock(parent); + end_dirop(dentry); if (err) { struct path path = { .mnt = mnt, .dentry = dentry }; diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index 07739055ac9f..04030a204d5a 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -48,24 +48,6 @@ static void ksmbd_vfs_inherit_owner(struct ksmbd_work *work, i_uid_write(inode, i_uid_read(parent_inode)); } -/** - * ksmbd_vfs_lock_parent() - lock parent dentry if it is stable - * @parent: parent dentry - * @child: child dentry - * - * Returns: %0 on success, %-ENOENT if the parent dentry is not stable - */ -int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child) -{ - inode_lock_nested(d_inode(parent), I_MUTEX_PARENT); - if (child->d_parent != parent) { - inode_unlock(d_inode(parent)); - return -ENOENT; - } - - return 0; -} - static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf, char *pathname, unsigned int flags, struct path *path, bool do_lock) @@ -1083,18 +1065,17 @@ int ksmbd_vfs_unlink(struct file *filp) return err; dir = dget_parent(dentry); - err = ksmbd_vfs_lock_parent(dir, dentry); - if (err) + dentry = start_removing_dentry(dir, dentry); + err = PTR_ERR(dentry); + if (IS_ERR(dentry)) goto out; - dget(dentry); if (S_ISDIR(d_inode(dentry)->i_mode)) err = vfs_rmdir(idmap, d_inode(dir), dentry); else err = vfs_unlink(idmap, d_inode(dir), dentry, NULL); - dput(dentry); - inode_unlock(d_inode(dir)); + end_dirop(dentry); if (err) ksmbd_debug(VFS, "failed to delete, err %d\n", err); out: diff --git a/include/linux/namei.h b/include/linux/namei.h index d765a23c87e4..b89be0ac5e87 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -87,6 +87,8 @@ struct dentry *start_removing(struct mnt_idmap *idmap, struct dentry *parent, struct qstr *name); struct dentry *start_creating_noperm(struct dentry *parent, struct qstr *name); struct dentry *start_removing_noperm(struct dentry *parent, struct qstr *name); +struct dentry *start_removing_dentry(struct dentry *parent, + struct dentry *child); void end_dirop(struct dentry *de); void end_dirop_mkdir(struct dentry *de, struct dentry *parent); diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 13260352198f..f33fc19c99a7 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -343,17 +343,17 @@ static void aafs_remove(struct dentry *dentry) if (!dentry || IS_ERR(dentry)) return; + /* ->d_parent is stable as rename is not supported */ dir = d_inode(dentry->d_parent); - inode_lock(dir); - if (simple_positive(dentry)) { + dentry = start_removing_dentry(dentry->d_parent, dentry); + if (!IS_ERR(dentry) && simple_positive(dentry)) { if (d_is_dir(dentry)) simple_rmdir(dir, dentry); else simple_unlink(dir, dentry); d_delete(dentry); - dput(dentry); } - inode_unlock(dir); + end_dirop(dentry); simple_release_fs(&aafs_mnt, &aafs_count); } -- 2.50.0.107.gf914562f5916.dirty