2020-02-19 03:10:44

by Suraj Jitindar Singh

Subject: [PATCH 2/3] ext4: fix potential race between s_group_info online resizing and access

During an online resize an array of pointers to s_group_info gets replaced
so it can get enlarged. If there is a concurrent access to the array in
ext4_get_group_info() and this memory has been reused then this can lead to
an invalid memory access.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206443
Signed-off-by: Suraj Jitindar Singh <[email protected]>
Cc: [email protected]
---
fs/ext4/ext4.h | 6 +++---
fs/ext4/mballoc.c | 10 ++++++----
2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 236fc6500340..3f4aaaae7da6 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2994,13 +2994,13 @@ static inline
struct ext4_group_info *ext4_get_group_info(struct super_block *sb,
ext4_group_t group)
{
- struct ext4_group_info ***grp_info;
+ struct ext4_group_info **grp_info;
long indexv, indexh;
BUG_ON(group >= EXT4_SB(sb)->s_groups_count);
- grp_info = EXT4_SB(sb)->s_group_info;
indexv = group >> (EXT4_DESC_PER_BLOCK_BITS(sb));
indexh = group & ((EXT4_DESC_PER_BLOCK(sb)) - 1);
- return grp_info[indexv][indexh];
+ grp_info = sbi_array_rcu_deref(EXT4_SB(sb), s_group_info, indexv);
+ return grp_info[indexh];
}

/*
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index f64838187559..0d9b17afc85f 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2356,7 +2356,7 @@ int ext4_mb_alloc_groupinfo(struct super_block *sb, ext4_group_t ngroups)
{
struct ext4_sb_info *sbi = EXT4_SB(sb);
unsigned size;
- struct ext4_group_info ***new_groupinfo;
+ struct ext4_group_info ***old_groupinfo, ***new_groupinfo;

size = (ngroups + EXT4_DESC_PER_BLOCK(sb) - 1) >>
EXT4_DESC_PER_BLOCK_BITS(sb);
@@ -2369,13 +2369,15 @@ int ext4_mb_alloc_groupinfo(struct super_block *sb, ext4_group_t ngroups)
ext4_msg(sb, KERN_ERR, "can't allocate buddy meta group");
return -ENOMEM;
}
- if (sbi->s_group_info) {
+ old_groupinfo = sbi->s_group_info;
+ if (sbi->s_group_info)
memcpy(new_groupinfo, sbi->s_group_info,
sbi->s_group_info_size * sizeof(*sbi->s_group_info));
- kvfree(sbi->s_group_info);
- }
sbi->s_group_info = new_groupinfo;
+ rcu_assign_pointer(sbi->s_group_info, new_groupinfo);
sbi->s_group_info_size = size / sizeof(*sbi->s_group_info);
+ if (old_groupinfo)
+ ext4_kvfree_array_rcu(old_groupinfo);
ext4_debug("allocated s_groupinfo array for %d meta_bg's\n",
sbi->s_group_info_size);
return 0;
--
2.17.1

2020-02-19 20:21:06

by Singh, Balbir

Subject: Re: [PATCH 2/3] ext4: fix potential race between s_group_info online resizing and access

On Tue, 2020-02-18 at 19:08 -0800, Suraj Jitindar Singh wrote:
> During an online resize an array of pointers to s_group_info gets replaced
> so it can get enlarged. If there is a concurrent access to the array in
> ext4_get_group_info() and this memory has been reused then this can lead to
> an invalid memory access.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206443
> Signed-off-by: Suraj Jitindar Singh <[email protected]>
> Cc: [email protected]
> ---
> fs/ext4/ext4.h | 6 +++---
> fs/ext4/mballoc.c | 10 ++++++----
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 236fc6500340..3f4aaaae7da6 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2994,13 +2994,13 @@ static inline
> struct ext4_group_info *ext4_get_group_info(struct super_block *sb,
> ext4_group_t group)
> {
> - struct ext4_group_info ***grp_info;
> + struct ext4_group_info **grp_info;
> long indexv, indexh;
> BUG_ON(group >= EXT4_SB(sb)->s_groups_count);
> - grp_info = EXT4_SB(sb)->s_group_info;
> indexv = group >> (EXT4_DESC_PER_BLOCK_BITS(sb));
> indexh = group & ((EXT4_DESC_PER_BLOCK(sb)) - 1);
> - return grp_info[indexv][indexh];
> + grp_info = sbi_array_rcu_deref(EXT4_SB(sb), s_group_info, indexv);
> + return grp_info[indexh];
> }
>
> /*
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index f64838187559..0d9b17afc85f 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2356,7 +2356,7 @@ int ext4_mb_alloc_groupinfo(struct super_block *sb,
> ext4_group_t ngroups)
> {
> struct ext4_sb_info *sbi = EXT4_SB(sb);
> unsigned size;
> - struct ext4_group_info ***new_groupinfo;
> + struct ext4_group_info ***old_groupinfo, ***new_groupinfo;
>
> size = (ngroups + EXT4_DESC_PER_BLOCK(sb) - 1) >>
> EXT4_DESC_PER_BLOCK_BITS(sb);
> @@ -2369,13 +2369,15 @@ int ext4_mb_alloc_groupinfo(struct super_block *sb,
> ext4_group_t ngroups)
> ext4_msg(sb, KERN_ERR, "can't allocate buddy meta group");
> return -ENOMEM;
> }
> - if (sbi->s_group_info) {
> + old_groupinfo = sbi->s_group_info;
> + if (sbi->s_group_info)
> memcpy(new_groupinfo, sbi->s_group_info,
> sbi->s_group_info_size * sizeof(*sbi->s_group_info));
> - kvfree(sbi->s_group_info);
> - }
> sbi->s_group_info = new_groupinfo;
> + rcu_assign_pointer(sbi->s_group_info, new_groupinfo);
> sbi->s_group_info_size = size / sizeof(*sbi->s_group_info);
> + if (old_groupinfo)
> + ext4_kvfree_array_rcu(old_groupinfo);
> ext4_debug("allocated s_groupinfo array for %d meta_bg's\n",
> sbi->s_group_info_size);
> return 0;

Reviewed-by: Balbir Singh <[email protected]>

2020-02-20 05:14:07

by Theodore Ts'o

Subject: Re: [PATCH 2/3] ext4: fix potential race between s_group_info online resizing and access

On Tue, Feb 18, 2020 at 07:08:50PM -0800, Suraj Jitindar Singh wrote:
> During an online resize an array of pointers to s_group_info gets replaced
> so it can get enlarged. If there is a concurrent access to the array in
> ext4_get_group_info() and this memory has been reused then this can lead to
> an invalid memory access.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206443
> Signed-off-by: Suraj Jitindar Singh <[email protected]>
> Cc: [email protected]

Thanks, applied.

- Ted