linux-kernel - Re: [PATCH] fs/namei: Remove redundant DCACHE_MANAGED_DENTRY check in __follow_mount

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <h6gfebegbbtqdjefr52kqdvfjlnpq4euzrq25mw4mdkapa2cfq@dy73qj5go474>
Date: Wed, 7 Jan 2026 15:44:27 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Breno Leitao <leitao@...ian.org>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, jlayton@...nel.org, rostedt@...dmis.org, kernel-team@...a.com
Subject: Re: [PATCH] fs/namei: Remove redundant DCACHE_MANAGED_DENTRY check
 in __follow_mount_rcu

On Mon, Jan 05, 2026 at 07:10:27AM -0800, Breno Leitao wrote:
> The check for DCACHE_MANAGED_DENTRY at the start of __follow_mount_rcu()
> is redundant because the only caller (handle_mounts) already verifies
> d_managed(dentry) before calling this function, so, dentry in
> __follow_mount_rcu() has always DCACHE_MANAGED_DENTRY set.
> 
> This early-out optimization never fires in practice - but it is marking
> as likely().
> 
> This was detected with branch profiling, which shows 100% misprediction
> in this likely.
> 
> Remove the whole if clause instead of removing the likely, given we
> know for sure that dentry is not DCACHE_MANAGED_DENTRY.
> 
> Signed-off-by: Breno Leitao <leitao@...ian.org>
> ---
>  fs/namei.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index bf0f66f0e9b9..774a2f5b0a10 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1623,9 +1623,6 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
>  	struct dentry *dentry = path->dentry;
>  	unsigned int flags = dentry->d_flags;
>  
> -	if (likely(!(flags & DCACHE_MANAGED_DENTRY)))
> -		return true;
> -

This makes me very uneasy.

You are seeing 100% misses on this one because you are never racing
against someone mounting and umounting on the dentry as you are doing
the lookup.

As in it is possible that by the time __follow_mount_rcu is invoked,
DCACHE_MANAGED_DENTRY is no longer set and with the check removed the
rest of the routine keeps executing.

AFAICS this turns harmless as is anyway, but I don't think that's safe
to rely on future-wise and more imporantly it is trivially avoidable.

I did not do it at the time because there are no d_ macros which operate
on already read flags and I could not be bothered to add them. In
retrospect a bad call, should have went with it and kept the open-coded
DCACHE_MANAGED_DENTRY check.

something like this:

diff --git a/fs/namei.c b/fs/namei.c
index bf0f66f0e9b9..c6279f8023cf 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1618,13 +1618,11 @@ EXPORT_SYMBOL(follow_down);
  * Try to skip to top of mountpoint pile in rcuwalk mode.  Fail if
  * we meet a managed dentry that would need blocking.
  */
-static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
+static bool __follow_mount_rcu(struct nameidata *nd, struct path *path, int flags)
 {
 	struct dentry *dentry = path->dentry;
-	unsigned int flags = dentry->d_flags;
 
-	if (likely(!(flags & DCACHE_MANAGED_DENTRY)))
-		return true;
+	VFS_BUG_ON(!(flags & DCACHE_MANAGED_DENTRY));
 
 	if (unlikely(nd->flags & LOOKUP_NO_XDEV))
 		return false;
@@ -1672,9 +1670,10 @@ static inline int handle_mounts(struct nameidata *nd, struct dentry *dentry,
 	path->dentry = dentry;
 	if (nd->flags & LOOKUP_RCU) {
 		unsigned int seq = nd->next_seq;
-		if (likely(!d_managed(dentry)))
+		unsigned int flags = READ_ONCE(dentry->d_flags);
+		if (likely(!(dentry->d_flags & DCACHE_MANAGED_DENTRY)))
 			return 0;
-		if (likely(__follow_mount_rcu(nd, path)))
+		if (likely(__follow_mount_rcu(nd, path, flags)))
 			return 0;
 		// *path and nd->next_seq might've been clobbered
 		path->mnt = nd->path.mnt;