VFS: fix possible failure to unlock in nfsd4_create_file()

atomic_create() in fs/namei.c drops the reference to the dentry
when it returns an error.
This behaviour was imported into dentry_create() so that it
will drop the reference if an error is returned from atomic_create(),
though not if vfs_create() returns an error (in the case where
->atomic_create is not supported).

The caller - nfsd4_create_file() - is made aware of this by checking
path->dentry, which will either be a counted reference to a dentry, or
an error pointer.

However the change to use start_creating()/end_creating() (which landed
shortly before the dentry_create() change landed, though was likely
developed around the same time) means that nfsd4_create_file() *needs* a
valid dentry so that it can unlock the parent.

The net result is that if NFSD exports a filesystem which uses
->atomic_create, and if a call to ->atomic_create returns an error, then
nfsd4_create_file() will pass an error pointer to end_creating()
and the parent will not be unlocked.

Fix this by changing dentry_create() to make sure path->dentry is always
a valid dentry, never an error-pointer.  The actual error is already
returned a different way.

Note that if ->atomic_create() returns a different dentry (which may not
be possible in practice) we are guaranteed (because it is only ever
provided by d_spliace_alias()) that it will have the same d_parent and
so it will have the same effect when passed to end_creating().

Fixes: 64a989dbd144 ("VFS/knfsd: Teach dentry_create() to use atomic_open()")
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/177969022571.3379282.16448744624428323496@noble.neil.brown.name
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com>
Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

diff --git a/fs/namei.c b/fs/namei.c
index c7fac83c9a85ef250bb424af0a91d581ee9f4919..4787244ca4a75bfbf0ae370738ceb086f4532e19 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -5024,6 +5024,7 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
 {
        struct file *file __free(fput) = NULL;
        struct dentry *dentry = path->dentry;
+       struct dentry *orig_dentry = dentry;
        struct dentry *dir = dentry->d_parent;
        struct inode *dir_inode = d_inode(dir);
        struct mnt_idmap *idmap;
@@ -5043,9 +5044,18 @@ struct file *dentry_create(struct path *path, int flags, umode_t mode,
                if (create_error)
                        flags &= ~O_CREAT;
 
+               /* atomic_open will dput(dentry) on error */
+               dget(orig_dentry);
                dentry = atomic_open(path, dentry, file, flags, mode);
                error = PTR_ERR_OR_ZERO(dentry);
 
+               if (IS_ERR(dentry))
+                       /* keep the original */
+                       dentry = orig_dentry;
+               else
+                       /* Drop the extra reference */
+                       dput(orig_dentry);
+
                if (unlikely(create_error) && error == -ENOENT)
                        error = create_error;