[PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[Jeff Layton]

If d_flags isn't what we expect, then it's good to display it. Add a new
DENTRY_WARN_ONCE() macro that also displays d_flags for the dentry.
Change D_FLAG_VERIFY() to call that instead of a generic WARN_ON_ONCE().

Change the existing hlist_unhashed() check in dentry_free() to use the
new macro, and add checks for other invariants of a dead dentry. Notably:

1) Ensure that DCACHE_LRU_LIST and DCACHE_SHRINK_LIST are not set.

2) Ensure that d_lockref is negative

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
While chatting with Al about this elusive UAF problem, we both noted
that it would be nice to know what d_flags are when these warnings pop.
This adds that, and checks for some other invariants in dentry_free().
---
 fs/dcache.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 2c61aeea41f4..210df5c0a1f0 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -426,9 +426,16 @@ static inline void __d_clear_type_and_inode(struct dentry *dentry)
 		this_cpu_inc(nr_dentry_negative);
 }
 
+#define DENTRY_WARN_ONCE(condition, dentry) \
+	WARN_ONCE((condition), "dentry=%p d_flags=0x%x\n", (dentry), (dentry)->d_flags)
+#define D_FLAG_VERIFY(dentry, x) \
+	DENTRY_WARN_ONCE(((dentry)->d_flags & (DCACHE_LRU_LIST | DCACHE_SHRINK_LIST)) != (x), (dentry))
+
 static void dentry_free(struct dentry *dentry)
 {
-	WARN_ON(d_really_is_positive(dentry));
+	DENTRY_WARN_ONCE(d_really_is_positive(dentry), dentry);
+	DENTRY_WARN_ONCE(dentry->d_lockref.count >= 0, dentry);
+	D_FLAG_VERIFY(dentry, 0);
 	if (unlikely(dname_external(dentry))) {
 		struct external_name *p = external_name(dentry);
 		if (likely(atomic_dec_and_test(&p->count))) {
@@ -495,7 +502,6 @@ static void dentry_unlink_inode(struct dentry * dentry)
  * These helper functions make sure we always follow the
  * rules. d_lock must be held by the caller.
  */
-#define D_FLAG_VERIFY(dentry,x) WARN_ON_ONCE(((dentry)->d_flags & (DCACHE_LRU_LIST | DCACHE_SHRINK_LIST)) != (x))
 static void d_lru_add(struct dentry *dentry)
 {
 	D_FLAG_VERIFY(dentry, 0);

---
base-commit: 4ee64205ffaa587e8114d84a67ac721399ccb369
change-id: 20260422-dcache-warn-31bd78e18dc3

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[Christian Brauner]

On Wed, Apr 22, 2026 at 07:29:48AM -0400, Jeff Layton wrote:
> If d_flags isn't what we expect, then it's good to display it. Add a new
> DENTRY_WARN_ONCE() macro that also displays d_flags for the dentry.
> Change D_FLAG_VERIFY() to call that instead of a generic WARN_ON_ONCE().
> 
> Change the existing hlist_unhashed() check in dentry_free() to use the
> new macro, and add checks for other invariants of a dead dentry. Notably:
> 
> 1) Ensure that DCACHE_LRU_LIST and DCACHE_SHRINK_LIST are not set.
> 
> 2) Ensure that d_lockref is negative
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> While chatting with Al about this elusive UAF problem, we both noted
> that it would be nice to know what d_flags are when these warnings pop.
> This adds that, and checks for some other invariants in dentry_free().
> ---
>  fs/dcache.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 2c61aeea41f4..210df5c0a1f0 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -426,9 +426,16 @@ static inline void __d_clear_type_and_inode(struct dentry *dentry)
>  		this_cpu_inc(nr_dentry_negative);
>  }
>  
> +#define DENTRY_WARN_ONCE(condition, dentry) \
> +	WARN_ONCE((condition), "dentry=%p d_flags=0x%x\n", (dentry), (dentry)->d_flags)
> +#define D_FLAG_VERIFY(dentry, x) \
> +	DENTRY_WARN_ONCE(((dentry)->d_flags & (DCACHE_LRU_LIST | DCACHE_SHRINK_LIST)) != (x), (dentry))

Would be nice if we had a bunch of dentry debug assert macros in
vfsdebug.h like we have for VFS_BUG_ON_INODE()/VFS_WARN_ON_INODE() in
general imo.

Re: [PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[Christian Brauner]

On Wed, 22 Apr 2026 07:29:48 -0400, Jeff Layton wrote:
> If d_flags isn't what we expect, then it's good to display it. Add a new
> DENTRY_WARN_ONCE() macro that also displays d_flags for the dentry.
> Change D_FLAG_VERIFY() to call that instead of a generic WARN_ON_ONCE().
> 
> Change the existing hlist_unhashed() check in dentry_free() to use the
> new macro, and add checks for other invariants of a dead dentry. Notably:
> 
> [...]

Applied to the vfs-7.2.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.2.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.2.misc

[1/1] dcache: add extra sanity checks of the dentry in dentry_free()
      https://git.kernel.org/vfs/vfs/c/94607391f5ae

Re: [PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[Jori Koolstra]

On Wed, Apr 22, 2026 at 07:29:48AM -0400, Jeff Layton wrote:
>  
> +#define DENTRY_WARN_ONCE(condition, dentry) \
> +	WARN_ONCE((condition), "dentry=%p d_flags=0x%x\n", (dentry), (dentry)->d_flags)
> 

I guess this is one of those instances where using %p is acceptable?
Maybe Documentation/process/deprecated.rst is worded a bit too harshly.

Re: [PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[Jeff Layton]

On Wed, 2026-04-22 at 15:16 +0200, Jori Koolstra wrote:
> On Wed, Apr 22, 2026 at 07:29:48AM -0400, Jeff Layton wrote:
> >  
> > +#define DENTRY_WARN_ONCE(condition, dentry) \
> > +	WARN_ONCE((condition), "dentry=%p d_flags=0x%x\n", (dentry), (dentry)->d_flags)
> > 
> 
> I guess this is one of those instances where using %p is acceptable?
> Maybe Documentation/process/deprecated.rst is worded a bit too harshly.

I'm not opposed to using something different, but I think the hashed
pointer value is fine here. Mostly I just want some way to tell whether
different warnings might have been triggered by the same dentry.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[Jeff Layton]

On Wed, 2026-04-22 at 15:06 +0200, Christian Brauner wrote:
> On Wed, Apr 22, 2026 at 07:29:48AM -0400, Jeff Layton wrote:
> > If d_flags isn't what we expect, then it's good to display it. Add a new
> > DENTRY_WARN_ONCE() macro that also displays d_flags for the dentry.
> > Change D_FLAG_VERIFY() to call that instead of a generic WARN_ON_ONCE().
> > 
> > Change the existing hlist_unhashed() check in dentry_free() to use the
> > new macro, and add checks for other invariants of a dead dentry. Notably:
> > 
> > 1) Ensure that DCACHE_LRU_LIST and DCACHE_SHRINK_LIST are not set.
> > 
> > 2) Ensure that d_lockref is negative
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > While chatting with Al about this elusive UAF problem, we both noted
> > that it would be nice to know what d_flags are when these warnings pop.
> > This adds that, and checks for some other invariants in dentry_free().
> > ---
> >  fs/dcache.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/dcache.c b/fs/dcache.c
> > index 2c61aeea41f4..210df5c0a1f0 100644
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -426,9 +426,16 @@ static inline void __d_clear_type_and_inode(struct dentry *dentry)
> >  		this_cpu_inc(nr_dentry_negative);
> >  }
> >  
> > +#define DENTRY_WARN_ONCE(condition, dentry) \
> > +	WARN_ONCE((condition), "dentry=%p d_flags=0x%x\n", (dentry), (dentry)->d_flags)
> > +#define D_FLAG_VERIFY(dentry, x) \
> > +	DENTRY_WARN_ONCE(((dentry)->d_flags & (DCACHE_LRU_LIST | DCACHE_SHRINK_LIST)) != (x), (dentry))
> 
> Would be nice if we had a bunch of dentry debug assert macros in
> vfsdebug.h like we have for VFS_BUG_ON_INODE()/VFS_WARN_ON_INODE() in
> general imo.

Good point. I guess I should have called this VFS_WARN_ON_DENTRY().

There are some other existing warnings in dcache.c that could be
converted to use this too.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] dcache: add extra sanity checks of the dentry in dentry_free()

[Jori Koolstra]

> Op 22-04-2026 15:53 CEST schreef Jeff Layton <jlayton@kernel.org>:
> 
>  
> On Wed, 2026-04-22 at 15:16 +0200, Jori Koolstra wrote:
> > On Wed, Apr 22, 2026 at 07:29:48AM -0400, Jeff Layton wrote:
> > >  
> > > +#define DENTRY_WARN_ONCE(condition, dentry) \
> > > +	WARN_ONCE((condition), "dentry=%p d_flags=0x%x\n", (dentry), (dentry)->d_flags)
> > > 
> > 
> > I guess this is one of those instances where using %p is acceptable?
> > Maybe Documentation/process/deprecated.rst is worded a bit too harshly.
> 
> I'm not opposed to using something different, but I think the hashed
> pointer value is fine here. Mostly I just want some way to tell whether
> different warnings might have been triggered by the same dentry.

Yes, I get that. And there isn't really any other clear marker for struct dentry
AFAIK. Also, this is probably better than using %pd.

Feel free to add:

Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>

> -- 
> Jeff Layton <jlayton@kernel.org>