Commit b3d4650d authored Apr 14, 2022 by NeilBrown Committed by Linus Torvalds Apr 14, 2022

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.

But 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 a 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 filename_parentat() is only interested in LOOKUP_REVAL, so we
split that out and store it in 'reval_flag'.  __lookup_hash() then gets
reval_flag combined with whatever create flags were determined to be
needed.

Reviewed-by: David Disseldorp <ddiss@suse.de>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

parent 115acbb5

fs/namei.c

+10 −12

Original line number	Diff line number	Diff line
		@@ -3673,18 +3673,14 @@ static struct dentry filename_create(int dfd, struct filename name,
		{
		struct dentry *dentry = ERR_PTR(-EEXIST);
		struct qstr last;
		bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
		unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
		unsigned int create_flags = LOOKUP_CREATE \| LOOKUP_EXCL;
		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;

		error = filename_parentat(dfd, name, lookup_flags, path, &last, &type);
		error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
		if (error)
		return ERR_PTR(error);

		@@ -3698,11 +3694,13 @@ 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. Suppress 'create' if there is a trailing
		* '/', and a directory wasn't requested.
		*/
		lookup_flags \|= LOOKUP_CREATE \| LOOKUP_EXCL;
		if (last.name[last.len] && !want_dir)
		create_flags = 0;
		inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
		dentry = __lookup_hash(&last, path->dentry, lookup_flags);
		dentry = __lookup_hash(&last, path->dentry, reval_flag \| create_flags);
		if (IS_ERR(dentry))
		goto unlock;

		@@ -3716,7 +3714,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 (unlikely(!create_flags)) {
		error = -ENOENT;
		goto fail;
		}