fuse: fix race between concurrent setattrs from multiple nodes
[ 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>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
a13b2634dd
commit
96715eb1a1
@@ -1860,6 +1860,7 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
|
|||||||
int err;
|
int err;
|
||||||
bool trust_local_cmtime = is_wb;
|
bool trust_local_cmtime = is_wb;
|
||||||
bool fault_blocked = false;
|
bool fault_blocked = false;
|
||||||
|
u64 attr_version;
|
||||||
|
|
||||||
if (!fc->default_permissions)
|
if (!fc->default_permissions)
|
||||||
attr->ia_valid |= ATTR_FORCE;
|
attr->ia_valid |= ATTR_FORCE;
|
||||||
@@ -1944,6 +1945,8 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
|
|||||||
if (fc->handle_killpriv_v2 && !capable(CAP_FSETID))
|
if (fc->handle_killpriv_v2 && !capable(CAP_FSETID))
|
||||||
inarg.valid |= FATTR_KILL_SUIDGID;
|
inarg.valid |= FATTR_KILL_SUIDGID;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
attr_version = fuse_get_attr_version(fm->fc);
|
||||||
fuse_setattr_fill(fc, &args, inode, &inarg, &outarg);
|
fuse_setattr_fill(fc, &args, inode, &inarg, &outarg);
|
||||||
err = fuse_simple_request(fm, &args);
|
err = fuse_simple_request(fm, &args);
|
||||||
if (err) {
|
if (err) {
|
||||||
@@ -1969,6 +1972,14 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
|
|||||||
/* FIXME: clear I_DIRTY_SYNC? */
|
/* 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,
|
fuse_change_attributes_common(inode, &outarg.attr, NULL,
|
||||||
ATTR_TIMEOUT(&outarg),
|
ATTR_TIMEOUT(&outarg),
|
||||||
fuse_get_cache_mask(inode));
|
fuse_get_cache_mask(inode));
|
||||||
|
Reference in New Issue
Block a user