Re: [PATCH RFC 06/17] ext4: add ext4_block_bitmap_csum_set_range() for incremental checksum update

在 2026/5/9 05:46, [email protected] 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] ext4_block_bitmap_csum_set_range() passes an incorrect `total_bits` value to `crc32c_flip_range()` when `EXT4_CLUSTERS_PER_GROUP(sb)` is not a multiple of 8, leading to corrupted checksums.
> --
>
> commit 49b9ace366232b1d340c2393c53c09a694007211
> Author: Baokun Li <[email protected]>
>
> ext4: add ext4_block_bitmap_csum_set_range() for incremental checksum update
>
> This commit adds a helper function to incrementally update the block bitmap
> checksum using crc32c_flip_range(). This efficiently computes the CRC delta
> for a range of flipped bits without re-scanning the entire bitmap buffer.
>
>> diff --git a/fs/ext4/bitmap.c b/fs/ext4/bitmap.c
>> index 46affc9e80ca3..00b0a3c748596 100644
>> --- a/fs/ext4/bitmap.c
>> +++ b/fs/ext4/bitmap.c
> [ ... ]
>> +void ext4_block_bitmap_csum_set_range(struct super_block *sb,
>> +				      struct ext4_group_desc *gdp,
>> +				      ext4_grpblk_t offset, ext4_grpblk_t len)
>> +{
>> +	__u32 new_csum, old_csum;
>> +
>> +	if (!ext4_has_feature_metadata_csum(sb))
>> +		return;
>> +
>> +	old_csum = ext4_block_bitmap_csum_get(sb, gdp);
>> +	new_csum = crc32c_flip_range(old_csum, EXT4_CLUSTERS_PER_GROUP(sb),
>> +				     offset, len);
> Does passing EXT4_CLUSTERS_PER_GROUP(sb) here cause a checksum mismatch if
> the clusters per group is not a multiple of 8?
>
> In ext4_block_bitmap_csum_set(), the full checksum is calculated over exactly
> EXT4_CLUSTERS_PER_GROUP(sb) / 8 bytes, ignoring fractional trailing bits:
>
> fs/ext4/bitmap.c:ext4_block_bitmap_csum_set() {
>     int sz = EXT4_CLUSTERS_PER_GROUP(sb) / 8;
>     ...
>     csum = ext4_chksum(sbi->s_csum_seed, (__u8 *)bh->b_data, sz);
> }
>
> If crc32c_flip_range() uses the unaligned bit count as total_bits, the
> trailing_bits shift calculation will use the wrong baseline length. This
> could result in an incorrect checksum delta being applied for bit flips
> within the byte-aligned region.
>
> Would this also erroneously apply a CRC delta if an allocation or free occurs
> in the fractional trailing bits that the full recalculation ignores?
>
>> +
>> +	ext4_block_bitmap_csum_store(sb, gdp, new_csum);
>> +}


Yes, I missed the case of errors=continue. In the next version, I will
revert to full calculation when EXT4_ERROR_FS is set.


Thanks,
Baokun