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 <[email protected]>
---
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
Thanks, this looks good to me!
Reviewed-by: Harshad Shirwadkar <[email protected]>
On Thu, Apr 29, 2021 at 3:43 AM Ritesh Harjani <[email protected]> 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 <[email protected]>
> ---
> 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