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@xxxxxxx>
Reviewed-by: Bernd Schubert <bschubert@xxxxxxx>
---
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() */
