linux-kernel - [PATCH v2] VFS: filename_create(): fix incorrect intent.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-id: <164869878475.25542.14478885718949986319@noble.neil.brown.name>
Date:   Thu, 31 Mar 2022 14:53:04 +1100
From:   "NeilBrown" <neilb@...e.de>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     linux-nfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Jeff Layton <jlayton@...nel.org>,
        David Disseldorp <ddiss@...e.de>
Subject: [PATCH v2] VFS: filename_create(): fix incorrect intent.


When asked to create a path ending '/', but which is not to be a
directory (LOOKUP_DIRECTORY not set), filename_create() will never try
to create the file.  If it doesn't exist, -ENOENT is reported.

However, it still passes LOOKUP_CREATE|LOOKUP_EXCL to the filesystems
->lookup() function, even though there is no intent to create.  This is
misleading and can cause incorrect behaviour.

If you try
   ln -s foo /path/dir/

where 'dir' is a directory on an NFS filesystem which is not currently
known in the dcache, this will fail with ENOENT.
As the name is not in the dcache, nfs_lookup gets called with
LOOKUP_CREATE|LOOKUP_EXCL and so it returns NULL without performing any
lookup, with the expectation that as subsequent call to create the
target will be made, and the lookup can be combined with the creation.
In the case with a trailing '/' and no LOOKUP_DIRECTORY, that call is never
made.  Instead filename_create() sees that the dentry is not (yet)
positive and returns -ENOENT - even though the directory actually
exists.

So only set LOOKUP_CREATE|LOOKUP_EXCL if there really is an intent
to create, and use the absence of these flags to decide if -ENOENT
should be returned.

Note that we now leave LOOKUP_DIRECTORY in lookup_flags as passed to
->lookup etc.  This seems more consistent with the comment which says
that only LOOKUP_REVAL and LOOKUP_DIRECTORY are relevant, and makes the
code a little cleaner.

Signed-off-by: NeilBrown <neilb@...e.de>
---
 fs/namei.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 3f1829b3ab5b..6d337d951dd2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3676,13 +3676,12 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	int type;
 	int err2;
 	int error;
-	bool is_dir = (lookup_flags & LOOKUP_DIRECTORY);
 
 	/*
 	 * Note that only LOOKUP_REVAL and LOOKUP_DIRECTORY matter here. Any
 	 * other flags passed in are ignored!
 	 */
-	lookup_flags &= LOOKUP_REVAL;
+	lookup_flags &= LOOKUP_REVAL | LOOKUP_DIRECTORY;
 
 	error = filename_parentat(dfd, name, lookup_flags, path, &last, &type);
 	if (error)
@@ -3698,9 +3697,11 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	/* don't fail immediately if it's r/o, at least try to report other errors */
 	err2 = mnt_want_write(path->mnt);
 	/*
-	 * Do the final lookup.
+	 * Do the final lookup.  Request 'create' only if there is no trailing
+	 * '/', or if directory is requested.
 	 */
-	lookup_flags |= LOOKUP_CREATE | LOOKUP_EXCL;
+	if (!last.name[last.len] || (lookup_flags & LOOKUP_DIRECTORY))
+		lookup_flags |= LOOKUP_CREATE | LOOKUP_EXCL;
 	inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
 	dentry = __lookup_hash(&last, path->dentry, lookup_flags);
 	if (IS_ERR(dentry))
@@ -3716,7 +3717,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	 * all is fine. Let's be bastards - you had / on the end, you've
 	 * been asking for (non-existent) directory. -ENOENT for you.
 	 */
-	if (unlikely(!is_dir && last.name[last.len])) {
+	if (!likely(lookup_flags & LOOKUP_CREATE)) {
 		error = -ENOENT;
 		goto fail;
 	}
-- 
2.35.1