[RFC PATCH] fs: touch up symlink clean up in lookup

[RFC PATCH] fs: touch up symlink clean up in lookup

[Mateusz Guzik]

Provide links_cleanup_rcu() and links_cleanup_ref() for rcu- and ref-
walks respectively.

The rather misleading drop_links() gets renamed to links_issue_delayed_calls(),
which spells out what it is actually doing.

Finally ->depth zeroing is moved to a place where symlinks are sorted out.

There are no changes in behavior, this however should be less
error-prone going forward.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---

this assumes the LOOKUP_CACHED fixup is applied:
https://lore.kernel.org/linux-fsdevel/20251217104854.GU1712166@ZenIV/T/#mff14d1dd88729f40fa94ada8beaa64e0c41097ff

technically the 2 patches could be combined, but I did not want to do it
given the bug

this booted, so it must be fine(tm)

Chris, can you feed this thing into the magic llm? Better yet, is it
something I can use myself? I passed the known buggy patch to gcc
-fanalyze and clang --analyze and neither managed to point out any
issues.

 fs/namei.c | 54 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 69d0aa9ad2a8..af6d111d6ccb 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -766,7 +766,7 @@ static bool path_connected(struct vfsmount *mnt, struct dentry *dentry)
 	return is_subdir(dentry, mnt->mnt_root);
 }
 
-static void drop_links(struct nameidata *nd)
+static void links_issue_delayed_calls(struct nameidata *nd)
 {
 	int i = nd->depth;
 	while (i--) {
@@ -774,6 +774,35 @@ static void drop_links(struct nameidata *nd)
 		do_delayed_call(&last->done);
 		clear_delayed_call(&last->done);
 	}
+}
+
+static void links_cleanup_rcu(struct nameidata *nd)
+{
+	VFS_BUG_ON(!(nd->flags & LOOKUP_RCU));
+
+	if (likely(!nd->depth))
+		return;
+
+	links_issue_delayed_calls(nd);
+	nd->depth = 0;
+}
+
+static void links_cleanup_ref(struct nameidata *nd)
+{
+	VFS_BUG_ON(nd->flags & LOOKUP_RCU);
+
+	if (likely(!nd->depth))
+		return;
+
+	links_issue_delayed_calls(nd);
+
+	path_put(&nd->path);
+	for (int i = 0; i < nd->depth; i++)
+		path_put(&nd->stack[i].link);
+	if (nd->state & ND_ROOT_GRABBED) {
+		path_put(&nd->root);
+		nd->state &= ~ND_ROOT_GRABBED;
+	}
 	nd->depth = 0;
 }
 
@@ -786,20 +815,11 @@ static void leave_rcu(struct nameidata *nd)
 
 static void terminate_walk(struct nameidata *nd)
 {
-	int depth = nd->depth;
-	if (unlikely(depth))
-		drop_links(nd);
-	if (!(nd->flags & LOOKUP_RCU)) {
-		int i;
-		path_put(&nd->path);
-		for (i = 0; i < depth; i++)
-			path_put(&nd->stack[i].link);
-		if (nd->state & ND_ROOT_GRABBED) {
-			path_put(&nd->root);
-			nd->state &= ~ND_ROOT_GRABBED;
-		}
-	} else {
+	if (nd->flags & LOOKUP_RCU) {
+		links_cleanup_rcu(nd);
 		leave_rcu(nd);
+	} else {
+		links_cleanup_ref(nd);
 	}
 	VFS_BUG_ON(nd->depth);
 	nd->path.mnt = NULL;
@@ -838,7 +858,7 @@ static bool legitimize_links(struct nameidata *nd)
 	for (i = 0; i < nd->depth; i++) {
 		struct saved *last = nd->stack + i;
 		if (unlikely(!legitimize_path(nd, &last->link, last->seq))) {
-			drop_links(nd);
+			links_issue_delayed_calls(nd);
 			nd->depth = i + 1;
 			return false;
 		}
@@ -884,7 +904,7 @@ static bool try_to_unlazy(struct nameidata *nd)
 	BUG_ON(!(nd->flags & LOOKUP_RCU));
 
 	if (unlikely(nd->flags & LOOKUP_CACHED)) {
-		drop_links(nd);
+		links_cleanup_rcu(nd);
 		goto out1;
 	}
 	if (unlikely(nd->depth && !legitimize_links(nd)))
@@ -923,7 +943,7 @@ static bool try_to_unlazy_next(struct nameidata *nd, struct dentry *dentry)
 	BUG_ON(!(nd->flags & LOOKUP_RCU));
 
 	if (unlikely(nd->flags & LOOKUP_CACHED)) {
-		drop_links(nd);
+		links_cleanup_rcu(nd);
 		goto out2;
 	}
 	if (unlikely(nd->depth && !legitimize_links(nd)))
-- 
2.48.1

Re: [RFC PATCH] fs: touch up symlink clean up in lookup

[Al Viro]

On Wed, Dec 17, 2025 at 12:01:05PM +0100, Mateusz Guzik wrote:

This is obviously broken.  

> +static void links_cleanup_ref(struct nameidata *nd)
> +{
> +	VFS_BUG_ON(nd->flags & LOOKUP_RCU);
> +
> +	if (likely(!nd->depth))
> +		return;
> +
> +	links_issue_delayed_calls(nd);
> +
> +	path_put(&nd->path);
	^^^^^^^^^^^^^^^^^^^

> +	for (int i = 0; i < nd->depth; i++)
> +		path_put(&nd->stack[i].link);

> +	if (nd->state & ND_ROOT_GRABBED) {
> +		path_put(&nd->root);
		^^^^^^^^^^^^^^^^^^^
> +		nd->state &= ~ND_ROOT_GRABBED;
> +	}

has nothing whatsoever to with links *AND* should not be
skipped even with no symlinks in the vicinity.

NAK.