linux-ext4 - Re: [PATCH v2] ext4: set csum seed in tmp inode while migrating to extents

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]

Date:   Tue, 14 Dec 2021 16:49:45 -0800
From:   "Darrick J. Wong" <djwong@...nel.org>
To:     Luís Henriques <lhenriques@...e.de>
Cc:     Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Jeroen van Wolffelaar <jeroen@...ffelaar.nl>
Subject: Re: [PATCH v2] ext4: set csum seed in tmp inode while migrating to
 extents

On Tue, Dec 14, 2021 at 05:50:58PM +0000, Luís Henriques wrote:
> When migrating to extents, the temporary inode will have it's own checksum
> seed.  This means that, when swapping the inodes data, the inode checksums
> will be incorrect.
> 
> This can be fixed by recalculating the extents checksums again.  Or simply
> by copying the seed into the temporary inode.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=213357
> Reported-by: Jeroen van Wolffelaar <jeroen@...ffelaar.nl>
> Signed-off-by: Luís Henriques <lhenriques@...e.de>
> ---
>  fs/ext4/migrate.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> changes since v1:
> 
> * Dropped tmp_ei variable
> * ->i_csum_seed is now initialised immediately after tmp_inode is created
> * New comment about the seed initialization and stating that recovery
>   needs to be fixed.
> 
> Cheers,
> --
> Luís
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 7e0b4f81c6c0..36dfc88ce05b 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -459,6 +459,17 @@ int ext4_ext_migrate(struct inode *inode)
>  		ext4_journal_stop(handle);
>  		goto out_unlock;
>  	}
> +	/*
> +	 * Use the correct seed for checksum (i.e. the seed from 'inode').  This
> +	 * is so that the metadata blocks will have the correct checksum after
> +	 * the migration.
> +	 *
> +	 * Note however that, if a crash occurs during the migration process,
> +	 * the recovery process is broken because the tmp_inode checksums will
> +	 * be wrong and the orphans cleanup will fail.

...and then what does the user do?

--D

> +	 */
> +	ei = EXT4_I(inode);
> +	EXT4_I(tmp_inode)->i_csum_seed = ei->i_csum_seed;
>  	i_size_write(tmp_inode, i_size_read(inode));
>  	/*
>  	 * Set the i_nlink to zero so it will be deleted later
> @@ -502,7 +513,6 @@ int ext4_ext_migrate(struct inode *inode)
>  		goto out_tmp_inode;
>  	}
>  
> -	ei = EXT4_I(inode);
>  	i_data = ei->i_data;
>  	memset(&lb, 0, sizeof(lb));
>