Re: [PATCH v2 2/3] ext4: fix corrupt backup group descriptors after online resize

[Jan Kara]

On Wed 16-11-22 15:28:01, Baokun Li wrote:
> In commit 9a8c5b0d0615 ("ext4: update the backup superblock's at the end
> of the online resize"), it is assumed that update_backups() only updates
> backup superblocks, so each b_data is treated as a backupsuper block to
> update its s_block_group_nr and s_checksum. However, update_backups()
> also updates the backup group descriptors, which causes the backup group
> descriptors to be corrupted.
> 
> The above commit fixes the problem of invalid checksum of the backup
> superblock. The root cause of this problem is that the checksum of
> ext4_update_super() is not set correctly. This problem has been fixed
> in the previous patch ("ext4: fix bad checksum after online resize").
> Therefore, roll back some modifications in the above commit.
> 
> Fixes: 9a8c5b0d0615 ("ext4: update the backup superblock's at the end of the online resize")
> Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>

So I agree commit 9a8c5b0d0615 is broken and does corrupt group
descriptors. However I don't see how PATCH 1/3 in this series would fix all
the problems commit 9a8c5b0d0615 is trying to fix. In particular checksums
on backup superblocks will not be properly set by the resize code AFAICT.

								Honza

> ---
>  fs/ext4/resize.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index cb99b410c9fa..32fbfc173571 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1158,7 +1158,6 @@ static void update_backups(struct super_block *sb, sector_t blk_off, char *data,
>  	while (group < sbi->s_groups_count) {
>  		struct buffer_head *bh;
>  		ext4_fsblk_t backup_block;
> -		struct ext4_super_block *es;
>  
>  		/* Out of journal space, and can't get more - abort - so sad */
>  		err = ext4_resize_ensure_credits_batch(handle, 1);
> @@ -1187,10 +1186,6 @@ static void update_backups(struct super_block *sb, sector_t blk_off, char *data,
>  		memcpy(bh->b_data, data, size);
>  		if (rest)
>  			memset(bh->b_data + size, 0, rest);
> -		es = (struct ext4_super_block *) bh->b_data;
> -		es->s_block_group_nr = cpu_to_le16(group);
> -		if (ext4_has_metadata_csum(sb))
> -			es->s_checksum = ext4_superblock_csum(sb, es);
>  		set_buffer_uptodate(bh);
>  		unlock_buffer(bh);
>  		err = ext4_handle_dirty_metadata(handle, NULL, bh);
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR