Skip to content

Commit d05dcfd

Browse files
ericvhericvh
committed
fs/9p: mitigate inode collisions
Detect and mitigate inode collsions that now occur since we fixed 9p generating duplicate inode structures. Underlying cause of these appears to be a race condition between reuse of inode numbers in underlying file system and cleanup of inode numbers in the client. Enabling caching makes this much more likely to happen as it increases cleanup latency due to writebacks. Reported-by: Kent Overstreet <[email protected]> Signed-off-by: Eric Van Hensbergen <[email protected]>
1 parent ed30a4a commit d05dcfd

File tree

4 files changed

+56
-22
lines changed

4 files changed

+56
-22
lines changed

‎fs/9p/v9fs.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,14 @@ extern int v9fs_vfs_rename(struct mnt_idmap *idmap,
179179
struct inode *old_dir, struct dentry *old_dentry,
180180
struct inode *new_dir, struct dentry *new_dentry,
181181
unsigned int flags);
182-
extern struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid);
182+
extern struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid,
183+
bool new);
183184
extern const struct inode_operations v9fs_dir_inode_operations_dotl;
184185
extern const struct inode_operations v9fs_file_inode_operations_dotl;
185186
extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
186187
extern const struct netfs_request_ops v9fs_req_ops;
187188
extern struct inode *v9fs_fid_iget_dotl(struct super_block *sb,
188-
struct p9_fid *fid);
189+
struct p9_fid *fid, bool new);
189190

190191
/* other default globals */
191192
#define V9FS_PORT 564
@@ -224,12 +225,12 @@ static inline int v9fs_proto_dotl(struct v9fs_session_info *v9ses)
224225
*/
225226
static inline struct inode *
226227
v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
227-
struct super_block *sb)
228+
struct super_block *sb, bool new)
228229
{
229230
if (v9fs_proto_dotl(v9ses))
230-
return v9fs_fid_iget_dotl(sb, fid);
231+
return v9fs_fid_iget_dotl(sb, fid, new);
231232
else
232-
return v9fs_fid_iget(sb, fid);
233+
return v9fs_fid_iget(sb, fid, new);
233234
}
234235

235236
#endif

‎fs/9p/vfs_inode.c‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,8 @@ void v9fs_evict_inode(struct inode *inode)
364364
clear_inode(inode);
365365
}
366366

367-
struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid)
367+
struct inode *
368+
v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid, bool new)
368369
{
369370
dev_t rdev;
370371
int retval;
@@ -376,8 +377,18 @@ struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid)
376377
inode = iget_locked(sb, QID2INO(&fid->qid));
377378
if (unlikely(!inode))
378379
return ERR_PTR(-ENOMEM);
379-
if (!(inode->i_state & I_NEW))
380-
return inode;
380+
if (!(inode->i_state & I_NEW)) {
381+
if (!new) {
382+
goto done;
383+
} else {
384+
p9_debug(P9_DEBUG_VFS, "WARNING: Inode collision %ld\n",
385+
inode->i_ino);
386+
iput(inode);
387+
remove_inode_hash(inode);
388+
inode = iget_locked(sb, QID2INO(&fid->qid));
389+
WARN_ON(!(inode->i_state & I_NEW));
390+
}
391+
}
381392

382393
/*
383394
* initialize the inode with the stat info
@@ -401,11 +412,11 @@ struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid)
401412
v9fs_set_netfs_context(inode);
402413
v9fs_cache_inode_get_cookie(inode);
403414
unlock_new_inode(inode);
415+
done:
404416
return inode;
405417
error:
406418
iget_failed(inode);
407419
return ERR_PTR(retval);
408-
409420
}
410421

411422
/**
@@ -437,8 +448,15 @@ static int v9fs_at_to_dotl_flags(int flags)
437448
*/
438449
static void v9fs_dec_count(struct inode *inode)
439450
{
440-
if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2)
441-
drop_nlink(inode);
451+
if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2) {
452+
if (inode->i_nlink) {
453+
drop_nlink(inode);
454+
} else {
455+
p9_debug(P9_DEBUG_VFS,
456+
"WARNING: unexpected i_nlink zero %d inode %ld\n",
457+
inode->i_nlink, inode->i_ino);
458+
}
459+
}
442460
}
443461

444462
/**
@@ -489,6 +507,9 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags)
489507
} else
490508
v9fs_dec_count(inode);
491509

510+
if (inode->i_nlink <= 0) /* no more refs unhash it */
511+
remove_inode_hash(inode);
512+
492513
v9fs_invalidate_inode_attr(inode);
493514
v9fs_invalidate_inode_attr(dir);
494515

@@ -554,7 +575,7 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir,
554575
/*
555576
* instantiate inode and assign the unopened fid to the dentry
556577
*/
557-
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
578+
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, true);
558579
if (IS_ERR(inode)) {
559580
err = PTR_ERR(inode);
560581
p9_debug(P9_DEBUG_VFS,
@@ -683,7 +704,7 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
683704
else if (IS_ERR(fid))
684705
inode = ERR_CAST(fid);
685706
else
686-
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
707+
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, false);
687708
/*
688709
* If we had a rename on the server and a parallel lookup
689710
* for the new name, then make sure we instantiate with

‎fs/9p/vfs_inode_dotl.c‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ static kgid_t v9fs_get_fsgid_for_create(struct inode *dir_inode)
5252
return current_fsgid();
5353
}
5454

55-
struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid)
55+
56+
57+
struct inode *
58+
v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid, bool new)
5659
{
5760
int retval;
5861
struct inode *inode;
@@ -62,8 +65,18 @@ struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid)
6265
inode = iget_locked(sb, QID2INO(&fid->qid));
6366
if (unlikely(!inode))
6467
return ERR_PTR(-ENOMEM);
65-
if (!(inode->i_state & I_NEW))
66-
return inode;
68+
if (!(inode->i_state & I_NEW)) {
69+
if (!new) {
70+
goto done;
71+
} else { /* deal with race condition in inode number reuse */
72+
p9_debug(P9_DEBUG_ERROR, "WARNING: Inode collision %lx\n",
73+
inode->i_ino);
74+
iput(inode);
75+
remove_inode_hash(inode);
76+
inode = iget_locked(sb, QID2INO(&fid->qid));
77+
WARN_ON(!(inode->i_state & I_NEW));
78+
}
79+
}
6780

6881
/*
6982
* initialize the inode with the stat info
@@ -90,12 +103,11 @@ struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid)
90103
goto error;
91104

92105
unlock_new_inode(inode);
93-
106+
done:
94107
return inode;
95108
error:
96109
iget_failed(inode);
97110
return ERR_PTR(retval);
98-
99111
}
100112

101113
struct dotl_openflag_map {
@@ -247,7 +259,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
247259
p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);
248260
goto out;
249261
}
250-
inode = v9fs_fid_iget_dotl(dir->i_sb, fid);
262+
inode = v9fs_fid_iget_dotl(dir->i_sb, fid, true);
251263
if (IS_ERR(inode)) {
252264
err = PTR_ERR(inode);
253265
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", err);
@@ -340,7 +352,7 @@ static int v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap,
340352
}
341353

342354
/* instantiate inode and assign the unopened fid to the dentry */
343-
inode = v9fs_fid_iget_dotl(dir->i_sb, fid);
355+
inode = v9fs_fid_iget_dotl(dir->i_sb, fid, true);
344356
if (IS_ERR(inode)) {
345357
err = PTR_ERR(inode);
346358
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
@@ -776,7 +788,7 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct inode *dir,
776788
err);
777789
goto error;
778790
}
779-
inode = v9fs_fid_iget_dotl(dir->i_sb, fid);
791+
inode = v9fs_fid_iget_dotl(dir->i_sb, fid, true);
780792
if (IS_ERR(inode)) {
781793
err = PTR_ERR(inode);
782794
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",

‎fs/9p/vfs_super.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
139139
else
140140
sb->s_d_op = &v9fs_dentry_operations;
141141

142-
inode = v9fs_get_inode_from_fid(v9ses, fid, sb);
142+
inode = v9fs_get_inode_from_fid(v9ses, fid, sb, true);
143143
if (IS_ERR(inode)) {
144144
retval = PTR_ERR(inode);
145145
goto release_sb;

0 commit comments

Comments
 (0)