linux-ext4 - [PATCH] libext2: clear BG_BLOCK_UNINIT in any group containing metadata

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F46835F.5080009@redhat.com>
Date:	Thu, 23 Feb 2012 12:20:15 -0600
From:	Eric Sandeen <sandeen@...hat.com>
To:	ext4 development <linux-ext4@...r.kernel.org>
CC:	Lukáš Czerner <lczerner@...hat.com>
Subject: [PATCH] libext2: clear BG_BLOCK_UNINIT in any group containing metadata

Running a test like the m_uninit test runs:

# mke2fs -F -o Linux -O uninit_bg testfs 131072

results in groups which have blocks allocated for metadata, but
which still have the BLOCK_UNINIT flag set:

Group 2: (Blocks 16385-24576) [INODE_UNINIT, BLOCK_UNINIT, ITABLE_ZEROED]
  Checksum 0x17c2, unused inodes 2048
  Block bitmap at 16385 (+0), Inode bitmap at 16386 (+1)
  Inode table at 16387-16642 (+2)

however, e2fsck doesn't find this error - which seems like another bug.

Doing another test like this:

mkfs.ext4 -I 256 -b 4096 -N 256 -G 1 -g 256 /dev/loop0 1024
mount /dev/loop0 /mnt/test
mkdir /mnt/test/dir
umount /mnt/test
fsck.ext4 -fn /dev/loop0

does find the error on fsck, I think because BLOCK_UNINIT gets
cleared, and the group isn't ignored at fsck time:

Free blocks count wrong for group #2 (253, counted=249).

In the first case, flex_bg is not set; in the 2nd it is set,
but s_log_groups_per_flex == 0 (due to -G 1), so in neither
case is flexbg_size set in ext2fs_allocate_group_table().

This leads to skipping the code which clears BLOCK_UNINIT after
the metadata is allocated.

I think the fix is to unconditionally clear the BLOCK_UNINIT flag
in whatever block group gets metadata allocated to it, as follows.

the m_uninit test needs to be fixed up too, as does e2fsck it seems,
to catch this error in the first place?

Sorry for the long commit message, if the patch looks right I can
try to do better ;)

Signed-off-by: Eric Sandeen <sandeen@...hat.com>
---

diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c
index 9f3d4e0..3462e85 100644
--- a/lib/ext2fs/alloc_tables.c
+++ b/lib/ext2fs/alloc_tables.c
@@ -88,6 +88,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
 	blk64_t		group_blk, start_blk, last_blk, new_blk, blk;
 	dgrp_t		last_grp = 0;
 	int		rem_grps = 0, flexbg_size = 0;
+	dgrp_t		gr;
 
 	group_blk = ext2fs_group_first_block2(fs, group);
 	last_blk = ext2fs_group_last_block2(fs, group);
@@ -141,13 +142,13 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
 			return retval;
 		ext2fs_mark_block_bitmap2(bmap, new_blk);
 		ext2fs_block_bitmap_loc_set(fs, group, new_blk);
+		gr = ext2fs_group_of_blk2(fs, new_blk);
+		ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
 		if (flexbg_size) {
-			dgrp_t gr = ext2fs_group_of_blk2(fs, new_blk);
 			ext2fs_bg_free_blocks_count_set(fs, gr, ext2fs_bg_free_blocks_count(fs, gr) - 1);
 			ext2fs_free_blocks_count_add(fs->super, -1);
-			ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
-			ext2fs_group_desc_csum_set(fs, gr);
 		}
+		ext2fs_group_desc_csum_set(fs, gr);
 	}
 
 	if (flexbg_size) {
@@ -172,13 +173,13 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
 			return retval;
 		ext2fs_mark_block_bitmap2(bmap, new_blk);
 		ext2fs_inode_bitmap_loc_set(fs, group, new_blk);
+		gr = ext2fs_group_of_blk2(fs, new_blk);
+		ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
 		if (flexbg_size) {
-			dgrp_t gr = ext2fs_group_of_blk2(fs, new_blk);
 			ext2fs_bg_free_blocks_count_set(fs, gr, ext2fs_bg_free_blocks_count(fs, gr) - 1);
 			ext2fs_free_blocks_count_add(fs->super, -1);
-			ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
-			ext2fs_group_desc_csum_set(fs, gr);
 		}
+		ext2fs_group_desc_csum_set(fs, gr);
 	}
 
 	/*
@@ -209,14 +210,13 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
 		     j < fs->inode_blocks_per_group;
 		     j++, blk++) {
 			ext2fs_mark_block_bitmap2(bmap, blk);
+			gr = ext2fs_group_of_blk2(fs, blk);
+			ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
 			if (flexbg_size) {
-				dgrp_t gr = ext2fs_group_of_blk2(fs, blk);
 				ext2fs_bg_free_blocks_count_set(fs, gr, ext2fs_bg_free_blocks_count(fs, gr) - 1);
 				ext2fs_free_blocks_count_add(fs->super, -1);
-				ext2fs_bg_flags_clear(fs, gr,
-						     EXT2_BG_BLOCK_UNINIT);
-				ext2fs_group_desc_csum_set(fs, gr);
 			}
+			ext2fs_group_desc_csum_set(fs, gr);
 		}
 		ext2fs_inode_table_loc_set(fs, group, new_blk);
 	}

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