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 8f699c67561fa..2775e95c8e8c3 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1925,6 +1925,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; @@ -2009,6 +2010,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) { @@ -2034,6 +2037,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);