Skip to content

Commit 5e3cc1e

Browse files
Hou TaoDominique Martinet
Hou Tao
authored and
Dominique Martinet
committed
9p: use inode->i_lock to protect i_size_write() under 32-bit
Use inode->i_lock to protect i_size_write(), else i_size_read() in generic_fillattr() may loop infinitely in read_seqcount_begin() when multiple processes invoke v9fs_vfs_getattr() or v9fs_vfs_getattr_dotl() simultaneously under 32-bit SMP environment, and a soft lockup will be triggered as show below: watchdog: BUG: soft lockup - CPU#5 stuck for 22s! [stat:2217] Modules linked in: CPU: 5 PID: 2217 Comm: stat Not tainted 5.0.0-rc1-00005-g7f702faf5a9e #4 Hardware name: Generic DT based system PC is at generic_fillattr+0x104/0x108 LR is at 0xec497f00 pc : [<802b8898>] lr : [<ec497f00>] psr: 200c0013 sp : ec497e2 ip : ed608030 fp : ec497e3c r10: 00000000 r9 : ec497f00 r8 : ed608030 r7 : ec497ebc r6 : ec497f00 r5 : ee5c1550 r4 : ee005780 r3 : 0000052d r2 : 00000000 r1 : ec497f00 r0 : ed608030 Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: ac48006a DAC: 00000051 CPU: 5 PID: 2217 Comm: stat Not tainted 5.0.0-rc1-00005-g7f702faf5a9e #4 Hardware name: Generic DT based system Backtrace: [<8010d974>] (dump_backtrace) from [<8010dc88>] (show_stack+0x20/0x24) [<8010dc68>] (show_stack) from [<80a1d194>] (dump_stack+0xb0/0xdc) [<80a1d0e4>] (dump_stack) from [<80109f34>] (show_regs+0x1c/0x20) [<80109f18>] (show_regs) from [<801d0a80>] (watchdog_timer_fn+0x280/0x2f8) [<801d0800>] (watchdog_timer_fn) from [<80198658>] (__hrtimer_run_queues+0x18c/0x380) [<801984cc>] (__hrtimer_run_queues) from [<80198e60>] (hrtimer_run_queues+0xb8/0xf0) [<80198da8>] (hrtimer_run_queues) from [<801973e8>] (run_local_timers+0x28/0x64) [<801973c0>] (run_local_timers) from [<80197460>] (update_process_times+0x3c/0x6c) [<80197424>] (update_process_times) from [<801ab2b8>] (tick_nohz_handler+0xe0/0x1bc) [<801ab1d8>] (tick_nohz_handler) from [<80843050>] (arch_timer_handler_virt+0x38/0x48) [<80843018>] (arch_timer_handler_virt) from [<80180a64>] (handle_percpu_devid_irq+0x8c/0x240) [<801809d8>] (handle_percpu_devid_irq) from [<8017ac20>] (generic_handle_irq+0x34/0x44) [<8017abec>] (generic_handle_irq) from [<8017b344>] (__handle_domain_irq+0x6c/0xc4) [<8017b2d8>] (__handle_domain_irq) from [<801022e0>] (gic_handle_irq+0x4c/0x88) [<80102294>] (gic_handle_irq) from [<80101a30>] (__irq_svc+0x70/0x98) [<802b8794>] (generic_fillattr) from [<8056b284>] (v9fs_vfs_getattr_dotl+0x74/0xa4) [<8056b210>] (v9fs_vfs_getattr_dotl) from [<802b8904>] (vfs_getattr_nosec+0x68/0x7c) [<802b889c>] (vfs_getattr_nosec) from [<802b895c>] (vfs_getattr+0x44/0x48) [<802b8918>] (vfs_getattr) from [<802b8a74>] (vfs_statx+0x9c/0xec) [<802b89d8>] (vfs_statx) from [<802b9428>] (sys_lstat64+0x48/0x78) [<802b93e0>] (sys_lstat64) from [<80101000>] (ret_fast_syscall+0x0/0x28) [dominique.martinet@cea.fr: updated comment to not refer to a function in another subsystem] Link: http://lkml.kernel.org/r/20190124063514.8571-2-houtao1@huawei.com Cc: stable@vger.kernel.org Fixes: 7549ae3 ("9p: Use the i_size_[read, write]() macros instead of using inode->i_size directly.") Reported-by: Xing Gaopeng <xingaopeng@huawei.com> Signed-off-by: Hou Tao <houtao1@huawei.com> Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
1 parent 3bbe8b1 commit 5e3cc1e

File tree

5 files changed

+53
-30
lines changed

5 files changed

+53
-30
lines changed

‎fs/9p/v9fs_vfs.h

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@
4040
*/
4141
#define P9_LOCK_TIMEOUT (30*HZ)
4242

43+
/* flags for v9fs_stat2inode() & v9fs_stat2inode_dotl() */
44+
#define V9FS_STAT2INODE_KEEP_ISIZE 1
45+
4346
extern struct file_system_type v9fs_fs_type;
4447
extern const struct address_space_operations v9fs_addr_operations;
4548
extern const struct file_operations v9fs_file_operations;
@@ -61,8 +64,10 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
6164
struct inode *inode, umode_t mode, dev_t);
6265
void v9fs_evict_inode(struct inode *inode);
6366
ino_t v9fs_qid2ino(struct p9_qid *qid);
64-
void v9fs_stat2inode(struct p9_wstat *, struct inode *, struct super_block *);
65-
void v9fs_stat2inode_dotl(struct p9_stat_dotl *, struct inode *);
67+
void v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
68+
struct super_block *sb, unsigned int flags);
69+
void v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
70+
unsigned int flags);
6671
int v9fs_dir_release(struct inode *inode, struct file *filp);
6772
int v9fs_file_open(struct inode *inode, struct file *file);
6873
void v9fs_inode2stat(struct inode *inode, struct p9_wstat *stat);
@@ -83,4 +88,18 @@ static inline void v9fs_invalidate_inode_attr(struct inode *inode)
8388
}
8489

8590
int v9fs_open_to_dotl_flags(int flags);
91+
92+
static inline void v9fs_i_size_write(struct inode *inode, loff_t i_size)
93+
{
94+
/*
95+
* 32-bit need the lock, concurrent updates could break the
96+
* sequences and make i_size_read() loop forever.
97+
* 64-bit updates are atomic and can skip the locking.
98+
*/
99+
if (sizeof(i_size) > sizeof(long))
100+
spin_lock(&inode->i_lock);
101+
i_size_write(inode, i_size);
102+
if (sizeof(i_size) > sizeof(long))
103+
spin_unlock(&inode->i_lock);
104+
}
86105
#endif

‎fs/9p/vfs_file.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,11 @@ v9fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
446446
i_size = i_size_read(inode);
447447
if (iocb->ki_pos > i_size) {
448448
inode_add_bytes(inode, iocb->ki_pos - i_size);
449-
i_size_write(inode, iocb->ki_pos);
449+
/*
450+
* Need to serialize against i_size_write() in
451+
* v9fs_stat2inode()
452+
*/
453+
v9fs_i_size_write(inode, iocb->ki_pos);
450454
}
451455
return retval;
452456
}

‎fs/9p/vfs_inode.c

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
538538
if (retval)
539539
goto error;
540540

541-
v9fs_stat2inode(st, inode, sb);
541+
v9fs_stat2inode(st, inode, sb, 0);
542542
v9fs_cache_inode_get_cookie(inode);
543543
unlock_new_inode(inode);
544544
return inode;
@@ -1092,7 +1092,7 @@ v9fs_vfs_getattr(const struct path *path, struct kstat *stat,
10921092
if (IS_ERR(st))
10931093
return PTR_ERR(st);
10941094

1095-
v9fs_stat2inode(st, d_inode(dentry), dentry->d_sb);
1095+
v9fs_stat2inode(st, d_inode(dentry), dentry->d_sb, 0);
10961096
generic_fillattr(d_inode(dentry), stat);
10971097

10981098
p9stat_free(st);
@@ -1170,12 +1170,13 @@ static int v9fs_vfs_setattr(struct dentry *dentry, struct iattr *iattr)
11701170
* @stat: Plan 9 metadata (mistat) structure
11711171
* @inode: inode to populate
11721172
* @sb: superblock of filesystem
1173+
* @flags: control flags (e.g. V9FS_STAT2INODE_KEEP_ISIZE)
11731174
*
11741175
*/
11751176

11761177
void
11771178
v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
1178-
struct super_block *sb)
1179+
struct super_block *sb, unsigned int flags)
11791180
{
11801181
umode_t mode;
11811182
char ext[32];
@@ -1216,10 +1217,11 @@ v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
12161217
mode = p9mode2perm(v9ses, stat);
12171218
mode |= inode->i_mode & ~S_IALLUGO;
12181219
inode->i_mode = mode;
1219-
i_size_write(inode, stat->length);
12201220

1221+
if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE))
1222+
v9fs_i_size_write(inode, stat->length);
12211223
/* not real number of blocks, but 512 byte ones ... */
1222-
inode->i_blocks = (i_size_read(inode) + 512 - 1) >> 9;
1224+
inode->i_blocks = (stat->length + 512 - 1) >> 9;
12231225
v9inode->cache_validity &= ~V9FS_INO_INVALID_ATTR;
12241226
}
12251227

@@ -1416,9 +1418,9 @@ int v9fs_refresh_inode(struct p9_fid *fid, struct inode *inode)
14161418
{
14171419
int umode;
14181420
dev_t rdev;
1419-
loff_t i_size;
14201421
struct p9_wstat *st;
14211422
struct v9fs_session_info *v9ses;
1423+
unsigned int flags;
14221424

14231425
v9ses = v9fs_inode2v9ses(inode);
14241426
st = p9_client_stat(fid);
@@ -1431,16 +1433,13 @@ int v9fs_refresh_inode(struct p9_fid *fid, struct inode *inode)
14311433
if ((inode->i_mode & S_IFMT) != (umode & S_IFMT))
14321434
goto out;
14331435

1434-
spin_lock(&inode->i_lock);
14351436
/*
14361437
* We don't want to refresh inode->i_size,
14371438
* because we may have cached data
14381439
*/
1439-
i_size = inode->i_size;
1440-
v9fs_stat2inode(st, inode, inode->i_sb);
1441-
if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)
1442-
inode->i_size = i_size;
1443-
spin_unlock(&inode->i_lock);
1440+
flags = (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE) ?
1441+
V9FS_STAT2INODE_KEEP_ISIZE : 0;
1442+
v9fs_stat2inode(st, inode, inode->i_sb, flags);
14441443
out:
14451444
p9stat_free(st);
14461445
kfree(st);

‎fs/9p/vfs_inode_dotl.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
143143
if (retval)
144144
goto error;
145145

146-
v9fs_stat2inode_dotl(st, inode);
146+
v9fs_stat2inode_dotl(st, inode, 0);
147147
v9fs_cache_inode_get_cookie(inode);
148148
retval = v9fs_get_acl(inode, fid);
149149
if (retval)
@@ -496,7 +496,7 @@ v9fs_vfs_getattr_dotl(const struct path *path, struct kstat *stat,
496496
if (IS_ERR(st))
497497
return PTR_ERR(st);
498498

499-
v9fs_stat2inode_dotl(st, d_inode(dentry));
499+
v9fs_stat2inode_dotl(st, d_inode(dentry), 0);
500500
generic_fillattr(d_inode(dentry), stat);
501501
/* Change block size to what the server returned */
502502
stat->blksize = st->st_blksize;
@@ -607,11 +607,13 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
607607
* v9fs_stat2inode_dotl - populate an inode structure with stat info
608608
* @stat: stat structure
609609
* @inode: inode to populate
610+
* @flags: ctrl flags (e.g. V9FS_STAT2INODE_KEEP_ISIZE)
610611
*
611612
*/
612613

613614
void
614-
v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode)
615+
v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
616+
unsigned int flags)
615617
{
616618
umode_t mode;
617619
struct v9fs_inode *v9inode = V9FS_I(inode);
@@ -631,7 +633,8 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode)
631633
mode |= inode->i_mode & ~S_IALLUGO;
632634
inode->i_mode = mode;
633635

634-
i_size_write(inode, stat->st_size);
636+
if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE))
637+
v9fs_i_size_write(inode, stat->st_size);
635638
inode->i_blocks = stat->st_blocks;
636639
} else {
637640
if (stat->st_result_mask & P9_STATS_ATIME) {
@@ -661,8 +664,9 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode)
661664
}
662665
if (stat->st_result_mask & P9_STATS_RDEV)
663666
inode->i_rdev = new_decode_dev(stat->st_rdev);
664-
if (stat->st_result_mask & P9_STATS_SIZE)
665-
i_size_write(inode, stat->st_size);
667+
if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE) &&
668+
stat->st_result_mask & P9_STATS_SIZE)
669+
v9fs_i_size_write(inode, stat->st_size);
666670
if (stat->st_result_mask & P9_STATS_BLOCKS)
667671
inode->i_blocks = stat->st_blocks;
668672
}
@@ -928,9 +932,9 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry,
928932

929933
int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
930934
{
931-
loff_t i_size;
932935
struct p9_stat_dotl *st;
933936
struct v9fs_session_info *v9ses;
937+
unsigned int flags;
934938

935939
v9ses = v9fs_inode2v9ses(inode);
936940
st = p9_client_getattr_dotl(fid, P9_STATS_ALL);
@@ -942,16 +946,13 @@ int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
942946
if ((inode->i_mode & S_IFMT) != (st->st_mode & S_IFMT))
943947
goto out;
944948

945-
spin_lock(&inode->i_lock);
946949
/*
947950
* We don't want to refresh inode->i_size,
948951
* because we may have cached data
949952
*/
950-
i_size = inode->i_size;
951-
v9fs_stat2inode_dotl(st, inode);
952-
if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)
953-
inode->i_size = i_size;
954-
spin_unlock(&inode->i_lock);
953+
flags = (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE) ?
954+
V9FS_STAT2INODE_KEEP_ISIZE : 0;
955+
v9fs_stat2inode_dotl(st, inode, flags);
955956
out:
956957
kfree(st);
957958
return 0;

‎fs/9p/vfs_super.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
172172
goto release_sb;
173173
}
174174
d_inode(root)->i_ino = v9fs_qid2ino(&st->qid);
175-
v9fs_stat2inode_dotl(st, d_inode(root));
175+
v9fs_stat2inode_dotl(st, d_inode(root), 0);
176176
kfree(st);
177177
} else {
178178
struct p9_wstat *st = NULL;
@@ -183,7 +183,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
183183
}
184184

185185
d_inode(root)->i_ino = v9fs_qid2ino(&st->qid);
186-
v9fs_stat2inode(st, d_inode(root), sb);
186+
v9fs_stat2inode(st, d_inode(root), sb, 0);
187187

188188
p9stat_free(st);
189189
kfree(st);

0 commit comments

Comments
 (0)