[PATCH AUTOSEL 6.15 17/21] fuse: fix race between concurrent setattrs from multiple nodes

[Sasha Levin <sashal@kernel.org>]

From: Guang Yuan Wu <gwu@ddn.com>

[ Upstream commit 69efbff69f89c9b2b72c4d82ad8b59706add768a ]

When mounting a user-space filesystem on multiple clients, after
concurrent ->setattr() calls from different node, stale inode
attributes may be cached in some node.

This is caused by fuse_setattr() racing with
fuse_reverse_inval_inode().

When filesystem server receives setattr request, the client node
with valid iattr cached will be required to update the fuse_inode's
attr_version and invalidate the cache by fuse_reverse_inval_inode(),
and at the next call to ->getattr() they will be fetched from user
space.

The race scenario is:
1. client-1 sends setattr (iattr-1) request to server
2. client-1 receives the reply from server
3. before client-1 updates iattr-1 to the cached attributes by
   fuse_change_attributes_common(), server receives another setattr
   (iattr-2) request from client-2
4. server requests client-1 to update the inode attr_version and
   invalidate the cached iattr, and iattr-1 becomes staled
5. client-2 receives the reply from server, and caches iattr-2
6. continue with step 2, client-1 invokes
   fuse_change_attributes_common(), and caches iattr-1

The issue has been observed from concurrent of chmod, chown, or
truncate, which all invoke ->setattr() call.

The solution is to use fuse_inode's attr_version to check whether
the attributes have been modified during the setattr request's
lifetime.  If so, mark the attributes as invalid in the function
fuse_change_attributes_common().

Signed-off-by: Guang Yuan Wu <gwu@ddn.com>
Reviewed-by: Bernd Schubert <bschubert@ddn.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Now I have enough context to analyze this commit. Let me examine the
core changes and understand the race condition:

**YES**

This commit should be backported to stable kernel trees. Here's my
extensive analysis:

## Bug Analysis

**1. Race Condition Description:**
The commit fixes a serious race condition in FUSE between
`fuse_setattr()` and `fuse_reverse_inval_inode()` when multiple clients
mount the same filesystem. The race occurs when:

1. Client-1 sends setattr request to server
2. Client-1 receives reply but before applying attributes via
   `fuse_change_attributes_common()`
3. Server receives another setattr from client-2
4. Server calls `fuse_reverse_inval_inode()` on client-1, which bumps
   `attr_version` (line 567 in inode.c: `fi->attr_version =
   atomic64_inc_return(&fc->attr_version)`)
5. Client-1 continues with step 2, applying now-stale attributes

**2. Code Changes Analysis:**

The fix adds two key changes to `fuse_do_setattr()` in fs/fuse/dir.c:

```c
+       u64 attr_version;
...
+       attr_version = fuse_get_attr_version(fm->fc);
        fuse_setattr_fill(fc, &args, inode, &inarg, &outarg);
        err = fuse_simple_request(fm, &args);
...
+       if (fi->attr_version > attr_version) {
+               /*
+                * Apply attributes, for example for fsnotify_change(),
but set
+                * attribute timeout to zero.
+                */
+               outarg.attr_valid = outarg.attr_valid_nsec = 0;
+       }
```

This captures the `attr_version` before the request and checks if it
changed during the request lifetime. If it did, the fix sets the
attribute timeout to zero, effectively marking them invalid.

## Backport Criteria Assessment

**1. Important Bug Fix:** ✅ YES
- Fixes data consistency issues in multi-client FUSE scenarios
- Can cause stale cached attributes (file size, permissions, timestamps)
- Affects critical operations: chmod, chown, truncate

**2. Minimal and Contained:** ✅ YES
- Very small change: ~10 lines added
- Localized to `fuse_do_setattr()` function only
- No architectural changes

**3. Low Regression Risk:** ✅ YES
- Uses existing FUSE infrastructure (`attr_version` mechanism)
- Conservative approach: when in doubt, invalidate attributes
- No changes to external APIs or data structures

**4. Critical Subsystem:** ✅ MODERATE CONCERN, BUT ACCEPTABLE
- FUSE is important but this is a targeted fix
- Only affects multi-client scenarios
- Doesn't change core VFS interactions

**5. Clear Side Effects:** ✅ MINIMAL
- May cause slightly more GETATTR requests when races occur
- But this is the correct behavior for consistency

## Comparison with Similar Commits

This commit follows the same pattern as similar commit #1 (marked YES
for backport), which also:
- Fixed attribute cache consistency in FUSE
- Used the `attr_version` mechanism
- Made minimal, targeted changes
- Fixed race conditions between operations

Unlike commits #2-#5 (marked NO), this commit:
- Doesn't introduce new features or optimizations
- Doesn't change architectural patterns
- Fixes a clear data corruption/consistency bug

## Conclusion

This commit perfectly fits stable tree criteria: it fixes an important
multi-client data consistency bug with minimal risk through a small,
well-contained change. The race condition can lead to stale file
attributes being cached, which is a serious issue for applications
relying on accurate metadata. The fix follows established FUSE patterns
and is conservative in approach.

 fs/fuse/dir.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 83ac192e7fdd1..fa90309030e21 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1946,6 +1946,7 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	int err;
 	bool trust_local_cmtime = is_wb;
 	bool fault_blocked = false;
+	u64 attr_version;
 
 	if (!fc->default_permissions)
 		attr->ia_valid |= ATTR_FORCE;
@@ -2030,6 +2031,8 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		if (fc->handle_killpriv_v2 && !capable(CAP_FSETID))
 			inarg.valid |= FATTR_KILL_SUIDGID;
 	}
+
+	attr_version = fuse_get_attr_version(fm->fc);
 	fuse_setattr_fill(fc, &args, inode, &inarg, &outarg);
 	err = fuse_simple_request(fm, &args);
 	if (err) {
@@ -2055,6 +2058,14 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		/* FIXME: clear I_DIRTY_SYNC? */
 	}
 
+	if (fi->attr_version > attr_version) {
+		/*
+		 * Apply attributes, for example for fsnotify_change(), but set
+		 * attribute timeout to zero.
+		 */
+		outarg.attr_valid = outarg.attr_valid_nsec = 0;
+	}
+
 	fuse_change_attributes_common(inode, &outarg.attr, NULL,
 				      ATTR_TIMEOUT(&outarg),
 				      fuse_get_cache_mask(inode), 0);
-- 
2.39.5