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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 19 Aug 2020 10:02:56 +0800
From:   Shijie Luo <luoshijie1@...wei.com>
To:     "Theodore Y. Ts'o" <tytso@....edu>, Jan Kara <jack@...e.cz>
CC:     <linux-ext4@...r.kernel.org>, <linfeilong@...wei.com>
Subject: Re: [PATCH] jbd2: remove useless variable chksum_seen in do_one_pass

Hi, Ted. Thanks for your reply, I think this one is perfect.

On 2020/8/19 3:14, Theodore Y. Ts'o wrote:
> I wonder if this is even cleaner?  What do folks think?
>
>    	    	    	 - Ted
>
> 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)) {
>
> .
>