KCSAN: data-race in path_lookupat / vfs_rename

["Jianzhou Zhao" <luckd0g@163.com>]

Subject: [BUG] fs/namei: KCSAN: data-race in path_lookupat / vfs_rename (`d_flags`)

Dear VFS Maintainers,

Our fuzzing tool, RacePilot, has detected a data race in the VFS subsystem on a 6.18 kernel version (`6.18.0-08691-g2061f18ad76e-dirty`). 

The race occurs between `vfs_rename()` modifying a victim dentry's `d_flags` via `dont_mount()` and a concurrent path lookup (`path_lookupat()`) executing lockless reads on the same `d_flags` via `d_managed()`.

### Call Trace & Context

```
BUG: KCSAN: data-race in path_lookupat / vfs_rename

write to 0xffff88802d62b180 of 4 bytes by task 4204 on cpu 0:
 dont_mount include/linux/dcache.h:391 [inline]
 vfs_rename+0xf03/0x13a0 fs/namei.c:5963
 do_renameat2+0x6a9/0x8e0 fs/namei.c:6072
 ...

read to 0xffff88802d62b180 of 4 bytes by task 4691 on cpu 1:
 d_managed include/linux/dcache.h:412 [inline]
 step_into fs/namei.c:2102 [inline]
 walk_component fs/namei.c:2258 [inline]
 lookup_last fs/namei.c:2753 [inline]
 path_lookupat+0x1fc/0x740 fs/namei.c:2777
 ...

value changed: 0x00300080 -> 0x00004180
```

#### Execution Flow & Code Context:

**1. The Write Path (`vfs_rename` -> `dont_mount`)**

During a rename operation, if a target (victim) dentry is going to be replaced, `vfs_rename` unmounts any mounts attached to it:
```c
5978: 	if (!(flags & RENAME_EXCHANGE) && target) {
5979: 		if (is_dir) {
5980: 			shrink_dcache_parent(new_dentry);
5981: 			target->i_flags |= S_DEAD;
5982: 		}
5983: 		dont_mount(new_dentry); // <--- Invokes dont_mount on the victim
5984: 		detach_mounts(new_dentry);
5985: 	}
```

Inside `dont_mount(struct dentry *dentry)` (`include/linux/dcache.h`), `d_flags` is updated while holding the `d_lock`:
```c
388: static inline void dont_mount(struct dentry *dentry)
389: {
390: 	spin_lock(&dentry->d_lock);
391: 	dentry->d_flags |= DCACHE_CANT_MOUNT; // <--- Read-Modify-Write
392: 	spin_unlock(&dentry->d_lock);
393: }
```

**2. The Read Path (`path_lookupat` -> `step_into` -> `d_managed`)**

Concurrently, a separate thread traversing the filesystem performs a path lookup and inspects the same intermediate dentry (the rename victim) within `step_into()`. It tests if the dentry needs mount-point translation by calling `d_managed()`:
```c
412: static inline bool d_managed(const struct dentry *dentry)
413: {
414: 	return dentry->d_flags & DCACHE_MANAGED_DENTRY; // <--- Lockless Read
415: }
```

### Root Cause Analysis

The data race is triggered because `d_managed()` executes a plain read of `dentry->d_flags` without holding `d_lock` (or using `READ_ONCE`), while `dont_mount()` concurrently modifies `dentry->d_flags` using a plain read-modify-write operation (`|= DCACHE_CANT_MOUNT`). 

Although `dont_mount()` properly protects the write within a `spin_lock`, the lockless reader is oblivious to it. KCSAN identifies this as a data race because the plain read in `d_managed()` can overlap with the unlocked portion of the compiler's emitted store sequence in another CPU.

Regrettably, we were unable to create a reproduction program for this bug.

### Potential Impact

While a torn read of a 32-bit aligned integer is highly unlikely on most modern architectures, plain concurrent access constitutes undefined behavior under the C memory model. The compiler might apply unforeseen transformations capable of producing a stale or transient state representation. However, pragmatically speaking, since the victim dentry is concurrently being purged/renamed, misinterpreting the managed flags would likely lead to subsequent validation failures upstream within namei.c, causing the traversal to bounce to slow paths safely. Therefore, we evaluate the severity of this issue as a minor data race that primarily risks undefined behavior and triggers KCSAN warnings.

### Proposed Fix

To silence the KCSAN warning and adhere securely to kernel concurrency practices, the lockless read of `d_flags` in `d_managed()` should be explicitly annotated using `data_race()` or, more robustly, `READ_ONCE()`. Using `READ_ONCE()` guarantees single-instruction load semantics preventing compiler optimizations that might induce tearing.

```diff
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -411,7 +411,7 @@ extern void dput(struct dentry *);
 
 static inline bool d_managed(const struct dentry *dentry)
 {
-	return dentry->d_flags & DCACHE_MANAGED_DENTRY;
+	return READ_ONCE(dentry->d_flags) & DCACHE_MANAGED_DENTRY;
 }
```
(Alternatively, using `data_race()` if no load-tearing dangers exist in theory on the supported architectures, depending on VFS lockless scaling conventions for d_flags).

We would be highly honored if this could be of any help.

Best regards,
[Your Name/Team]
RacePilot Team