On 2024/2/20 1:10, Jan Kara wrote:
> Currently we ignore s_clusters_per_group field in the on-disk superblock
> if bigalloc feature is not enabled. However e2fsprogs don't even open
> the filesystem if s_clusters_per_group is invalid. This results in an
> odd state where kernel happily works with the filesystem while even
> e2fsck refuses to touch it. Verify that s_clusters_per_group is valid
> even if bigalloc feature is not enabled to make things consistent. Due
> to current e2fsprogs behavior it is unlikely there are filesystems out
> in the wild (except for intentionally fuzzed ones) with invalid
> s_clusters_per_group counts.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
Thanks, looks good to me.
Reviewed-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
> fs/ext4/super.c | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> Changes since v1:
> * share code checking s_clusters_per_group for !bigalloc & bigalloc configs
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 0f931d0c227d..0a34e0b23541 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4422,22 +4422,6 @@ static int ext4_handle_clustersize(struct super_block *sb)
> }
> sbi->s_cluster_bits = le32_to_cpu(es->s_log_cluster_size) -
> le32_to_cpu(es->s_log_block_size);
> - sbi->s_clusters_per_group =
> - le32_to_cpu(es->s_clusters_per_group);
> - if (sbi->s_clusters_per_group > sb->s_blocksize * 8) {
> - ext4_msg(sb, KERN_ERR,
> - "#clusters per group too big: %lu",
> - sbi->s_clusters_per_group);
> - return -EINVAL;
> - }
> - if (sbi->s_blocks_per_group !=
> - (sbi->s_clusters_per_group * (clustersize / sb->s_blocksize))) {
> - ext4_msg(sb, KERN_ERR, "blocks per group (%lu) and "
> - "clusters per group (%lu) inconsistent",
> - sbi->s_blocks_per_group,
> - sbi->s_clusters_per_group);
> - return -EINVAL;
> - }
> } else {
> if (clustersize != sb->s_blocksize) {
> ext4_msg(sb, KERN_ERR,
> @@ -4451,9 +4435,21 @@ static int ext4_handle_clustersize(struct super_block *sb)
> sbi->s_blocks_per_group);
> return -EINVAL;
> }
> - sbi->s_clusters_per_group = sbi->s_blocks_per_group;
> sbi->s_cluster_bits = 0;
> }
> + sbi->s_clusters_per_group = le32_to_cpu(es->s_clusters_per_group);
> + if (sbi->s_clusters_per_group > sb->s_blocksize * 8) {
> + ext4_msg(sb, KERN_ERR, "#clusters per group too big: %lu",
> + sbi->s_clusters_per_group);
> + return -EINVAL;
> + }
> + if (sbi->s_blocks_per_group !=
> + (sbi->s_clusters_per_group * (clustersize / sb->s_blocksize))) {
> + ext4_msg(sb, KERN_ERR,
> + "blocks per group (%lu) and clusters per group (%lu) inconsistent",
> + sbi->s_blocks_per_group, sbi->s_clusters_per_group);
> + return -EINVAL;
> + }
> sbi->s_cluster_ratio = clustersize / sb->s_blocksize;
>
> /* Do we have standard group size of clustersize * 8 blocks ? */
>
