From patchwork Tue Oct 13 05:42:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?5bi45Yek5qWg?= X-Patchwork-Id: 1381323 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-ext4-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=hikvision.com Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4C9PpT01R5z9sS8 for ; Tue, 13 Oct 2020 16:52:48 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732372AbgJMFwr (ORCPT ); Tue, 13 Oct 2020 01:52:47 -0400 Received: from mail.hikvision.com ([115.236.50.29]:47170 "EHLO mail.hikvision.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732329AbgJMFwr (ORCPT ); Tue, 13 Oct 2020 01:52:47 -0400 X-Greylist: delayed 628 seconds by postgrey-1.27 at vger.kernel.org; Tue, 13 Oct 2020 01:52:45 EDT Received: from hik-cirblue-mta01.localdomain (unknown [10.1.154.164]) by Forcepoint Email with ESMTP id A61734B27F974DD49CFA; Tue, 13 Oct 2020 13:42:11 +0800 (CST) Received: by hik-cirblue-mta01.localdomain (Postfix, from userid 89) id 9E7D11480042; Tue, 13 Oct 2020 13:41:30 +0800 (CST) Received: from HIK-MBX-CN-02.hikvision.com (HIK-MBX-CN-02.hikvision.com [10.1.7.117]) by hik-cirblue-mta01.localdomain (Postfix) with ESMTP id 9052A1480044; Tue, 13 Oct 2020 13:41:29 +0800 (CST) Received: from HIK-MBX-CN-00.hikvision.com (10.1.7.113) by HIK-MBX-CN-02.hikvision.com (10.1.7.117) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1415.2; Tue, 13 Oct 2020 13:42:07 +0800 Received: from HIK-MBX-CN-00.hikvision.com ([fe80::4cd8:233f:871e:fd8c]) by HIK-MBX-CN-00.hikvision.com ([fe80::4cd8:233f:871e:fd8c%14]) with mapi id 15.01.1415.007; Tue, 13 Oct 2020 13:42:08 +0800 From: =?eucgb2312_cn?b?s6O37+mq?= To: Jan Kara , Ted Tso CC: "linux-ext4@vger.kernel.org" , "adilger@dilger.ca" , changfengnan Subject: =?eucgb2312_cn?b?tPC4tDogW1BBVENIIHY2XSBqYmQyOiBhdm9pZCB0cmFuc2FjdGlvbiBy?= =?eucgb2312_cn?b?ZXVzZSBhZnRlciByZWZvcm1hdHRpbmc=?= Thread-Topic: [PATCH v6] jbd2: avoid transaction reuse after reformatting Thread-Index: AQHWoLeYOJO/9VO2o0Ky2ICarpDLjqmVBKBw Date: Tue, 13 Oct 2020 05:42:07 +0000 Message-ID: References: <20201012164900.20197-1-jack@suse.cz> In-Reply-To: <20201012164900.20197-1-jack@suse.cz> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.1.7.137] MIME-Version: 1.0 X-CG-TRANSID: FBF602DE7EEC403F88CCE315FDA4AE36 Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org This version is good for my test cases. -----邮件原件----- 发件人: Jan Kara 发送时间: 2020年10月13日 0:49 收件人: Ted Tso 抄送: linux-ext4@vger.kernel.org; 常凤楠 ; adilger@dilger.ca; changfengnan ; Jan Kara 主题: [PATCH v6] jbd2: avoid transaction reuse after reformatting From: changfengnan When ext4 is formatted with lazy_journal_init=1 and transactions from the previous filesystem are still on disk, it is possible that they are considered during a recovery after a crash. Because the checksum seed has changed, the CRC check will fail, and the journal recovery fails with checksum error although the journal is otherwise perfectly valid. Fix the problem by checking commit block time stamps to determine whether the data in the journal block is just stale or whether it is indeed corrupt. Reported-by: kernel test robot Reviewed-by: Andreas Dilger Signed-off-by: Fengnan Chang Signed-off-by: Jan Kara --- fs/jbd2/recovery.c | 78 +++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 66 insertions(+), 12 deletions(-) Changes since v5: - rebase on current upstream kernel - reduced some code duplication and indentation level Changes since v4: - fix logic to handle also v2/v3 journal checksum mismatch in the commit block if (descr_csum_size > 0 && !jbd2_descriptor_block_csum_verify(journal, bh->b_data)) { -printk(KERN_ERR "JBD2: Invalid checksum " - "recovering block %lu in log\n", - next_log_block); -err = -EFSBADCRC; -brelse(bh); -goto failed; +/* + * PASS_SCAN can see stale blocks due to lazy + * journal init. Don't error out on those yet. + */ +if (pass != PASS_SCAN) { +pr_err("JBD2: Invalid checksum recovering block %lu in log\n", + next_log_block); +err = -EFSBADCRC; +brelse(bh); +goto failed; +} +need_check_commit_time = true; +jbd_debug(1, +"invalid descriptor block found in %lu\n", +next_log_block); } /* If it is a valid descriptor block, replay it @@ -535,6 +546,7 @@ static int do_one_pass(journal_t *journal, if (pass != PASS_REPLAY) { if (pass == PASS_SCAN && jbd2_has_feature_checksum(journal) && + !need_check_commit_time && !info->end_transaction) { if (calc_chksums(journal, bh, &next_log_block, @@ -683,11 +695,41 @@ static int do_one_pass(journal_t *journal, * mentioned conditions. Hence assume * "Interrupted Commit".) */ +commit_time = be64_to_cpu( +((struct commit_header *)bh->b_data)->h_commit_sec); +/* + * If need_check_commit_time is set, it means we are in + * PASS_SCAN and csum verify failed before. If + * commit_time is increasing, it's the same journal, + * otherwise it is stale journal block, just end this + * recovery. + */ +if (need_check_commit_time) { +if (commit_time >= last_trans_commit_time) { +pr_err("JBD2: Invalid checksum found in transaction %u\n", + next_commit_ID); +err = -EFSBADCRC; +brelse(bh); +goto failed; +} +ignore_crc_mismatch: +/* + * It likely does not belong to same journal, + * just end this recovery with success. + */ +jbd_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n", + next_commit_ID); +err = 0; +brelse(bh); +goto done; +} -/* Found an expected commit block: if checksums - * are present verify them in PASS_SCAN; else not +/* + * Found an expected commit block: if checksums + * are present, verify them in PASS_SCAN; else not * much to do other than move on to the next sequence - * number. */ + * number. + */ if (pass == PASS_SCAN && jbd2_has_feature_checksum(journal)) { struct commit_header *cbh = @@ -719,6 +761,8 @@ static int do_one_pass(journal_t *journal, !jbd2_commit_block_csum_verify(journal, bh->b_data)) { chksum_error: +if (commit_time < last_trans_commit_time) +goto ignore_crc_mismatch; info->end_transaction = next_commit_ID; if (!jbd2_has_feature_async_commit(journal)) { @@ -728,11 +772,24 @@ static int do_one_pass(journal_t *journal, break; } } +if (pass == PASS_SCAN) +last_trans_commit_time = commit_time; brelse(bh); next_commit_ID++; continue; case JBD2_REVOKE_BLOCK: +/* + * Check revoke block crc in pass_scan, if csum verify + * failed, check commit block time later. + */ +if (pass == PASS_SCAN && + !jbd2_descriptor_block_csum_verify(journal, + bh->b_data)) { +jbd_debug(1, "JBD2: invalid revoke block found in %lu\n", + next_log_block); +need_check_commit_time = true; +} /* If we aren't in the REVOKE pass, then we can * just skip over this block. */ if (pass != PASS_REVOKE) { @@ -800,9 +857,6 @@ static int scan_revoke_records(journal_t *journal, struct buffer_head *bh, offset = sizeof(jbd2_journal_revoke_header_t); rcount = be32_to_cpu(header->r_count); -if (!jbd2_descriptor_block_csum_verify(journal, header)) -return -EFSBADCRC; - if (jbd2_journal_has_csum_v2or3(journal)) csum_size = sizeof(struct jbd2_journal_block_tail); if (rcount > journal->j_blocksize - csum_size) -- 2.16.4 diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c index faa97d748474..fb134c7a12c8 100644 --- a/fs/jbd2/recovery.c +++ b/fs/jbd2/recovery.c @@ -428,6 +428,8 @@ static int do_one_pass(journal_t *journal, __u32crc32_sum = ~0; /* Transactional Checksums */ intdescr_csum_size = 0; intblock_error = 0; +boolneed_check_commit_time = false; +__u64last_trans_commit_time = 0, commit_time; /* * First thing is to establish what we expect to find in the log @@ -520,12 +522,21 @@ static int do_one_pass(journal_t *journal,