[PATCH -next] ext4: recover csum seed of tmp_inode after migrating to extents

[PATCH -next] ext4: recover csum seed of tmp_inode after migrating to extents

[Li Lingfeng]

When migrating to extents, the checksum seed of temporary inode
need to be replaced by inode's, otherwise the inode checksums
will be incorrect when swapping the inodes data.

However, the temporary inode can not match it's checksum to
itself since it has lost it's own checksum seed.

mkfs.ext4 -F /dev/sdc
mount /dev/sdc /mnt/sdc
xfs_io -fc "pwrite 4k 4k" -c "fsync" /mnt/sdc/testfile
chattr -e /mnt/sdc/testfile
chattr +e /mnt/sdc/testfile
umount /dev/sdc
fsck -fn /dev/sdc

========
...
Pass 1: Checking inodes, blocks, and sizes
Inode 13 passes checks, but checksum does not match inode.  Fix? no
...
========

The fix is simple, save the checksum seed of temporary inode, and
recover it after migrating to extents.

Fixes: e81c9302a6c3 ("ext4: set csum seed in tmp inode while migrating to extents")
Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/ext4/migrate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 49912814f3d8..04320715d61f 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -417,7 +417,7 @@ int ext4_ext_migrate(struct inode *inode)
 	struct inode *tmp_inode = NULL;
 	struct migrate_struct lb;
 	unsigned long max_entries;
-	__u32 goal;
+	__u32 goal, tmp_csum_seed;
 	uid_t owner[2];
 
 	/*
@@ -465,6 +465,7 @@ int ext4_ext_migrate(struct inode *inode)
 	 * the migration.
 	 */
 	ei = EXT4_I(inode);
+	tmp_csum_seed = EXT4_I(tmp_inode)->i_csum_seed;
 	EXT4_I(tmp_inode)->i_csum_seed = ei->i_csum_seed;
 	i_size_write(tmp_inode, i_size_read(inode));
 	/*
@@ -575,6 +576,7 @@ int ext4_ext_migrate(struct inode *inode)
 	 * the inode is not visible to user space.
 	 */
 	tmp_inode->i_blocks = 0;
+	EXT4_I(tmp_inode)->i_csum_seed = tmp_csum_seed;
 
 	/* Reset the extent details */
 	ext4_ext_tree_init(handle, tmp_inode);
-- 
2.31.1

Re: [PATCH -next] ext4: recover csum seed of tmp_inode after migrating to extents

[Jan Kara]

On Fri 17-06-22 14:25:15, Li Lingfeng wrote:
> When migrating to extents, the checksum seed of temporary inode
> need to be replaced by inode's, otherwise the inode checksums
> will be incorrect when swapping the inodes data.
> 
> However, the temporary inode can not match it's checksum to
> itself since it has lost it's own checksum seed.
> 
> mkfs.ext4 -F /dev/sdc
> mount /dev/sdc /mnt/sdc
> xfs_io -fc "pwrite 4k 4k" -c "fsync" /mnt/sdc/testfile
> chattr -e /mnt/sdc/testfile
> chattr +e /mnt/sdc/testfile
> umount /dev/sdc
> fsck -fn /dev/sdc
> 
> ========
> ...
> Pass 1: Checking inodes, blocks, and sizes
> Inode 13 passes checks, but checksum does not match inode.  Fix? no
> ...
> ========
> 
> The fix is simple, save the checksum seed of temporary inode, and
> recover it after migrating to extents.
> 
> Fixes: e81c9302a6c3 ("ext4: set csum seed in tmp inode while migrating to extents")
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>

Yeah, the migration code and checksums never quite worked together. That's
the reason why we'd decided to deprecate this code. But I guess this fix is
simple enough and improves things so feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/migrate.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 49912814f3d8..04320715d61f 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -417,7 +417,7 @@ int ext4_ext_migrate(struct inode *inode)
>  	struct inode *tmp_inode = NULL;
>  	struct migrate_struct lb;
>  	unsigned long max_entries;
> -	__u32 goal;
> +	__u32 goal, tmp_csum_seed;
>  	uid_t owner[2];
>  
>  	/*
> @@ -465,6 +465,7 @@ int ext4_ext_migrate(struct inode *inode)
>  	 * the migration.
>  	 */
>  	ei = EXT4_I(inode);
> +	tmp_csum_seed = EXT4_I(tmp_inode)->i_csum_seed;
>  	EXT4_I(tmp_inode)->i_csum_seed = ei->i_csum_seed;
>  	i_size_write(tmp_inode, i_size_read(inode));
>  	/*
> @@ -575,6 +576,7 @@ int ext4_ext_migrate(struct inode *inode)
>  	 * the inode is not visible to user space.
>  	 */
>  	tmp_inode->i_blocks = 0;
> +	EXT4_I(tmp_inode)->i_csum_seed = tmp_csum_seed;
>  
>  	/* Reset the extent details */
>  	ext4_ext_tree_init(handle, tmp_inode);
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH -next] ext4: recover csum seed of tmp_inode after migrating to extents

[Theodore Ts'o]

On Fri, 17 Jun 2022 14:25:15 +0800, Li Lingfeng wrote:
> When migrating to extents, the checksum seed of temporary inode
> need to be replaced by inode's, otherwise the inode checksums
> will be incorrect when swapping the inodes data.
> 
> However, the temporary inode can not match it's checksum to
> itself since it has lost it's own checksum seed.
> 
> [...]

Applied, thanks!

[1/1] ext4: recover csum seed of tmp_inode after migrating to extents
      commit: d335dce171806dd285494cdb49ca2959825c6bb9

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>