summaryrefslogtreecommitdiff
path: root/fs/9p/vfs_inode_dotl.c
diff options
context:
space:
mode:
authorJianyong Wu <jianyong.wu@arm.com>2020-09-23 22:11:46 +0800
committerDominique Martinet <asmadeus@codewreck.org>2020-11-19 17:20:39 +0100
commit6636b6dcc3db2258cd0585b8078c1c225c4b6dde (patch)
tree00d30096cadaafcee0cd79206fa0f6da315aa445 /fs/9p/vfs_inode_dotl.c
parent478ba09edc1f2f2ee27180a06150cb2d1a686f9c (diff)
9p: add refcount to p9_fid struct
Fix race issue in fid contention. Eric's and Greg's patch offer a mechanism to fix open-unlink-f*syscall bug in 9p. But there is race issue in fid parallel accesses. As Greg's patch stores all of fids from opened files into according inode, so all the lookup fid ops can retrieve fid from inode preferentially. But there is no mechanism to handle the fid contention issue. For example, there are two threads get the same fid in the same time and one of them clunk the fid before the other thread ready to discard the fid. In this scenario, it will lead to some fatal problems, even kernel core dump. I introduce a mechanism to fix this race issue. A counter field introduced into p9_fid struct to store the reference counter to the fid. When a fid is allocated from the inode or dentry, the counter will increase, and will decrease at the end of its occupation. It is guaranteed that the fid won't be clunked before the reference counter go down to 0, then we can avoid the clunked fid to be used. tests: race issue test from the old test case: for file in {01..50}; do touch f.${file}; done seq 1 1000 | xargs -n 1 -P 50 -I{} cat f.* > /dev/null open-unlink-f*syscall test: I have tested for f*syscall include: ftruncate fstat fchown fchmod faccessat. Link: http://lkml.kernel.org/r/20200923141146.90046-5-jianyong.wu@arm.com Fixes: 478ba09edc1f ("fs/9p: search open fids first") Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Diffstat (limited to 'fs/9p/vfs_inode_dotl.c')
-rw-r--r--fs/9p/vfs_inode_dotl.c34
1 files changed, 28 insertions, 6 deletions
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 08f2e089fb0e..823c2eb5f1bf 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -296,6 +296,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
/* instantiate inode and assign the unopened fid to the dentry */
fid = p9_client_walk(dfid, 1, &name, 1);
+ p9_client_clunk(dfid);
if (IS_ERR(fid)) {
err = PTR_ERR(fid);
p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);
@@ -408,7 +409,6 @@ static int v9fs_vfs_mkdir_dotl(struct inode *dir,
err = p9_client_mkdir_dotl(dfid, name, mode, gid, &qid);
if (err < 0)
goto error;
-
fid = p9_client_walk(dfid, 1, &name, 1);
if (IS_ERR(fid)) {
err = PTR_ERR(fid);
@@ -452,6 +452,7 @@ error:
if (fid)
p9_client_clunk(fid);
v9fs_put_acl(dacl, pacl);
+ p9_client_clunk(dfid);
return err;
}
@@ -479,6 +480,7 @@ v9fs_vfs_getattr_dotl(const struct path *path, struct kstat *stat,
*/
st = p9_client_getattr_dotl(fid, P9_STATS_ALL);
+ p9_client_clunk(fid);
if (IS_ERR(st))
return PTR_ERR(st);
@@ -540,7 +542,7 @@ static int v9fs_mapped_iattr_valid(int iattr_valid)
int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
{
- int retval;
+ int retval, use_dentry = 0;
struct p9_fid *fid = NULL;
struct p9_iattr_dotl p9attr;
struct inode *inode = d_inode(dentry);
@@ -565,8 +567,10 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
fid = iattr->ia_file->private_data;
WARN_ON(!fid);
}
- if (!fid)
+ if (!fid) {
fid = v9fs_fid_lookup(dentry);
+ use_dentry = 1;
+ }
if (IS_ERR(fid))
return PTR_ERR(fid);
@@ -575,8 +579,11 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
filemap_write_and_wait(inode->i_mapping);
retval = p9_client_setattr(fid, &p9attr);
- if (retval < 0)
+ if (retval < 0) {
+ if (use_dentry)
+ p9_client_clunk(fid);
return retval;
+ }
if ((iattr->ia_valid & ATTR_SIZE) &&
iattr->ia_size != i_size_read(inode))
@@ -588,9 +595,15 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
if (iattr->ia_valid & ATTR_MODE) {
/* We also want to update ACL when we update mode bits */
retval = v9fs_acl_chmod(inode, fid);
- if (retval < 0)
+ if (retval < 0) {
+ if (use_dentry)
+ p9_client_clunk(fid);
return retval;
+ }
}
+ if (use_dentry)
+ p9_client_clunk(fid);
+
return 0;
}
@@ -742,6 +755,7 @@ error:
if (fid)
p9_client_clunk(fid);
+ p9_client_clunk(dfid);
return err;
}
@@ -770,11 +784,15 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct inode *dir,
return PTR_ERR(dfid);
oldfid = v9fs_fid_lookup(old_dentry);
- if (IS_ERR(oldfid))
+ if (IS_ERR(oldfid)) {
+ p9_client_clunk(dfid);
return PTR_ERR(oldfid);
+ }
err = p9_client_link(dfid, oldfid, dentry->d_name.name);
+ p9_client_clunk(dfid);
+ p9_client_clunk(oldfid);
if (err < 0) {
p9_debug(P9_DEBUG_VFS, "p9_client_link failed %d\n", err);
return err;
@@ -789,6 +807,7 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct inode *dir,
return PTR_ERR(fid);
v9fs_refresh_inode_dotl(fid, d_inode(old_dentry));
+ p9_client_clunk(fid);
}
ihold(d_inode(old_dentry));
d_instantiate(dentry, d_inode(old_dentry));
@@ -887,6 +906,8 @@ error:
if (fid)
p9_client_clunk(fid);
v9fs_put_acl(dacl, pacl);
+ p9_client_clunk(dfid);
+
return err;
}
@@ -915,6 +936,7 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry,
if (IS_ERR(fid))
return ERR_CAST(fid);
retval = p9_client_readlink(fid, &target);
+ p9_client_clunk(fid);
if (retval)
return ERR_PTR(retval);
set_delayed_call(done, kfree_link, target);