Currently, data-race like [1] occur in fs/ext4/ialloc.c
find_group_other() and find_group_orlov() read multiple ext4_groups but
do not protect them with locks, which causes data-race. I think it would
be appropriate to add ext4_lock_group() at an appropriate location to solve
this.
[1]
==================================================================
BUG: KCSAN: data-race in ext4_free_inodes_count / ext4_free_inodes_set
write to 0xffff88810404300e of 2 bytes by task 6254 on cpu 1:
ext4_free_inodes_set+0x1f/0x80 fs/ext4/super.c:405
__ext4_new_inode+0x15ca/0x2200 fs/ext4/ialloc.c:1216
ext4_symlink+0x242/0x5a0 fs/ext4/namei.c:3391
vfs_symlink+0xca/0x1d0 fs/namei.c:4615
do_symlinkat+0xe3/0x340 fs/namei.c:4641
__do_sys_symlinkat fs/namei.c:4657 [inline]
__se_sys_symlinkat fs/namei.c:4654 [inline]
__x64_sys_symlinkat+0x5e/0x70 fs/namei.c:4654
x64_sys_call+0x1dda/0x2d60 arch/x86/include/generated/asm/syscalls_64.h:267
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0x54/0x120 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x76/0x7e
read to 0xffff88810404300e of 2 bytes by task 6257 on cpu 0:
ext4_free_inodes_count+0x1c/0x80 fs/ext4/super.c:349
find_group_other fs/ext4/ialloc.c:594 [inline]
__ext4_new_inode+0x6ec/0x2200 fs/ext4/ialloc.c:1017
ext4_symlink+0x242/0x5a0 fs/ext4/namei.c:3391
vfs_symlink+0xca/0x1d0 fs/namei.c:4615
do_symlinkat+0xe3/0x340 fs/namei.c:4641
__do_sys_symlinkat fs/namei.c:4657 [inline]
__se_sys_symlinkat fs/namei.c:4654 [inline]
__x64_sys_symlinkat+0x5e/0x70 fs/namei.c:4654
x64_sys_call+0x1dda/0x2d60 arch/x86/include/generated/asm/syscalls_64.h:267
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0x54/0x120 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x76/0x7e
value changed: 0x185c -> 0x185b
Fixes: ac27a0ec112a ("[PATCH] ext4: initial copy of files from ext3")
Signed-off-by: Jeongjun Park <aha310510@xxxxxxxxx>
---
fs/ext4/ialloc.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 9dfd768ed9f8..5cae247ff21f 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -500,11 +500,14 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent,
for (i = 0; i < flex_size; i++) {
if (grp+i >= real_ngroups)
break;
+ ext4_lock_group(sb, grp+i);
desc = ext4_get_group_desc(sb, grp+i, NULL);
if (desc && ext4_free_inodes_count(sb, desc)) {
*group = grp+i;
+ ext4_unlock_group(sb, grp+i);
return 0;
}
+ ext4_unlock_group(sb, grp+i);
}
goto fallback;
}
@@ -544,14 +547,17 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent,
parent_group = EXT4_I(parent)->i_block_group;
for (i = 0; i < ngroups; i++) {
grp = (parent_group + i) % ngroups;
+ ext4_lock_group(sb, grp);
desc = ext4_get_group_desc(sb, grp, NULL);
if (desc) {
grp_free = ext4_free_inodes_count(sb, desc);
if (grp_free && grp_free >= avefreei) {
*group = grp;
+ ext4_unlock_group(sb, grp);
return 0;
}
}
+ ext4_unlock_group(sb, grp);
}
if (avefreei) {
@@ -590,11 +596,14 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
if (last > ngroups)
last = ngroups;
for (i = parent_group; i < last; i++) {
+ ext4_lock_group(sb, i);
desc = ext4_get_group_desc(sb, i, NULL);
if (desc && ext4_free_inodes_count(sb, desc)) {
*group = i;
+ ext4_unlock_group(sb, i);
return 0;
}
+ ext4_unlock_group(sb, i);
}
if (!retry && EXT4_I(parent)->i_last_alloc_group != ~0) {
retry = 1;
@@ -616,10 +625,14 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
* Try to place the inode in its parent directory
*/
*group = parent_group;
+ ext4_lock_group(sb, *group);
desc = ext4_get_group_desc(sb, *group, NULL);
if (desc && ext4_free_inodes_count(sb, desc) &&
- ext4_free_group_clusters(sb, desc))
+ ext4_free_group_clusters(sb, desc)) {
+ ext4_unlock_group(sb, *group);
return 0;
+ }
+ ext4_unlock_group(sb, *group);
/*
* We're going to place this inode in a different blockgroup from its
@@ -640,10 +653,14 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
*group += i;
if (*group >= ngroups)
*group -= ngroups;
+ ext4_lock_group(sb, *group);
desc = ext4_get_group_desc(sb, *group, NULL);
if (desc && ext4_free_inodes_count(sb, desc) &&
- ext4_free_group_clusters(sb, desc))
+ ext4_free_group_clusters(sb, desc)) {
+ ext4_unlock_group(sb, *group);
return 0;
+ }
+ ext4_unlock_group(sb, *group);
}
/*
@@ -654,9 +671,13 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
for (i = 0; i < ngroups; i++) {
if (++*group >= ngroups)
*group = 0;
+ ext4_lock_group(sb, *group);
desc = ext4_get_group_desc(sb, *group, NULL);
- if (desc && ext4_free_inodes_count(sb, desc))
+ if (desc && ext4_free_inodes_count(sb, desc)) {
+ ext4_unlock_group(sb, *group);
return 0;
+ }
+ ext4_unlock_group(sb, *group);
}
return -1;
--
