linux-ext4 - Re: [PATCH] jbd2: remove useless variable chksum_seen in do_one

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] [day] [month] [year] [list]

Message-ID: <20200819084858.GF1902@quack2.suse.cz>
Date:   Wed, 19 Aug 2020 10:48:58 +0200
From:   Jan Kara <jack@...e.cz>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     Jan Kara <jack@...e.cz>, Shijie Luo <luoshijie1@...wei.com>,
        linux-ext4@...r.kernel.org, linfeilong@...wei.com
Subject: Re: [PATCH] jbd2: remove useless variable chksum_seen in do_one_pass

On Tue 18-08-20 15:14:59, Theodore Y. Ts'o wrote:
> I wonder if this is even cleaner?  What do folks think?
> 
>   	    	    	 - Ted

Yup, looks good to me. Feel free to add:

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

once you add proper changelog and signed-off-by.

									Honza

> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 2ed278f0dced..4373abbfd19a 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -690,14 +690,11 @@ static int do_one_pass(journal_t *journal,
>  			 * number. */
>  			if (pass == PASS_SCAN &&
>  			    jbd2_has_feature_checksum(journal)) {
> -				int chksum_err, chksum_seen;
>  				struct commit_header *cbh =
>  					(struct commit_header *)bh->b_data;
>  				unsigned found_chksum =
>  					be32_to_cpu(cbh->h_chksum[0]);
>  
> -				chksum_err = chksum_seen = 0;
> -
>  				if (info->end_transaction) {
>  					journal->j_failed_commit =
>  						info->end_transaction;
> @@ -705,42 +702,23 @@ static int do_one_pass(journal_t *journal,
>  					break;
>  				}
>  
> -				if (crc32_sum == found_chksum &&
> -				    cbh->h_chksum_type == JBD2_CRC32_CHKSUM &&
> -				    cbh->h_chksum_size ==
> -						JBD2_CRC32_CHKSUM_SIZE)
> -				       chksum_seen = 1;
> -				else if (!(cbh->h_chksum_type == 0 &&
> -					     cbh->h_chksum_size == 0 &&
> -					     found_chksum == 0 &&
> -					     !chksum_seen))
> -				/*
> -				 * If fs is mounted using an old kernel and then
> -				 * kernel with journal_chksum is used then we
> -				 * get a situation where the journal flag has
> -				 * checksum flag set but checksums are not
> -				 * present i.e chksum = 0, in the individual
> -				 * commit blocks.
> -				 * Hence to avoid checksum failures, in this
> -				 * situation, this extra check is added.
> -				 */
> -						chksum_err = 1;
> -
> -				if (chksum_err) {
> -					info->end_transaction = next_commit_ID;
> -
> -					if (!jbd2_has_feature_async_commit(journal)) {
> -						journal->j_failed_commit =
> -							next_commit_ID;
> -						brelse(bh);
> -						break;
> -					}
> -				}
> +                                /* Neither checksum match nor unused? */
> +				if (!((crc32_sum == found_chksum &&
> +				       cbh->h_chksum_type ==
> +						JBD2_CRC32_CHKSUM &&
> +				       cbh->h_chksum_size ==
> +						JBD2_CRC32_CHKSUM_SIZE) ||
> +				      (cbh->h_chksum_type == 0 &&
> +				       cbh->h_chksum_size == 0 &&
> +				       found_chksum == 0)))
> +					goto chksum_error;
> +
>  				crc32_sum = ~0;
>  			}
>  			if (pass == PASS_SCAN &&
>  			    !jbd2_commit_block_csum_verify(journal,
>  							   bh->b_data)) {
> +			chksum_error:
>  				info->end_transaction = next_commit_ID;
>  
>  				if (!jbd2_has_feature_async_commit(journal)) {
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR