[PATCH V4] fs/fuse: fix race between concurrent setattr from multiple nodes

[PATCH V4] fs/fuse: fix race between concurrent setattr from multiple nodes

[Guang Yuan Wu]

    fuse: fix race between concurrent setattrs from multiple nodes
    
    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>

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

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 83ac192e7fdd..a961c3ed7b26 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1946,6 +1946,8 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	int err;
 	bool trust_local_cmtime = is_wb;
 	bool fault_blocked = false;
+	bool invalid_attr = false;
+	u64 attr_version;
 
 	if (!fc->default_permissions)
 		attr->ia_valid |= ATTR_FORCE;
@@ -2030,6 +2032,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,8 +2059,14 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		/* FIXME: clear I_DIRTY_SYNC? */
 	}
 
+	if (attr_version != 0 && fi->attr_version > attr_version)
+		/* Applying attributes, for example for fsnotify_change(), and
+		 * set i_time with 0 as attributes timeout value.
+		 */
+		invalid_attr = true;
+
 	fuse_change_attributes_common(inode, &outarg.attr, NULL,
-				      ATTR_TIMEOUT(&outarg),
+				      invalid_attr ? 0 : ATTR_TIMEOUT(&outarg),
 				      fuse_get_cache_mask(inode), 0);
 	oldsize = inode->i_size;
 	/* see the comment in fuse_change_attributes() */

Re: [PATCH V4] fs/fuse: fix race between concurrent setattr from multiple nodes

[Miklos Szeredi]

On Fri, 2 May 2025 at 06:04, Guang Yuan Wu <gwu@ddn.com> wrote:
>
>     fuse: fix race between concurrent setattrs from multiple nodes
>
>     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>

Applied with minor modification (see fuse.git#for-next).  Thanks.

Miklos