DCACHE_PERSISTENT records that d_make_persistent() took a reference on
the dentry with dget_dlock(). d_make_discardable() announces that
precondition but does not act on it:

	spin_lock(&dentry->d_lock);
	WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT));
	dentry->d_flags &= ~DCACHE_PERSISTENT;
	dentry->d_lockref.count--;
	finish_dput(dentry);

so a caller that never made the dentry persistent still has a reference
taken from it.  finish_dput() runs dentry_kill(), so the result is not a
stale flag but a d_lockref underflow that can free a dentry another
holder still references.

The same file already enforces the invariant rather than announcing it,
in select_collect_umount():

	if (dentry->d_flags & DCACHE_PERSISTENT) {
		dentry->d_flags &= ~DCACHE_PERSISTENT;
		dentry->d_lockref.count--;
	}

All seven in-tree callers are correctly paired (fs/libfs.c, fs/devpts,
fs/autofs, fs/tracefs).  Both d_make_persistent() and
d_make_discardable() are EXPORT_SYMBOL, so the contract is module-facing
and currently unenforced.

Make the check act.  On a mismatch the failure mode becomes a leaked
pinned dentry with a warning naming the caller, instead of a freed live
one.  The early return unlocks explicitly, since finish_dput() - which
normally releases d_lock - is no longer reached on that path.

Demonstrated with a late_initcall calling d_make_discardable() on a
d_alloc_name() dentry, which by construction lacks DCACHE_PERSISTENT.
Same kernel, only fs/dcache.c differs:

	unpatched:  count before=2 after=1	(reference dropped)
	patched:    count before=2 after=2	(refcount untouched)

Both kernels emit the warning; only the patched one declines to act on
it.

Fixes: bacdf1d70bbe ("primitives for maintaining persisitency")
Signed-off-by: Narek Jilavyan <njilav@gmail.com>
---
 fs/dcache.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3e9af9de7..897a100e7 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1046,7 +1046,16 @@ EXPORT_SYMBOL(dput);
 void d_make_discardable(struct dentry *dentry)
 {
 	spin_lock(&dentry->d_lock);
-	WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT));
+	/*
+	 * DCACHE_PERSISTENT records that d_make_persistent() took a reference.
+	 * If it is clear there is no such reference to return, and dropping one
+	 * anyway underflows d_lockref and frees a dentry someone else still
+	 * holds. Refuse instead, as select_collect_umount() already does.
+	 */
+	if (WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT))) {
+		spin_unlock(&dentry->d_lock);
+		return;
+	}
 	dentry->d_flags &= ~DCACHE_PERSISTENT;
 	dentry->d_lockref.count--;
 	finish_dput(dentry);
-- 
2.43.0

> DCACHE_PERSISTENT records that d_make_persistent() took a reference on
> the dentry with dget_dlock(). d_make_discardable() announces that
> precondition but does not act on it:
> 
> 	spin_lock(&dentry->d_lock);
> 	WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT));
> 	dentry->d_flags &= ~DCACHE_PERSISTENT;
> 	dentry->d_lockref.count--;
> 	finish_dput(dentry);
> 
> so a caller that never made the dentry persistent still has a reference
> taken from it.  finish_dput() runs dentry_kill(), so the result is not a
> stale flag but a d_lockref underflow that can free a dentry another
> holder still references.
> 
> The same file already enforces the invariant rather than announcing it,
> in select_collect_umount():
> 
> 	if (dentry->d_flags & DCACHE_PERSISTENT) {
> 		dentry->d_flags &= ~DCACHE_PERSISTENT;
> 		dentry->d_lockref.count--;
> 	}
> 
> All seven in-tree callers are correctly paired (fs/libfs.c, fs/devpts,
> fs/autofs, fs/tracefs).  Both d_make_persistent() and
> d_make_discardable() are EXPORT_SYMBOL, so the contract is module-facing
> and currently unenforced.
> 
> Make the check act.  On a mismatch the failure mode becomes a leaked
> pinned dentry with a warning naming the caller, instead of a freed live
> one.  The early return unlocks explicitly, since finish_dput() - which
> normally releases d_lock - is no longer reached on that path.
> 
> Demonstrated with a late_initcall calling d_make_discardable() on a
> d_alloc_name() dentry, which by construction lacks DCACHE_PERSISTENT.
> Same kernel, only fs/dcache.c differs:
> 
> 	unpatched:  count before=2 after=1	(reference dropped)
> 	patched:    count before=2 after=2	(refcount untouched)
> 
> Both kernels emit the warning; only the patched one declines to act on
> it.
> 
> Fixes: bacdf1d70bbe ("primitives for maintaining persisitency")
> Signed-off-by: Narek Jilavyan <njilav@gmail.com>

I struggle to the see the point in this change.

--