| Message ID | 6cceb9a75c54bef8fa9696c1b08c8df5ff6169e2.1619692410.git.riteshh@linux.ibm.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | ext4: Fix accessing uninit percpu counter variable with fast_commit | expand |
Thanks, this looks good to me! Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> On Thu, Apr 29, 2021 at 3:43 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote: > > When running generic/527 with fast_commit configuration, below issue is > seen on Power. > With fast_commit, during ext4_fc_replay() (which can be called from > ext4_fill_super()), if inode eviction happens then it can access an > uninitialized percpu counter variable. > > This patch adds the check b4 accessing the counters in ext4_free_inode() path. > > [ 321.165371] run fstests generic/527 at 2021-04-29 08:38:43 > [ 323.027786] EXT4-fs (dm-0): mounted filesystem with ordered data mode. Opts: block_validity. Quota mode: none. > [ 323.618772] BUG: Unable to handle kernel data access on read at 0x1fbd80000 > [ 323.619767] Faulting instruction address: 0xc000000000bae78c > cpu 0x1: Vector: 300 (Data Access) at [c000000010706ef0] > pc: c000000000bae78c: percpu_counter_add_batch+0x3c/0x100 > lr: c0000000006d0bb0: ext4_free_inode+0x780/0xb90 > pid = 5593, comm = mount > ext4_free_inode+0x780/0xb90 > ext4_evict_inode+0xa8c/0xc60 > evict+0xfc/0x1e0 > ext4_fc_replay+0xc50/0x20f0 > do_one_pass+0xfe0/0x1350 > jbd2_journal_recover+0x184/0x2e0 > jbd2_journal_load+0x1c0/0x4a0 > ext4_fill_super+0x2458/0x4200 > mount_bdev+0x1dc/0x290 > ext4_mount+0x28/0x40 > legacy_get_tree+0x4c/0xa0 > vfs_get_tree+0x4c/0x120 > path_mount+0xcf8/0xd70 > do_mount+0x80/0xd0 > sys_mount+0x3fc/0x490 > system_call_exception+0x384/0x3d0 > system_call_common+0xec/0x278 > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> > --- > fs/ext4/ialloc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c > index 755a68bb7e22..e4a92642e487 100644 > --- a/fs/ext4/ialloc.c > +++ b/fs/ext4/ialloc.c > @@ -322,14 +322,16 @@ void ext4_free_inode(handle_t *handle, struct inode *inode) > if (is_directory) { > count = ext4_used_dirs_count(sb, gdp) - 1; > ext4_used_dirs_set(sb, gdp, count); > - percpu_counter_dec(&sbi->s_dirs_counter); > + if (percpu_counter_initialized(&sbi->s_dirs_counter)) > + percpu_counter_dec(&sbi->s_dirs_counter); > } > ext4_inode_bitmap_csum_set(sb, block_group, gdp, bitmap_bh, > EXT4_INODES_PER_GROUP(sb) / 8); > ext4_group_desc_csum_set(sb, block_group, gdp); > ext4_unlock_group(sb, block_group); > > - percpu_counter_inc(&sbi->s_freeinodes_counter); > + if (percpu_counter_initialized(&sbi->s_freeinodes_counter)) > + percpu_counter_inc(&sbi->s_freeinodes_counter); > if (sbi->s_log_groups_per_flex) { > struct flex_groups *fg; > > -- > 2.30.2 >
On Thu, Apr 29, 2021 at 04:13:44PM +0530, Ritesh Harjani wrote: > When running generic/527 with fast_commit configuration, below issue is > seen on Power. > With fast_commit, during ext4_fc_replay() (which can be called from > ext4_fill_super()), if inode eviction happens then it can access an > uninitialized percpu counter variable. > > This patch adds the check b4 accessing the counters in ext4_free_inode() path. Applied, thanks. - Ted
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c index 755a68bb7e22..e4a92642e487 100644 --- a/fs/ext4/ialloc.c +++ b/fs/ext4/ialloc.c @@ -322,14 +322,16 @@ void ext4_free_inode(handle_t *handle, struct inode *inode) if (is_directory) { count = ext4_used_dirs_count(sb, gdp) - 1; ext4_used_dirs_set(sb, gdp, count); - percpu_counter_dec(&sbi->s_dirs_counter); + if (percpu_counter_initialized(&sbi->s_dirs_counter)) + percpu_counter_dec(&sbi->s_dirs_counter); } ext4_inode_bitmap_csum_set(sb, block_group, gdp, bitmap_bh, EXT4_INODES_PER_GROUP(sb) / 8); ext4_group_desc_csum_set(sb, block_group, gdp); ext4_unlock_group(sb, block_group); - percpu_counter_inc(&sbi->s_freeinodes_counter); + if (percpu_counter_initialized(&sbi->s_freeinodes_counter)) + percpu_counter_inc(&sbi->s_freeinodes_counter); if (sbi->s_log_groups_per_flex) { struct flex_groups *fg;
When running generic/527 with fast_commit configuration, below issue is seen on Power. With fast_commit, during ext4_fc_replay() (which can be called from ext4_fill_super()), if inode eviction happens then it can access an uninitialized percpu counter variable. This patch adds the check b4 accessing the counters in ext4_free_inode() path. [ 321.165371] run fstests generic/527 at 2021-04-29 08:38:43 [ 323.027786] EXT4-fs (dm-0): mounted filesystem with ordered data mode. Opts: block_validity. Quota mode: none. [ 323.618772] BUG: Unable to handle kernel data access on read at 0x1fbd80000 [ 323.619767] Faulting instruction address: 0xc000000000bae78c cpu 0x1: Vector: 300 (Data Access) at [c000000010706ef0] pc: c000000000bae78c: percpu_counter_add_batch+0x3c/0x100 lr: c0000000006d0bb0: ext4_free_inode+0x780/0xb90 pid = 5593, comm = mount ext4_free_inode+0x780/0xb90 ext4_evict_inode+0xa8c/0xc60 evict+0xfc/0x1e0 ext4_fc_replay+0xc50/0x20f0 do_one_pass+0xfe0/0x1350 jbd2_journal_recover+0x184/0x2e0 jbd2_journal_load+0x1c0/0x4a0 ext4_fill_super+0x2458/0x4200 mount_bdev+0x1dc/0x290 ext4_mount+0x28/0x40 legacy_get_tree+0x4c/0xa0 vfs_get_tree+0x4c/0x120 path_mount+0xcf8/0xd70 do_mount+0x80/0xd0 sys_mount+0x3fc/0x490 system_call_exception+0x384/0x3d0 system_call_common+0xec/0x278 Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> --- fs/ext4/ialloc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- 2.30.2