diff mbox series

[6/7] namei: clean up do_linkat retry logic

Message ID 20210712123649.1102392-7-dkadashev@gmail.com (mailing list archive)
State New, archived
Headers show
Series namei: clean up retry logic in various do_* functions | expand

Commit Message

Dmitry Kadashev July 12, 2021, 12:36 p.m. UTC 
Moving the main logic to a helper function makes the whole thing much
easier to follow.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <christian.brauner@ubuntu.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/io-uring/CAHk-=wiG+sN+2zSoAOggKCGue2kOJvw3rQySvQXsZstRQFTN+g@mail.gmail.com/
Signed-off-by: Dmitry Kadashev <dkadashev@gmail.com>
---
 fs/namei.c | 80 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 36 deletions(-)

Comments

Linus Torvalds July 12, 2021, 6:57 p.m. UTC | #1 
On Mon, Jul 12, 2021 at 5:37 AM Dmitry Kadashev <dkadashev@gmail.com> wrote:
>
> Moving the main logic to a helper function makes the whole thing much
> easier to follow.

Ok, this has the same thing as the previous patches had.

I see why the old code tried to avoid the repeat of some tests, but
honestly, that "retry_estale()" might as well be marked "unlikely()",
and we might as well do the test again if it triggers.

That said, in this case we actually end up doing other things too in
"do_linkat()", so I guess it could go either way.

              Linus
diff mbox series

Patch

diff --git a/fs/namei.c b/fs/namei.c
index c9110ac83ccb..5e4fa8b65a8d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4380,48 +4380,22 @@  int vfs_link(struct dentry *old_dentry, struct user_namespace *mnt_userns,
 }
 EXPORT_SYMBOL(vfs_link);
 
-/*
- * Hardlinks are often used in delicate situations.  We avoid
- * security-related surprises by not following symlinks on the
- * newname.  --KAB
- *
- * We don't follow them on the oldname either to be compatible
- * with linux 2.0, and to avoid hard-linking to directories
- * and other special files.  --ADM
- */
-int do_linkat(int olddfd, struct filename *old, int newdfd,
-	      struct filename *new, int flags)
+static int linkat_helper(int olddfd, struct filename *old, int newdfd,
+			 struct filename *new, unsigned int lookup_flags)
 {
 	struct user_namespace *mnt_userns;
 	struct dentry *new_dentry;
 	struct path old_path, new_path;
 	struct inode *delegated_inode = NULL;
-	int how = 0;
 	int error;
 
-	if ((flags & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH)) != 0) {
-		error = -EINVAL;
-		goto out_putnames;
-	}
-	/*
-	 * To use null names we require CAP_DAC_READ_SEARCH
-	 * This ensures that not everyone will be able to create
-	 * handlink using the passed filedescriptor.
-	 */
-	if (flags & AT_EMPTY_PATH && !capable(CAP_DAC_READ_SEARCH)) {
-		error = -ENOENT;
-		goto out_putnames;
-	}
-
-	if (flags & AT_SYMLINK_FOLLOW)
-		how |= LOOKUP_FOLLOW;
 retry:
-	error = __filename_lookup(olddfd, old, how, &old_path, NULL);
+	error = __filename_lookup(olddfd, old, lookup_flags, &old_path, NULL);
 	if (error)
-		goto out_putnames;
+		return error;
 
 	new_dentry = __filename_create(newdfd, new, &new_path,
-					(how & LOOKUP_REVAL));
+					(lookup_flags & LOOKUP_REVAL));
 	error = PTR_ERR(new_dentry);
 	if (IS_ERR(new_dentry))
 		goto out_putpath;
@@ -4447,14 +4421,48 @@  int do_linkat(int olddfd, struct filename *old, int newdfd,
 			goto retry;
 		}
 	}
+out_putpath:
+	path_put(&old_path);
+	return error;
+}
+
+/*
+ * Hardlinks are often used in delicate situations.  We avoid
+ * security-related surprises by not following symlinks on the
+ * newname.  --KAB
+ *
+ * We don't follow them on the oldname either to be compatible
+ * with linux 2.0, and to avoid hard-linking to directories
+ * and other special files.  --ADM
+ */
+int do_linkat(int olddfd, struct filename *old, int newdfd,
+	      struct filename *new, int flags)
+{
+	int error, how = 0;
+
+	if ((flags & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH)) != 0) {
+		error = -EINVAL;
+		goto out;
+	}
+	/*
+	 * To use null names we require CAP_DAC_READ_SEARCH
+	 * This ensures that not everyone will be able to create
+	 * handlink using the passed filedescriptor.
+	 */
+	if (flags & AT_EMPTY_PATH && !capable(CAP_DAC_READ_SEARCH)) {
+		error = -ENOENT;
+		goto out;
+	}
+
+	if (flags & AT_SYMLINK_FOLLOW)
+		how |= LOOKUP_FOLLOW;
+
+	error = linkat_helper(olddfd, old, newdfd, new, how);
 	if (retry_estale(error, how)) {
-		path_put(&old_path);
 		how |= LOOKUP_REVAL;
-		goto retry;
+		error = linkat_helper(olddfd, old, newdfd, new, how);
 	}
-out_putpath:
-	path_put(&old_path);
-out_putnames:
+out:
 	putname(old);
 	putname(new);