[PATCH] fs/dcache: annotate the lockless d_unhashed() check in retain_dentry()

[Nguyen Ngoc Thang]

KCSAN reports a data race between __d_drop() clearing d_hash.pprev
and the d_unhashed() test in retain_dentry(), reached from fast_dput()
without ->d_lock held:

  BUG: KCSAN: data-race in dentry_kill / fast_dput

  write to 0xffff888107394e50 of 8 bytes by task 3667 on cpu 0:
   __d_drop fs/dcache.c:578 [inline]
   dentry_kill+0x139/0x500 fs/dcache.c:824
   finish_dput+0x16/0x1d0 fs/dcache.c:1001
   dput+0x48/0x50 fs/dcache.c:1042
   lookup_one_qstr_excl+0x156/0x240
   filename_unlinkat+0x149/0x410

  read to 0xffff888107394e50 of 8 bytes by task 3668 on cpu 1:
   hlist_bl_unhashed include/linux/list_bl.h:54 [inline]
   d_unhashed include/linux/dcache.h:396 [inline]
   retain_dentry fs/dcache.c:872 [inline]
   fast_dput+0x64/0x300 fs/dcache.c:971
   dput+0x1f/0x50 fs/dcache.c:1040
   handle_mounts fs/namei.c:1753 [inline]
   step_into_slowpath+0x38c/0x4c0 fs/namei.c:2104
   path_lookupat+0x2eb/0x4c0
   do_readlinkat+0x74/0x2f0

  value changed: 0xffff8882376dc230 -> 0x0000000000000000

The race is intentional. As the comment above retain_dentry() says,
without ->d_lock the predicates are not stable, and it is enough that
the dentry was hashed at some point after the reference was dropped.
If the lockless reader sees a stale "hashed" value, the dentry is kept
with a zero refcount, exactly as if it had been dropped just after the
check. If it sees "unhashed", it takes ->d_lock and rechecks. A single
aligned pointer load cannot tear, so neither outcome is harmful.

Mark the read with data_race() to document this and silence KCSAN.
No functional change.

Reproduced with the syzbot C reproducer on a KCSAN kernel in QEMU;
with this patch the dentry_kill race no longer shows up.

Reported-by: syzbot+1cfd29c242c7cd26da02@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=1cfd29c242c7cd26da02
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@xxxxxxxxx>
---
 fs/dcache.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1b1a81f10da6..3eb3d7e7ab0f 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -868,8 +868,9 @@ static inline bool retain_dentry(struct dentry *dentry, bool locked)
 	smp_rmb();
 	d_flags = READ_ONCE(dentry->d_flags);
 
-	// Unreachable? Nobody would be able to look it up, no point retaining
-	if (unlikely(d_unhashed(dentry)))
+	// Unreachable? Nobody would be able to look it up, no point retaining.
+	// Racy when !locked: a stale answer only costs a recheck under ->d_lock.
+	if (unlikely(data_race(d_unhashed(dentry))))
 		return false;
 
 	// Same if it's disconnected
-- 
2.43.0