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

[Andreas Dilger <adilger@dilger.ca>]

On Nov 1, 2026, at 22:24, Zhao Gongyi <zhaogongyi@bytedance.com> wrote:
> 
> 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>

I think a lot of this class of bug could be avoided by doing very minimal
checks on the group descriptors initially (and other metadata, enough to
load and validate the journal inode itself is sane). The journal inode is
otherwise read-only so its allocations should never be modified.  Then
replaying the journal if it is dirty, and restarting the superblock/GDT
checks from the beginning to validate the _current_ blocks are sane.

That should only be done once per mount. To keep sanity after checks, we
could validate the journal never contains updates to its own inode (or at
least not until an SMR "wandering journal/log structure" feature exists). 

Cheers, Andreas

> ---
> 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)


Cheers, Andreas