[PATCH] fs: refuse to drop a dentry reference d_make_persistent() never took

Narek Jilavyan <[email protected]>
Newsgroups gmane.linux.file-systems,gmane.linux.kernel
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.