* [PATCH] ext4: fix resize when resizing within single group
@ 2012-02-02 9:05 Lukas Czerner
2012-02-03 11:13 ` Yongqiang Yang
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Lukas Czerner @ 2012-02-02 9:05 UTC (permalink / raw)
To: linux-ext4; +Cc: Theodore Ts'o, Yongqiang Yang, Milan Broz, Lukas Czerner
When resizing file system in the way that the new size of the file
system is still in the same group (no new groups are added), then we can
hit a BUG_ON in ext4_alloc_group_tables()
BUG_ON(flex_gd->count == 0 || group_data == NULL);
because flex_gd->count is zero. The reason is the missing check for such
case, so the code always extend the last group fully and then attempt to
add more groups, but at that time n_blocks_count is actually smaller
than o_blocks_count.
It can be easily reproduced like this:
mkfs.ext4 -b 4096 /dev/sda 30M
mount /dev/sda /mnt/test
resize2fs /dev/sda 50M
Fix this by checking whether the resize happens within the singe group
and only add that many blocks into the last group to satisfy user
request. Then o_blocks_count == n_blocks_count and the resize will exit
successfully without and attempt to add more groups into the fs.
Also fix mixing together block number and blocks count which might be
confusing and can easily lead to off-by-one errors (but it is actually
not the case here since the two occurrence of this mix-up will cancel
each other).
Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Reported-by: Milan Broz <mbroz@redhat.com>
---
fs/ext4/resize.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index f9d948f..3fed79d 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1582,7 +1582,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
ext4_fsblk_t o_blocks_count;
ext4_group_t o_group;
ext4_group_t n_group;
- ext4_grpblk_t offset;
+ ext4_grpblk_t offset, add;
unsigned long n_desc_blocks;
unsigned long o_desc_blocks;
unsigned long desc_blocks;
@@ -1605,7 +1605,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
return 0;
ext4_get_group_no_and_offset(sb, n_blocks_count - 1, &n_group, &offset);
- ext4_get_group_no_and_offset(sb, o_blocks_count, &o_group, &offset);
+ ext4_get_group_no_and_offset(sb, o_blocks_count - 1, &o_group, &offset);
n_desc_blocks = (n_group + EXT4_DESC_PER_BLOCK(sb)) /
EXT4_DESC_PER_BLOCK(sb);
@@ -1634,10 +1634,12 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
}
brelse(bh);
- if (offset != 0) {
- /* extend the last group */
- ext4_grpblk_t add;
- add = EXT4_BLOCKS_PER_GROUP(sb) - offset;
+ /* extend the last group */
+ if (n_group == o_group)
+ add = n_blocks_count - o_blocks_count;
+ else
+ add = EXT4_BLOCKS_PER_GROUP(sb) - (offset + 1);
+ if (add > 0) {
err = ext4_group_extend_no_check(sb, o_blocks_count, add);
if (err)
goto out;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: fix resize when resizing within single group
2012-02-02 9:05 [PATCH] ext4: fix resize when resizing within single group Lukas Czerner
@ 2012-02-03 11:13 ` Yongqiang Yang
2012-02-21 0:20 ` Eric Sandeen
2012-02-21 4:03 ` Ted Ts'o
2 siblings, 0 replies; 4+ messages in thread
From: Yongqiang Yang @ 2012-02-03 11:13 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-ext4, Theodore Ts'o, Milan Broz
Thanks! The patch looks good to me.
On Thu, Feb 2, 2012 at 5:05 PM, Lukas Czerner <lczerner@redhat.com> wrote:
> When resizing file system in the way that the new size of the file
> system is still in the same group (no new groups are added), then we can
> hit a BUG_ON in ext4_alloc_group_tables()
>
> BUG_ON(flex_gd->count == 0 || group_data == NULL);
>
> because flex_gd->count is zero. The reason is the missing check for such
> case, so the code always extend the last group fully and then attempt to
> add more groups, but at that time n_blocks_count is actually smaller
> than o_blocks_count.
>
> It can be easily reproduced like this:
>
> mkfs.ext4 -b 4096 /dev/sda 30M
> mount /dev/sda /mnt/test
> resize2fs /dev/sda 50M
>
> Fix this by checking whether the resize happens within the singe group
> and only add that many blocks into the last group to satisfy user
> request. Then o_blocks_count == n_blocks_count and the resize will exit
> successfully without and attempt to add more groups into the fs.
>
> Also fix mixing together block number and blocks count which might be
> confusing and can easily lead to off-by-one errors (but it is actually
> not the case here since the two occurrence of this mix-up will cancel
> each other).
>
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Reported-by: Milan Broz <mbroz@redhat.com>
> ---
> fs/ext4/resize.c | 14 ++++++++------
> 1 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index f9d948f..3fed79d 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1582,7 +1582,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
> ext4_fsblk_t o_blocks_count;
> ext4_group_t o_group;
> ext4_group_t n_group;
> - ext4_grpblk_t offset;
> + ext4_grpblk_t offset, add;
> unsigned long n_desc_blocks;
> unsigned long o_desc_blocks;
> unsigned long desc_blocks;
> @@ -1605,7 +1605,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
> return 0;
>
> ext4_get_group_no_and_offset(sb, n_blocks_count - 1, &n_group, &offset);
> - ext4_get_group_no_and_offset(sb, o_blocks_count, &o_group, &offset);
> + ext4_get_group_no_and_offset(sb, o_blocks_count - 1, &o_group, &offset);
>
> n_desc_blocks = (n_group + EXT4_DESC_PER_BLOCK(sb)) /
> EXT4_DESC_PER_BLOCK(sb);
> @@ -1634,10 +1634,12 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
> }
> brelse(bh);
>
> - if (offset != 0) {
> - /* extend the last group */
> - ext4_grpblk_t add;
> - add = EXT4_BLOCKS_PER_GROUP(sb) - offset;
> + /* extend the last group */
> + if (n_group == o_group)
> + add = n_blocks_count - o_blocks_count;
> + else
> + add = EXT4_BLOCKS_PER_GROUP(sb) - (offset + 1);
> + if (add > 0) {
> err = ext4_group_extend_no_check(sb, o_blocks_count, add);
> if (err)
> goto out;
> --
> 1.7.4.4
>
--
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: fix resize when resizing within single group
2012-02-02 9:05 [PATCH] ext4: fix resize when resizing within single group Lukas Czerner
2012-02-03 11:13 ` Yongqiang Yang
@ 2012-02-21 0:20 ` Eric Sandeen
2012-02-21 4:03 ` Ted Ts'o
2 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2012-02-21 0:20 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-ext4, Theodore Ts'o, Yongqiang Yang, Milan Broz
On 2/2/12 3:05 AM, Lukas Czerner wrote:
> When resizing file system in the way that the new size of the file
> system is still in the same group (no new groups are added), then we can
> hit a BUG_ON in ext4_alloc_group_tables()
>
> BUG_ON(flex_gd->count == 0 || group_data == NULL);
>
> because flex_gd->count is zero. The reason is the missing check for such
> case, so the code always extend the last group fully and then attempt to
> add more groups, but at that time n_blocks_count is actually smaller
> than o_blocks_count.
>
> It can be easily reproduced like this:
>
> mkfs.ext4 -b 4096 /dev/sda 30M
> mount /dev/sda /mnt/test
> resize2fs /dev/sda 50M
>
> Fix this by checking whether the resize happens within the singe group
> and only add that many blocks into the last group to satisfy user
> request. Then o_blocks_count == n_blocks_count and the resize will exit
> successfully without and attempt to add more groups into the fs.
>
> Also fix mixing together block number and blocks count which might be
> confusing and can easily lead to off-by-one errors (but it is actually
> not the case here since the two occurrence of this mix-up will cancel
> each other).
>
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Reported-by: Milan Broz <mbroz@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Ted, if this hits linux-next we can pull it into fedora and close a bug they're tracking. :)
thanks,
-Eric
> ---
> fs/ext4/resize.c | 14 ++++++++------
> 1 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index f9d948f..3fed79d 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1582,7 +1582,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
> ext4_fsblk_t o_blocks_count;
> ext4_group_t o_group;
> ext4_group_t n_group;
> - ext4_grpblk_t offset;
> + ext4_grpblk_t offset, add;
> unsigned long n_desc_blocks;
> unsigned long o_desc_blocks;
> unsigned long desc_blocks;
> @@ -1605,7 +1605,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
> return 0;
>
> ext4_get_group_no_and_offset(sb, n_blocks_count - 1, &n_group, &offset);
> - ext4_get_group_no_and_offset(sb, o_blocks_count, &o_group, &offset);
> + ext4_get_group_no_and_offset(sb, o_blocks_count - 1, &o_group, &offset);
>
> n_desc_blocks = (n_group + EXT4_DESC_PER_BLOCK(sb)) /
> EXT4_DESC_PER_BLOCK(sb);
> @@ -1634,10 +1634,12 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
> }
> brelse(bh);
>
> - if (offset != 0) {
> - /* extend the last group */
> - ext4_grpblk_t add;
> - add = EXT4_BLOCKS_PER_GROUP(sb) - offset;
> + /* extend the last group */
> + if (n_group == o_group)
> + add = n_blocks_count - o_blocks_count;
> + else
> + add = EXT4_BLOCKS_PER_GROUP(sb) - (offset + 1);
> + if (add > 0) {
> err = ext4_group_extend_no_check(sb, o_blocks_count, add);
> if (err)
> goto out;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: fix resize when resizing within single group
2012-02-02 9:05 [PATCH] ext4: fix resize when resizing within single group Lukas Czerner
2012-02-03 11:13 ` Yongqiang Yang
2012-02-21 0:20 ` Eric Sandeen
@ 2012-02-21 4:03 ` Ted Ts'o
2 siblings, 0 replies; 4+ messages in thread
From: Ted Ts'o @ 2012-02-21 4:03 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-ext4, Yongqiang Yang, Milan Broz
On Thu, Feb 02, 2012 at 10:05:55AM +0100, Lukas Czerner wrote:
> When resizing file system in the way that the new size of the file
> system is still in the same group (no new groups are added), then we can
> hit a BUG_ON in ext4_alloc_group_tables()
>
> BUG_ON(flex_gd->count == 0 || group_data == NULL);
>
> because flex_gd->count is zero. The reason is the missing check for such
> case, so the code always extend the last group fully and then attempt to
> add more groups, but at that time n_blocks_count is actually smaller
> than o_blocks_count.
>
> It can be easily reproduced like this:
>
> mkfs.ext4 -b 4096 /dev/sda 30M
> mount /dev/sda /mnt/test
> resize2fs /dev/sda 50M
>
> Fix this by checking whether the resize happens within the singe group
> and only add that many blocks into the last group to satisfy user
> request. Then o_blocks_count == n_blocks_count and the resize will exit
> successfully without and attempt to add more groups into the fs.
>
> Also fix mixing together block number and blocks count which might be
> confusing and can easily lead to off-by-one errors (but it is actually
> not the case here since the two occurrence of this mix-up will cancel
> each other).
>
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Reported-by: Milan Broz <mbroz@redhat.com>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-02-21 4:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-02 9:05 [PATCH] ext4: fix resize when resizing within single group Lukas Czerner
2012-02-03 11:13 ` Yongqiang Yang
2012-02-21 0:20 ` Eric Sandeen
2012-02-21 4:03 ` Ted Ts'o
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).