linux-kernel - Re: [PATCH -next] ext4: recover csum seed of tmp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220617104441.lfhuca6tflg2oxah@quack3.lan>
Date:   Fri, 17 Jun 2022 12:44:41 +0200
From:   Jan Kara <jack@...e.cz>
To:     Li Lingfeng <lilingfeng3@...wei.com>
Cc:     linux-ext4@...r.kernel.org, tytso@....edu,
        adilger.kernel@...ger.ca, linux-kernel@...r.kernel.org,
        jack@...e.cz, yi.zhang@...wei.com, yukuai3@...wei.com,
        libaokun2@...wei.com
Subject: Re: [PATCH -next] ext4: recover csum seed of tmp_inode after
 migrating to extents

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@...wei.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@...e.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@...e.com>
SUSE Labs, CR