[PATCH] ext4: reject impossible group free cluster counts

[PATCH] ext4: reject impossible group free cluster counts

[Zhao Gongyi]

ext4_count_free_clusters() initializes global free-space accounting from
group descriptors. A corrupt bigalloc image advertises 201327130 free
clusters in a 32-cluster group. The mount succeeds and delayed
allocation later sees an impossible free-space total.

Reject counts above the group's cluster capacity during descriptor
validation. Reuse the existing capacity calculation so bigalloc and
short final groups use the same bound as the block bitmap.

Journal replay can overwrite a descriptor after the initial check. A
crafted transaction changes a valid count to 32769 in a 32768-cluster
group; a pre-replay-only check lets both read-only and read-write mounts
succeed. Recheck after recovery, before initializing free-space
counters. Limit the second scan to mounts needing recovery.

Apply the bound to read-only mounts too, since they use the descriptor
count for in-memory accounting.

Signed-off-by: Zhao Gongyi <zhaogongyi@bytedance.com>
---
 fs/ext4/balloc.c | 10 +++++-----
 fs/ext4/ext4.h   |  2 ++
 fs/ext4/super.c  | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 778fe87..0f36630 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -159,8 +159,8 @@ static unsigned ext4_num_overhead_clusters(struct super_block *sb,
 	return num_clusters;
 }
 
-static unsigned int num_clusters_in_group(struct super_block *sb,
-					  ext4_group_t block_group)
+unsigned int ext4_num_clusters_in_group(struct super_block *sb,
+					ext4_group_t block_group)
 {
 	unsigned int blocks;
 
@@ -228,7 +228,7 @@ static int ext4_init_block_bitmap(struct super_block *sb,
 	 * the blocksize * 8 ( which is the size of bitmap ), set rest
 	 * of the block bitmap to 1
 	 */
-	ext4_mark_bitmap_end(num_clusters_in_group(sb, block_group),
+	ext4_mark_bitmap_end(ext4_num_clusters_in_group(sb, block_group),
 			     sb->s_blocksize * 8, bh->b_data);
 	return 0;
 }
@@ -240,7 +240,7 @@ unsigned ext4_free_clusters_after_init(struct super_block *sb,
 				       ext4_group_t block_group,
 				       struct ext4_group_desc *gdp)
 {
-	return num_clusters_in_group(sb, block_group) -
+	return ext4_num_clusters_in_group(sb, block_group) -
 		ext4_num_overhead_clusters(sb, block_group, gdp);
 }
 
@@ -313,7 +313,7 @@ static ext4_fsblk_t ext4_valid_block_bitmap_padding(struct super_block *sb,
 {
 	ext4_grpblk_t next_zero_bit;
 	unsigned long bitmap_size = sb->s_blocksize * 8;
-	unsigned int offset = num_clusters_in_group(sb, block_group);
+	unsigned int offset = ext4_num_clusters_in_group(sb, block_group);
 
 	if (bitmap_size <= offset)
 		return 0;
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8d3af06..3956545 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3468,6 +3468,8 @@ extern ext4_fsblk_t ext4_inode_bitmap(struct super_block *sb,
 				      struct ext4_group_desc *bg);
 extern ext4_fsblk_t ext4_inode_table(struct super_block *sb,
 				     struct ext4_group_desc *bg);
+unsigned int ext4_num_clusters_in_group(struct super_block *sb,
+					ext4_group_t block_group);
 extern __u32 ext4_free_group_clusters(struct super_block *sb,
 				      struct ext4_group_desc *bg);
 extern __u32 ext4_free_inodes_count(struct super_block *sb,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 7d8aedf..83e4ef6 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3355,6 +3355,36 @@ void ext4_group_desc_csum_set(struct super_block *sb, __u32 block_group,
 	gdp->bg_checksum = ext4_group_desc_csum(sb, block_group, gdp);
 }
 
+static bool ext4_check_group_free_clusters(struct super_block *sb,
+					   ext4_group_t group,
+					  struct ext4_group_desc *gdp)
+{
+	unsigned int free_clusters = ext4_free_group_clusters(sb, gdp);
+	unsigned int group_clusters = ext4_num_clusters_in_group(sb, group);
+
+	if (free_clusters > group_clusters) {
+		ext4_msg(sb, KERN_ERR,
+			 "%s: group %u has invalid free clusters count %u > %u",
+			 __func__, group, free_clusters, group_clusters);
+		return false;
+	}
+	return true;
+}
+
+static int ext4_check_replayed_free_clusters(struct super_block *sb)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	ext4_group_t group;
+
+	for (group = 0; group < sbi->s_groups_count; group++) {
+		struct ext4_group_desc *gdp = ext4_get_group_desc(sb, group, NULL);
+
+		if (!ext4_check_group_free_clusters(sb, group, gdp))
+			return -EFSCORRUPTED;
+	}
+	return 0;
+}
+
 /* Called at mount-time, super-block is locked */
 static int ext4_check_descriptors(struct super_block *sb,
 				  ext4_fsblk_t sb_block,
@@ -3467,6 +3497,8 @@ static int ext4_check_descriptors(struct super_block *sb,
 			}
 		}
 		ext4_unlock_group(sb, i);
+		if (!ext4_check_group_free_clusters(sb, i, gdp))
+			return 0;
 		if (!flexbg_flag)
 			first_block += EXT4_BLOCKS_PER_GROUP(sb);
 	}
@@ -5567,6 +5599,12 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		err = ext4_load_and_init_journal(sb, es, ctx);
 		if (err)
 			goto failed_mount3a;
+		/* Journal replay may change descriptors after the initial check. */
+		if (needs_recovery) {
+			err = ext4_check_replayed_free_clusters(sb);
+			if (err)
+				goto failed_mount_wq;
+		}
 		if (bdev_read_only(sb->s_bdev))
 		    needs_recovery = 0;
 	} else if (test_opt(sb, NOLOAD) && !sb_rdonly(sb) &&

base-commit: 8c4f6e3376348fc3162eed86a05b933921c8f9ed
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] ext4: reject impossible group free cluster counts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4: journal replay bypasses ext4_check_replayed_free_clusters()
--

commit 57978228cd9b85af900a61d55a732ad888e33b8c
Author: Zhao Gongyi <zhaogongyi@bytedance.com>

ext4: reject impossible group free cluster counts

This commit rejects group descriptor free cluster counts that exceed the
group's cluster capacity during mount-time descriptor validation, and
rechecks the free cluster counts after journal replay when recovery is
needed.

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 7d8aedfbbb87f..b7dd1d4a3b179 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3355,6 +3355,36 @@ void ext4_group_desc_csum_set(struct super_block *sb, __u32 block_group,
>  	gdp->bg_checksum = ext4_group_desc_csum(sb, block_group, gdp);
>  }
>  
> +static bool ext4_check_group_free_clusters(struct super_block *sb,
> +					   ext4_group_t group,
> +					  struct ext4_group_desc *gdp)
> +{
> +	unsigned int free_clusters = ext4_free_group_clusters(sb, gdp);
> +	unsigned int group_clusters = ext4_num_clusters_in_group(sb, group);
> +
> +	if (free_clusters > group_clusters) {
> +		ext4_msg(sb, KERN_ERR,
> +			 "%s: group %u has invalid free clusters count %u > %u",
> +			 __func__, group, free_clusters, group_clusters);
> +		return false;
> +	}
> +	return true;
> +}
> +
> +static int ext4_check_replayed_free_clusters(struct super_block *sb)
> +{
> +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> +	ext4_group_t group;
> +
> +	for (group = 0; group < sbi->s_groups_count; group++) {
> +		struct ext4_group_desc *gdp = ext4_get_group_desc(sb, group, NULL);
> +
> +		if (!ext4_check_group_free_clusters(sb, group, gdp))
> +			return -EFSCORRUPTED;
> +	}
> +	return 0;
> +}

[ ... ]

> @@ -5567,6 +5599,12 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  		err = ext4_load_and_init_journal(sb, es, ctx);
>  		if (err)
>  			goto failed_mount3a;
> +		/* Journal replay may change descriptors after the initial check. */
> +		if (needs_recovery) {
> +			err = ext4_check_replayed_free_clusters(sb);
> +			if (err)
> +				goto failed_mount_wq;
> +		}

[Severity: High]
Can a journal transaction that replays both the superblock buffer and a
group descriptor buffer bypass ext4_check_replayed_free_clusters()?

During __ext4_fill_super(), ext4_check_geometry() and
ext4_group_desc_init() -> ext4_check_descriptors() validate the initial
superblock and group descriptors before journal recovery. Next,
ext4_load_and_init_journal() -> ext4_load_journal() ->
jbd2_journal_load() -> jbd2_journal_recover() -> jbd2_do_replay() replays
journal blocks into the block device buffer cache.

Because sbi->s_es points directly into sbi->s_sbh->b_data, a journal
transaction containing logical_sb_block can overwrite s_blocks_count_lo,
s_blocks_count_hi, and s_first_data_block in sbi->s_es (ext4_load_journal()
only saves and restores the EXT4_S_ERR_START..EXT4_S_ERR_END range).

When ext4_check_replayed_free_clusters() subsequently calls
ext4_check_group_free_clusters() -> ext4_num_clusters_in_group() for the
final block group:

fs/ext4/balloc.c:ext4_num_clusters_in_group() {
	if (block_group == ext4_get_groups_count(sb) - 1) {
		blocks = ext4_blocks_count(EXT4_SB(sb)->s_es) -
			ext4_group_first_block_no(sb, block_group);
	} else
		blocks = EXT4_BLOCKS_PER_GROUP(sb);
	return EXT4_NUM_B2C(EXT4_SB(sb), blocks);
}

blocks is computed from the live, unvalidated sbi->s_es buffer. If the
replayed s_blocks_count is smaller than ext4_group_first_block_no(sb,
block_group), the unsigned int blocks calculation underflows to near
UINT_MAX; if larger, blocks can exceed EXT4_BLOCKS_PER_GROUP(sb).
EXT4_NUM_B2C() then returns an inflated group_clusters upper bound, allowing
an impossible replayed bg_free_blocks_count to pass the free_clusters <=
group_clusters check and corrupt in-memory free-cluster accounting as well
as bitmap initialization in ext4_init_block_bitmap() and
ext4_valid_block_bitmap_padding().

In addition, only calling ext4_check_replayed_free_clusters() after
recovery leaves any replayed changes to bg_block_bitmap, bg_inode_bitmap,
bg_inode_table, bg_checksum, and bg_flags (EXT4_BG_INODE_ZEROED for
first_not_zeroed) unvalidated. Should superblock geometry and full group
descriptors via ext4_check_descriptors() be revalidated after journal
replay instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102042428.65204-1-zhaogongyi@bytedance.com?part=1