offline shrink bug w/ testcase & image

This fs image is problematic for offline resize2fs shrink. (sorry, it expands to 11G)

# resize2fs <image> 2507776

will lead to corruption[1] because the new size is in the middle of a block group which has its inode & block bitmaps towards the end; the bitmaps are left beyond the end of the new filesystem.

I've been trying to find my way around resize2fs, but bleah, I must admit to being perpetually lost.

I'm not sure there's any simple functionality to handle this case; does anything else ever need to move bitmaps?

I was looking at blocks_to_move(), and within the "shrinking" loop I added logic to detect this case:

                /* If this (first) group will remain but bitmaps are past EOF, move them */
                if (g == ext2fs_group_of_blk2(fs, ext2fs_blocks_count(fs->super))) {
                        int retval;
                        blk64_t new_size = ext2fs_blocks_count(fs->super);

                        if ((IS_BLOCK_BM(fs, g, blk) || IS_INODE_BM(fs, g, blk)) &&
                            (blk >= new_size)) {

                            <do something>
                        }

but at this point I'm not sure what to do. The magic around identifying & marking & reserving & accounting for moved blocks has me stumped.

I'm happy to keep looking at it but could use some pointers, or if the fix is obvious to someone, by all means have at it. :)

Thanks,
-Eric

[1] - like this -

e2fsck: Group descriptors look bad... trying backup blocks... Block bitmap for group 76 is not in group. (block 2508258) Relocate? yes

Inode bitmap for group 76 is not in group. (block 2508259) Relocate? yes

One or more block group descriptor checksums are invalid. Fix? yes

Group descriptor 76 checksum is invalid. FIXED.
Pass 1: Checking inodes, blocks, and sizes
Relocating group 76's block bitmap to 2097161... Relocating group 76's inode bitmap to 2097162...
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Block bitmap differences: +(2097161--2097162) +(2490368--2490874) Fix? yes

Free blocks count wrong for group #64 (28187, counted=28185). Fix? yes

Free blocks count wrong (2430591, counted=2430589). Fix? yes

testfile: ***** FILE SYSTEM WAS MODIFIED *****
testfile: 11/624624 files (0.0% non-contiguous), 77187/2507776 blocks

It is possible to have a flex_bg filesystem with block groups which have inode & block bitmaps at some point well past the start of the group.

If an offline shrink puts the new size somewhere between the start of the block group and the (old) location of the bitmaps, they can be left beyond the end of the filesystem, i.e. result in fs corruption.

Check each remaining block group for whether its bitmaps are beyond the end of the new filesystem, and reallocate them in a new location if needed.

---

I have no idea if this is the right approach or not ;)

index aa2364c..263dea1 100644
@@ -854,6 +854,7 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
 	dgrp_t		i, max_groups, g;
 	blk64_t		blk, group_blk;
 	blk64_t		old_blocks, new_blocks;
+	blk64_t		new_size;
 	unsigned int	meta_bg, meta_bg_size;
 	errcode_t	retval;
 	ext2_filsys 	fs, old_fs;

@@ -882,6 +883,32 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
 	fs = rfs->new_fs;
 
 	/*
+	 * If we're shrinking the filesystem, we need to move any group's
+	 * bitmaps which are beyond the end of the new filesystem.
+	 */
+	new_size = ext2fs_blocks_count(fs->super);
+	if (ext2fs_blocks_count(fs->super) < ext2fs_blocks_count(old_fs->super)) {
+		for (g = 0; g < fs->group_desc_count; g++) {
+			/*
+			 * ext2fs_allocate_group_table re-allocates bitmaps
+			 * which are set to block 0.
+			 */
+			if (ext2fs_block_bitmap_loc(fs, g) >= new_size) {
+				ext2fs_block_bitmap_loc_set(fs, g, 0);
+				retval = ext2fs_allocate_group_table(fs, g, 0);
+				if (retval)
+					return retval;
+			} 
+			if (ext2fs_inode_bitmap_loc(fs, g) >= new_size) {
+				ext2fs_inode_bitmap_loc_set(fs, g, 0);
+				retval = ext2fs_allocate_group_table(fs, g, 0);
+				if (retval)
+					return retval;
+			} 
+		}
+	}
+
+	/*
 	 * If we're shrinking the filesystem, we need to move all of
 	 * the blocks that don't fit any more
 	 */

Looks good, thanks for the patch. I made a few minor changes to fix up some extra whitespace and a minor optimization.

- Ted

index 28131c2..204b10a 100644
@@ -889,7 +889,7 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
 	 * bitmaps which are beyond the end of the new filesystem.
 	 */
 	new_size = ext2fs_blocks_count(fs->super);
-	if (ext2fs_blocks_count(fs->super) < ext2fs_blocks_count(old_fs->super)) {
+	if (new_size < ext2fs_blocks_count(old_fs->super)) {
 		for (g = 0; g < fs->group_desc_count; g++) {
 			/*
 			 * ext2fs_allocate_group_table re-allocates bitmaps

@@ -900,13 +900,13 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
 				retval = ext2fs_allocate_group_table(fs, g, 0);
 				if (retval)
 					return retval;
-			} 
+			}
 			if (ext2fs_inode_bitmap_loc(fs, g) >= new_size) {
 				ext2fs_inode_bitmap_loc_set(fs, g, 0);
 				retval = ext2fs_allocate_group_table(fs, g, 0);
 				if (retval)
 					return retval;
-			} 
+			}
 		}
 	}
 

Great, thanks for the review.