ext4: fix online resize group descriptors corruption

From: Frederic Bohe <[email protected]>

This is the patch for the group descriptor table corruption during online resize pointed out by Theodore Tso.
The issue was due to the ext4 group descriptor which can be either 32 or 64 bytes long.
Only the 64 bytes structure was taken into account.

---
URL for the discussion about this issue: http://article.gmane.org/gmane.comp.file-systems.ext4/7226

The patch has been tested with linux-2.6.26-rc4-ext4-1 and e2fsprogs-git-master-2008-06-07-365857912e27914afa8857af5adf74ee19ca9e03 under kvm and loopback device and without mballoc.

It has also been tested with linux-2.6.26-rc5, ext4-patch-queue-f245e945ae491e7931076706217b63d4bf0b5a1b and e2fsprogs-365857912e27914afa8857af5adf74ee19ca9e03 with a physical hard disk and without mballoc

The patch does not correct the mballoc case (Oops in ext4_mb_release). There is still an issue when resizing a flex_bg filesystem.

 resize.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff -rup a/fs/ext4/resize.c b/fs/ext4/resize.c
@@ -855,7 +855,8 @@ int ext4_group_add(struct super_block *s
 	 */
 
 	/* Update group descriptor block for new group */
-	gdp = (struct ext4_group_desc *)primary->b_data + gdb_off;
+	gdp = (struct ext4_group_desc *)(
+	       (__u8 *)primary->b_data + gdb_off * EXT4_DESC_SIZE(sb));
 
 	ext4_block_bitmap_set(sb, gdp, input->block_bitmap); /* LV FIXME */
 	ext4_inode_bitmap_set(sb, gdp, input->inode_bitmap); /* LV FIXME */



From: Frederic Bohe <[email protected]>

This is the patch for the group descriptor table corruption during online resize pointed out by Theodore Tso.
The issue was due to the ext4 group descriptor which can be either 32 or 64 bytes long.
Only the 64 bytes structure was taken into account.

Signed-off-by: Frederic Bohe <[email protected]>
---
 resize.c | 3 ++-

 1 file changed, 2 insertions(+), 1 deletion(-)

diff -rup a/fs/ext4/resize.c b/fs/ext4/resize.c
@@ -855,7 +855,8 @@ int ext4_group_add(struct super_block *s
 	 */
 
 	/* Update group descriptor block for new group */
-	gdp = (struct ext4_group_desc *)primary->b_data + gdb_off;
+	gdp = (struct ext4_group_desc *)((char *)primary->b_data +
+	                                 gdb_off * EXT4_DESC_SIZE(sb));
 
 	ext4_block_bitmap_set(sb, gdp, input->block_bitmap); /* LV FIXME */
 	ext4_inode_bitmap_set(sb, gdp, input->inode_bitmap); /* LV FIXME */

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

There was a tab issue in the v2 patch, sorry for the spam...

From: Frederic Bohe <[email protected]>

This is the patch for the group descriptor table corruption during online resize pointed out by Theodore Tso.
The issue was due to the ext4 group descriptor which can be either 32 or 64 bytes long.
Only the 64 bytes structure was taken into account.t.

Signed-off-by: Frederic Bohe <[email protected]>

---
 resize.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff -rup a/fs/ext4/resize.c b/fs/ext4/resize.c
@@ -855,7 +855,8 @@ int ext4_group_add(struct super_block *s
 	 */
 
 	/* Update group descriptor block for new group */
-	gdp = (struct ext4_group_desc *)primary->b_data + gdb_off;
+	gdp = (struct ext4_group_desc *)((char *)primary->b_data +
+					 gdb_off * EXT4_DESC_SIZE(sb));
 
 	ext4_block_bitmap_set(sb, gdp, input->block_bitmap); /* LV FIXME */
 	ext4_inode_bitmap_set(sb, gdp, input->inode_bitmap); /* LV FIXME */

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Andreas,

thank you for this review of Frédéric's patch
and your comments that help.

Frédéric just joined our team a few days ago and
this kind of help is very usefull.

Frédéric has not a great experience in Linux but he has a strong one in embedded systems and telecoms. Correct me where I am wrong Frédéric. ;-)

jean-pierre

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Frederic,

Many thanks! How much testing have you done with the patch? Does it fix growing the filesystems from a filesystem with say, 18 block groups to 32 block groups? Have you tried with with different filesystem features, including with and without flex_bg and uninit_bg?

Regards,

- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Ted,

Concerning flex_bg and uninit_bg : it's currently not possible to resize a filesystem with those features. It seems there are issues with resize2fs.
We have also used nomballoc switch when mounting because of the oops when unmounting.

We have tried both physical hard disk drive and loop back devices. We have tested several filesystem resize with few (adding 1 group) to many (adding 250 groups) more groups. All give correct group descriptors using dump2fs and no problem when writing files and unmounting filesystems.

This is a typical test case:

mkfs.ext4 -E test_fs -O ^flex_bg /dev/sdc1 10M mount -t ext4dev -o nomballoc /dev/sdc1 /mnt/test dumpe2fs /dev/sdc1
resize2fs /dev/sdc1 100M
dumpe2fs /dev/sdc1
dd if=/dev/urandom of=/mnt/test/data bs=95M count=1 umount /mnt/test
fsck.ext4 -f /dev/sdc1

This test being run for several sizes of filesystem (from 10M with 1K blocks to 1G with 4K blocks).

Let me know if we have missed something or if you want more testings.

Regards

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html