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

Jeff Layton posted 1 patch 1 month, 3 weeks ago
fs/dcache.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH] dcache: add extra sanity checks of the dentry in dentry_free()
Posted by Jeff Layton 1 month, 3 weeks ago
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()
Posted by Christian Brauner 1 month, 3 weeks ago
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()
Posted by Christian Brauner 1 month, 3 weeks ago
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()
Posted by Jeff Layton 1 month, 3 weeks ago
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>