| Message ID | 20201006103154.7130-1-jack@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] jbd2: avoid transaction reuse after reformatting | expand |
Hi Jan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on ext4/dev]
[also build test WARNING on linus/master v5.9-rc8 next-20201002]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: i386-randconfig-s001-20201005 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-201-g24bdaac6-dirty
# https://github.com/0day-ci/linux/commit/ca606dc0beaec78e8b2b6ec2f112b2192534d9e2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337
git checkout ca606dc0beaec78e8b2b6ec2f112b2192534d9e2
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
echo
echo "sparse warnings: (new ones prefixed by >>)"
echo
>> fs/jbd2/recovery.c:713:41: sparse: sparse: incorrect type in initializer (different base types) @@ expected restricted __be64 [usertype] commit_time @@ got unsigned long long @@
fs/jbd2/recovery.c:713:41: sparse: expected restricted __be64 [usertype] commit_time
>> fs/jbd2/recovery.c:713:41: sparse: got unsigned long long
fs/jbd2/recovery.c:730:45: sparse: sparse: restricted __be64 degrades to integer
fs/jbd2/recovery.c:730:60: sparse: sparse: restricted __be64 degrades to integer
vim +713 fs/jbd2/recovery.c
415
416 static int do_one_pass(journal_t *journal,
417 struct recovery_info *info, enum passtype pass)
418 {
419 unsigned int first_commit_ID, next_commit_ID;
420 unsigned long next_log_block;
421 int err, success = 0;
422 journal_superblock_t * sb;
423 journal_header_t * tmp;
424 struct buffer_head * bh;
425 unsigned int sequence;
426 int blocktype;
427 int tag_bytes = journal_tag_bytes(journal);
428 __u32 crc32_sum = ~0; /* Transactional Checksums */
429 int descr_csum_size = 0;
430 int block_error = 0;
431 bool need_check_commit_time = false;
432 __be64 last_trans_commit_time = 0;
433
434 /*
435 * First thing is to establish what we expect to find in the log
436 * (in terms of transaction IDs), and where (in terms of log
437 * block offsets): query the superblock.
438 */
439
440 sb = journal->j_superblock;
441 next_commit_ID = be32_to_cpu(sb->s_sequence);
442 next_log_block = be32_to_cpu(sb->s_start);
443
444 first_commit_ID = next_commit_ID;
445 if (pass == PASS_SCAN)
446 info->start_transaction = first_commit_ID;
447
448 jbd_debug(1, "Starting recovery pass %d\n", pass);
449
450 /*
451 * Now we walk through the log, transaction by transaction,
452 * making sure that each transaction has a commit block in the
453 * expected place. Each complete transaction gets replayed back
454 * into the main filesystem.
455 */
456
457 while (1) {
458 int flags;
459 char * tagp;
460 journal_block_tag_t * tag;
461 struct buffer_head * obh;
462 struct buffer_head * nbh;
463
464 cond_resched();
465
466 /* If we already know where to stop the log traversal,
467 * check right now that we haven't gone past the end of
468 * the log. */
469
470 if (pass != PASS_SCAN)
471 if (tid_geq(next_commit_ID, info->end_transaction))
472 break;
473
474 jbd_debug(2, "Scanning for sequence ID %u at %lu/%lu\n",
475 next_commit_ID, next_log_block, journal->j_last);
476
477 /* Skip over each chunk of the transaction looking
478 * either the next descriptor block or the final commit
479 * record. */
480
481 jbd_debug(3, "JBD2: checking block %ld\n", next_log_block);
482 err = jread(&bh, journal, next_log_block);
483 if (err)
484 goto failed;
485
486 next_log_block++;
487 wrap(journal, next_log_block);
488
489 /* What kind of buffer is it?
490 *
491 * If it is a descriptor block, check that it has the
492 * expected sequence number. Otherwise, we're all done
493 * here. */
494
495 tmp = (journal_header_t *)bh->b_data;
496
497 if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER)) {
498 brelse(bh);
499 break;
500 }
501
502 blocktype = be32_to_cpu(tmp->h_blocktype);
503 sequence = be32_to_cpu(tmp->h_sequence);
504 jbd_debug(3, "Found magic %d, sequence %d\n",
505 blocktype, sequence);
506
507 if (sequence != next_commit_ID) {
508 brelse(bh);
509 break;
510 }
511
512 /* OK, we have a valid descriptor block which matches
513 * all of the sequence number checks. What are we going
514 * to do with it? That depends on the pass... */
515
516 switch(blocktype) {
517 case JBD2_DESCRIPTOR_BLOCK:
518 /* Verify checksum first */
519 if (jbd2_journal_has_csum_v2or3(journal))
520 descr_csum_size =
521 sizeof(struct jbd2_journal_block_tail);
522 if (descr_csum_size > 0 &&
523 !jbd2_descriptor_block_csum_verify(journal,
524 bh->b_data)) {
525 /*
526 * PASS_SCAN can see stale blocks due to lazy
527 * journal init. Don't error out on those yet.
528 */
529 if (pass != PASS_SCAN) {
530 pr_err("JBD2: Invalid checksum "
531 "recovering block %lu in log\n",
532 next_log_block);
533 err = -EFSBADCRC;
534 brelse(bh);
535 goto failed;
536 }
537 need_check_commit_time = true;
538 jbd_debug(1,
539 "invalid descriptor block found in %lu\n",
540 next_log_block);
541 }
542
543 /* If it is a valid descriptor block, replay it
544 * in pass REPLAY; if journal_checksums enabled, then
545 * calculate checksums in PASS_SCAN, otherwise,
546 * just skip over the blocks it describes. */
547 if (pass != PASS_REPLAY) {
548 if (pass == PASS_SCAN &&
549 jbd2_has_feature_checksum(journal) &&
550 !need_check_commit_time &&
551 !info->end_transaction) {
552 if (calc_chksums(journal, bh,
553 &next_log_block,
554 &crc32_sum)) {
555 put_bh(bh);
556 break;
557 }
558 put_bh(bh);
559 continue;
560 }
561 next_log_block += count_tags(journal, bh);
562 wrap(journal, next_log_block);
563 put_bh(bh);
564 continue;
565 }
566
567 /* A descriptor block: we can now write all of
568 * the data blocks. Yay, useful work is finally
569 * getting done here! */
570
571 tagp = &bh->b_data[sizeof(journal_header_t)];
572 while ((tagp - bh->b_data + tag_bytes)
573 <= journal->j_blocksize - descr_csum_size) {
574 unsigned long io_block;
575
576 tag = (journal_block_tag_t *) tagp;
577 flags = be16_to_cpu(tag->t_flags);
578
579 io_block = next_log_block++;
580 wrap(journal, next_log_block);
581 err = jread(&obh, journal, io_block);
582 if (err) {
583 /* Recover what we can, but
584 * report failure at the end. */
585 success = err;
586 printk(KERN_ERR
587 "JBD2: IO error %d recovering "
588 "block %ld in log\n",
589 err, io_block);
590 } else {
591 unsigned long long blocknr;
592
593 J_ASSERT(obh != NULL);
594 blocknr = read_tag_block(journal,
595 tag);
596
597 /* If the block has been
598 * revoked, then we're all done
599 * here. */
600 if (jbd2_journal_test_revoke
601 (journal, blocknr,
602 next_commit_ID)) {
603 brelse(obh);
604 ++info->nr_revoke_hits;
605 goto skip_write;
606 }
607
608 /* Look for block corruption */
609 if (!jbd2_block_tag_csum_verify(
610 journal, tag, obh->b_data,
611 be32_to_cpu(tmp->h_sequence))) {
612 brelse(obh);
613 success = -EFSBADCRC;
614 printk(KERN_ERR "JBD2: Invalid "
615 "checksum recovering "
616 "data block %llu in "
617 "log\n", blocknr);
618 block_error = 1;
619 goto skip_write;
620 }
621
622 /* Find a buffer for the new
623 * data being restored */
624 nbh = __getblk(journal->j_fs_dev,
625 blocknr,
626 journal->j_blocksize);
627 if (nbh == NULL) {
628 printk(KERN_ERR
629 "JBD2: Out of memory "
630 "during recovery.\n");
631 err = -ENOMEM;
632 brelse(bh);
633 brelse(obh);
634 goto failed;
635 }
636
637 lock_buffer(nbh);
638 memcpy(nbh->b_data, obh->b_data,
639 journal->j_blocksize);
640 if (flags & JBD2_FLAG_ESCAPE) {
641 *((__be32 *)nbh->b_data) =
642 cpu_to_be32(JBD2_MAGIC_NUMBER);
643 }
644
645 BUFFER_TRACE(nbh, "marking dirty");
646 set_buffer_uptodate(nbh);
647 mark_buffer_dirty(nbh);
648 BUFFER_TRACE(nbh, "marking uptodate");
649 ++info->nr_replays;
650 /* ll_rw_block(WRITE, 1, &nbh); */
651 unlock_buffer(nbh);
652 brelse(obh);
653 brelse(nbh);
654 }
655
656 skip_write:
657 tagp += tag_bytes;
658 if (!(flags & JBD2_FLAG_SAME_UUID))
659 tagp += 16;
660
661 if (flags & JBD2_FLAG_LAST_TAG)
662 break;
663 }
664
665 brelse(bh);
666 continue;
667
668 case JBD2_COMMIT_BLOCK:
669 /* How to differentiate between interrupted commit
670 * and journal corruption ?
671 *
672 * {nth transaction}
673 * Checksum Verification Failed
674 * |
675 * ____________________
676 * | |
677 * async_commit sync_commit
678 * | |
679 * | GO TO NEXT "Journal Corruption"
680 * | TRANSACTION
681 * |
682 * {(n+1)th transanction}
683 * |
684 * _______|______________
685 * | |
686 * Commit block found Commit block not found
687 * | |
688 * "Journal Corruption" |
689 * _____________|_________
690 * | |
691 * nth trans corrupt OR nth trans
692 * and (n+1)th interrupted interrupted
693 * before commit block
694 * could reach the disk.
695 * (Cannot find the difference in above
696 * mentioned conditions. Hence assume
697 * "Interrupted Commit".)
698 */
699
700 /*
701 * Found an expected commit block: if checksums
702 * are present, verify them in PASS_SCAN; else not
703 * much to do other than move on to the next sequence
704 * number.
705 */
706 if (pass == PASS_SCAN &&
707 jbd2_has_feature_checksum(journal)) {
708 struct commit_header *cbh =
709 (struct commit_header *)bh->b_data;
710 unsigned found_chksum =
711 be32_to_cpu(cbh->h_chksum[0]);
712 __be64 commit_time =
> 713 be64_to_cpu(cbh->h_commit_sec);
714
715 if (info->end_transaction) {
716 journal->j_failed_commit =
717 info->end_transaction;
718 brelse(bh);
719 break;
720 }
721
722 /*
723 * If need_check_commit_time is set, it means
724 * csum verify failed before, if commit_time is
725 * increasing, it's same journal, otherwise it
726 * is stale journal block, just end this
727 * recovery.
728 */
729 if (need_check_commit_time) {
730 if (commit_time >= last_trans_commit_time) {
731 pr_err("JBD2: Invalid checksum found in transaction %u\n",
732 next_commit_ID);
733 err = -EFSBADCRC;
734 brelse(bh);
735 goto failed;
736 }
737 /*
738 * It likely does not belong to same
739 * journal, just end this recovery with
740 * success.
741 */
742 jbd_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n",
743 next_commit_ID);
744 err = 0;
745 brelse(bh);
746 goto done;
747 }
748
749 /* Neither checksum match nor unused? */
750 if (!((crc32_sum == found_chksum &&
751 cbh->h_chksum_type ==
752 JBD2_CRC32_CHKSUM &&
753 cbh->h_chksum_size ==
754 JBD2_CRC32_CHKSUM_SIZE) ||
755 (cbh->h_chksum_type == 0 &&
756 cbh->h_chksum_size == 0 &&
757 found_chksum == 0)))
758 goto chksum_error;
759
760 crc32_sum = ~0;
761 last_trans_commit_time = commit_time;
762 }
763 if (pass == PASS_SCAN &&
764 !jbd2_commit_block_csum_verify(journal,
765 bh->b_data)) {
766 chksum_error:
767 info->end_transaction = next_commit_ID;
768
769 if (!jbd2_has_feature_async_commit(journal)) {
770 journal->j_failed_commit =
771 next_commit_ID;
772 brelse(bh);
773 break;
774 }
775 }
776 brelse(bh);
777 next_commit_ID++;
778 continue;
779
780 case JBD2_REVOKE_BLOCK:
781 /*
782 * Check revoke block crc in pass_scan, if csum verify
783 * failed, check commit block time later.
784 */
785 if (pass == PASS_SCAN &&
786 !jbd2_descriptor_block_csum_verify(journal,
787 bh->b_data)) {
788 jbd_debug(1, "JBD2: invalid revoke block found in %lu\n",
789 next_log_block);
790 need_check_commit_time = true;
791 }
792 /* If we aren't in the REVOKE pass, then we can
793 * just skip over this block. */
794 if (pass != PASS_REVOKE) {
795 brelse(bh);
796 continue;
797 }
798
799 err = scan_revoke_records(journal, bh,
800 next_commit_ID, info);
801 brelse(bh);
802 if (err)
803 goto failed;
804 continue;
805
806 default:
807 jbd_debug(3, "Unrecognised magic %d, end of scan.\n",
808 blocktype);
809 brelse(bh);
810 goto done;
811 }
812 }
813
814 done:
815 /*
816 * We broke out of the log scan loop: either we came to the
817 * known end of the log or we found an unexpected block in the
818 * log. If the latter happened, then we know that the "current"
819 * transaction marks the end of the valid log.
820 */
821
822 if (pass == PASS_SCAN) {
823 if (!info->end_transaction)
824 info->end_transaction = next_commit_ID;
825 } else {
826 /* It's really bad news if different passes end up at
827 * different places (but possible due to IO errors). */
828 if (info->end_transaction != next_commit_ID) {
829 printk(KERN_ERR "JBD2: recovery pass %d ended at "
830 "transaction %u, expected %u\n",
831 pass, next_commit_ID, info->end_transaction);
832 if (!success)
833 success = -EIO;
834 }
835 }
836 if (block_error && success == 0)
837 success = -EIO;
838 return success;
839
840 failed:
841 return err;
842 }
843
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Jan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on ext4/dev]
[also build test WARNING on linus/master v5.9-rc8 next-20201002]
[cannot apply to tytso-fscrypt/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: s390-randconfig-s032-20201005 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-201-g24bdaac6-dirty
# https://github.com/0day-ci/linux/commit/ca606dc0beaec78e8b2b6ec2f112b2192534d9e2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337
git checkout ca606dc0beaec78e8b2b6ec2f112b2192534d9e2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
echo
echo "sparse warnings: (new ones prefixed by >>)"
echo
>> fs/jbd2/recovery.c:713:41: sparse: sparse: incorrect type in initializer (different base types) @@ expected restricted __be64 [usertype] commit_time @@ got unsigned long long [usertype] @@
>> fs/jbd2/recovery.c:713:41: sparse: expected restricted __be64 [usertype] commit_time
>> fs/jbd2/recovery.c:713:41: sparse: got unsigned long long [usertype]
>> fs/jbd2/recovery.c:730:45: sparse: sparse: restricted __be64 degrades to integer
fs/jbd2/recovery.c:730:60: sparse: sparse: restricted __be64 degrades to integer
vim +713 fs/jbd2/recovery.c
415
416 static int do_one_pass(journal_t *journal,
417 struct recovery_info *info, enum passtype pass)
418 {
419 unsigned int first_commit_ID, next_commit_ID;
420 unsigned long next_log_block;
421 int err, success = 0;
422 journal_superblock_t * sb;
423 journal_header_t * tmp;
424 struct buffer_head * bh;
425 unsigned int sequence;
426 int blocktype;
427 int tag_bytes = journal_tag_bytes(journal);
428 __u32 crc32_sum = ~0; /* Transactional Checksums */
429 int descr_csum_size = 0;
430 int block_error = 0;
431 bool need_check_commit_time = false;
432 __be64 last_trans_commit_time = 0;
433
434 /*
435 * First thing is to establish what we expect to find in the log
436 * (in terms of transaction IDs), and where (in terms of log
437 * block offsets): query the superblock.
438 */
439
440 sb = journal->j_superblock;
441 next_commit_ID = be32_to_cpu(sb->s_sequence);
442 next_log_block = be32_to_cpu(sb->s_start);
443
444 first_commit_ID = next_commit_ID;
445 if (pass == PASS_SCAN)
446 info->start_transaction = first_commit_ID;
447
448 jbd_debug(1, "Starting recovery pass %d\n", pass);
449
450 /*
451 * Now we walk through the log, transaction by transaction,
452 * making sure that each transaction has a commit block in the
453 * expected place. Each complete transaction gets replayed back
454 * into the main filesystem.
455 */
456
457 while (1) {
458 int flags;
459 char * tagp;
460 journal_block_tag_t * tag;
461 struct buffer_head * obh;
462 struct buffer_head * nbh;
463
464 cond_resched();
465
466 /* If we already know where to stop the log traversal,
467 * check right now that we haven't gone past the end of
468 * the log. */
469
470 if (pass != PASS_SCAN)
471 if (tid_geq(next_commit_ID, info->end_transaction))
472 break;
473
474 jbd_debug(2, "Scanning for sequence ID %u at %lu/%lu\n",
475 next_commit_ID, next_log_block, journal->j_last);
476
477 /* Skip over each chunk of the transaction looking
478 * either the next descriptor block or the final commit
479 * record. */
480
481 jbd_debug(3, "JBD2: checking block %ld\n", next_log_block);
482 err = jread(&bh, journal, next_log_block);
483 if (err)
484 goto failed;
485
486 next_log_block++;
487 wrap(journal, next_log_block);
488
489 /* What kind of buffer is it?
490 *
491 * If it is a descriptor block, check that it has the
492 * expected sequence number. Otherwise, we're all done
493 * here. */
494
495 tmp = (journal_header_t *)bh->b_data;
496
497 if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER)) {
498 brelse(bh);
499 break;
500 }
501
502 blocktype = be32_to_cpu(tmp->h_blocktype);
503 sequence = be32_to_cpu(tmp->h_sequence);
504 jbd_debug(3, "Found magic %d, sequence %d\n",
505 blocktype, sequence);
506
507 if (sequence != next_commit_ID) {
508 brelse(bh);
509 break;
510 }
511
512 /* OK, we have a valid descriptor block which matches
513 * all of the sequence number checks. What are we going
514 * to do with it? That depends on the pass... */
515
516 switch(blocktype) {
517 case JBD2_DESCRIPTOR_BLOCK:
518 /* Verify checksum first */
519 if (jbd2_journal_has_csum_v2or3(journal))
520 descr_csum_size =
521 sizeof(struct jbd2_journal_block_tail);
522 if (descr_csum_size > 0 &&
523 !jbd2_descriptor_block_csum_verify(journal,
524 bh->b_data)) {
525 /*
526 * PASS_SCAN can see stale blocks due to lazy
527 * journal init. Don't error out on those yet.
528 */
529 if (pass != PASS_SCAN) {
530 pr_err("JBD2: Invalid checksum "
531 "recovering block %lu in log\n",
532 next_log_block);
533 err = -EFSBADCRC;
534 brelse(bh);
535 goto failed;
536 }
537 need_check_commit_time = true;
538 jbd_debug(1,
539 "invalid descriptor block found in %lu\n",
540 next_log_block);
541 }
542
543 /* If it is a valid descriptor block, replay it
544 * in pass REPLAY; if journal_checksums enabled, then
545 * calculate checksums in PASS_SCAN, otherwise,
546 * just skip over the blocks it describes. */
547 if (pass != PASS_REPLAY) {
548 if (pass == PASS_SCAN &&
549 jbd2_has_feature_checksum(journal) &&
550 !need_check_commit_time &&
551 !info->end_transaction) {
552 if (calc_chksums(journal, bh,
553 &next_log_block,
554 &crc32_sum)) {
555 put_bh(bh);
556 break;
557 }
558 put_bh(bh);
559 continue;
560 }
561 next_log_block += count_tags(journal, bh);
562 wrap(journal, next_log_block);
563 put_bh(bh);
564 continue;
565 }
566
567 /* A descriptor block: we can now write all of
568 * the data blocks. Yay, useful work is finally
569 * getting done here! */
570
571 tagp = &bh->b_data[sizeof(journal_header_t)];
572 while ((tagp - bh->b_data + tag_bytes)
573 <= journal->j_blocksize - descr_csum_size) {
574 unsigned long io_block;
575
576 tag = (journal_block_tag_t *) tagp;
577 flags = be16_to_cpu(tag->t_flags);
578
579 io_block = next_log_block++;
580 wrap(journal, next_log_block);
581 err = jread(&obh, journal, io_block);
582 if (err) {
583 /* Recover what we can, but
584 * report failure at the end. */
585 success = err;
586 printk(KERN_ERR
587 "JBD2: IO error %d recovering "
588 "block %ld in log\n",
589 err, io_block);
590 } else {
591 unsigned long long blocknr;
592
593 J_ASSERT(obh != NULL);
594 blocknr = read_tag_block(journal,
595 tag);
596
597 /* If the block has been
598 * revoked, then we're all done
599 * here. */
600 if (jbd2_journal_test_revoke
601 (journal, blocknr,
602 next_commit_ID)) {
603 brelse(obh);
604 ++info->nr_revoke_hits;
605 goto skip_write;
606 }
607
608 /* Look for block corruption */
609 if (!jbd2_block_tag_csum_verify(
610 journal, tag, obh->b_data,
611 be32_to_cpu(tmp->h_sequence))) {
612 brelse(obh);
613 success = -EFSBADCRC;
614 printk(KERN_ERR "JBD2: Invalid "
615 "checksum recovering "
616 "data block %llu in "
617 "log\n", blocknr);
618 block_error = 1;
619 goto skip_write;
620 }
621
622 /* Find a buffer for the new
623 * data being restored */
624 nbh = __getblk(journal->j_fs_dev,
625 blocknr,
626 journal->j_blocksize);
627 if (nbh == NULL) {
628 printk(KERN_ERR
629 "JBD2: Out of memory "
630 "during recovery.\n");
631 err = -ENOMEM;
632 brelse(bh);
633 brelse(obh);
634 goto failed;
635 }
636
637 lock_buffer(nbh);
638 memcpy(nbh->b_data, obh->b_data,
639 journal->j_blocksize);
640 if (flags & JBD2_FLAG_ESCAPE) {
641 *((__be32 *)nbh->b_data) =
642 cpu_to_be32(JBD2_MAGIC_NUMBER);
643 }
644
645 BUFFER_TRACE(nbh, "marking dirty");
646 set_buffer_uptodate(nbh);
647 mark_buffer_dirty(nbh);
648 BUFFER_TRACE(nbh, "marking uptodate");
649 ++info->nr_replays;
650 /* ll_rw_block(WRITE, 1, &nbh); */
651 unlock_buffer(nbh);
652 brelse(obh);
653 brelse(nbh);
654 }
655
656 skip_write:
657 tagp += tag_bytes;
658 if (!(flags & JBD2_FLAG_SAME_UUID))
659 tagp += 16;
660
661 if (flags & JBD2_FLAG_LAST_TAG)
662 break;
663 }
664
665 brelse(bh);
666 continue;
667
668 case JBD2_COMMIT_BLOCK:
669 /* How to differentiate between interrupted commit
670 * and journal corruption ?
671 *
672 * {nth transaction}
673 * Checksum Verification Failed
674 * |
675 * ____________________
676 * | |
677 * async_commit sync_commit
678 * | |
679 * | GO TO NEXT "Journal Corruption"
680 * | TRANSACTION
681 * |
682 * {(n+1)th transanction}
683 * |
684 * _______|______________
685 * | |
686 * Commit block found Commit block not found
687 * | |
688 * "Journal Corruption" |
689 * _____________|_________
690 * | |
691 * nth trans corrupt OR nth trans
692 * and (n+1)th interrupted interrupted
693 * before commit block
694 * could reach the disk.
695 * (Cannot find the difference in above
696 * mentioned conditions. Hence assume
697 * "Interrupted Commit".)
698 */
699
700 /*
701 * Found an expected commit block: if checksums
702 * are present, verify them in PASS_SCAN; else not
703 * much to do other than move on to the next sequence
704 * number.
705 */
706 if (pass == PASS_SCAN &&
707 jbd2_has_feature_checksum(journal)) {
708 struct commit_header *cbh =
709 (struct commit_header *)bh->b_data;
710 unsigned found_chksum =
711 be32_to_cpu(cbh->h_chksum[0]);
712 __be64 commit_time =
> 713 be64_to_cpu(cbh->h_commit_sec);
714
715 if (info->end_transaction) {
716 journal->j_failed_commit =
717 info->end_transaction;
718 brelse(bh);
719 break;
720 }
721
722 /*
723 * If need_check_commit_time is set, it means
724 * csum verify failed before, if commit_time is
725 * increasing, it's same journal, otherwise it
726 * is stale journal block, just end this
727 * recovery.
728 */
729 if (need_check_commit_time) {
> 730 if (commit_time >= last_trans_commit_time) {
731 pr_err("JBD2: Invalid checksum found in transaction %u\n",
732 next_commit_ID);
733 err = -EFSBADCRC;
734 brelse(bh);
735 goto failed;
736 }
737 /*
738 * It likely does not belong to same
739 * journal, just end this recovery with
740 * success.
741 */
742 jbd_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n",
743 next_commit_ID);
744 err = 0;
745 brelse(bh);
746 goto done;
747 }
748
749 /* Neither checksum match nor unused? */
750 if (!((crc32_sum == found_chksum &&
751 cbh->h_chksum_type ==
752 JBD2_CRC32_CHKSUM &&
753 cbh->h_chksum_size ==
754 JBD2_CRC32_CHKSUM_SIZE) ||
755 (cbh->h_chksum_type == 0 &&
756 cbh->h_chksum_size == 0 &&
757 found_chksum == 0)))
758 goto chksum_error;
759
760 crc32_sum = ~0;
761 last_trans_commit_time = commit_time;
762 }
763 if (pass == PASS_SCAN &&
764 !jbd2_commit_block_csum_verify(journal,
765 bh->b_data)) {
766 chksum_error:
767 info->end_transaction = next_commit_ID;
768
769 if (!jbd2_has_feature_async_commit(journal)) {
770 journal->j_failed_commit =
771 next_commit_ID;
772 brelse(bh);
773 break;
774 }
775 }
776 brelse(bh);
777 next_commit_ID++;
778 continue;
779
780 case JBD2_REVOKE_BLOCK:
781 /*
782 * Check revoke block crc in pass_scan, if csum verify
783 * failed, check commit block time later.
784 */
785 if (pass == PASS_SCAN &&
786 !jbd2_descriptor_block_csum_verify(journal,
787 bh->b_data)) {
788 jbd_debug(1, "JBD2: invalid revoke block found in %lu\n",
789 next_log_block);
790 need_check_commit_time = true;
791 }
792 /* If we aren't in the REVOKE pass, then we can
793 * just skip over this block. */
794 if (pass != PASS_REVOKE) {
795 brelse(bh);
796 continue;
797 }
798
799 err = scan_revoke_records(journal, bh,
800 next_commit_ID, info);
801 brelse(bh);
802 if (err)
803 goto failed;
804 continue;
805
806 default:
807 jbd_debug(3, "Unrecognised magic %d, end of scan.\n",
808 blocktype);
809 brelse(bh);
810 goto done;
811 }
812 }
813
814 done:
815 /*
816 * We broke out of the log scan loop: either we came to the
817 * known end of the log or we found an unexpected block in the
818 * log. If the latter happened, then we know that the "current"
819 * transaction marks the end of the valid log.
820 */
821
822 if (pass == PASS_SCAN) {
823 if (!info->end_transaction)
824 info->end_transaction = next_commit_ID;
825 } else {
826 /* It's really bad news if different passes end up at
827 * different places (but possible due to IO errors). */
828 if (info->end_transaction != next_commit_ID) {
829 printk(KERN_ERR "JBD2: recovery pass %d ended at "
830 "transaction %u, expected %u\n",
831 pass, next_commit_ID, info->end_transaction);
832 if (!success)
833 success = -EIO;
834 }
835 }
836 if (block_error && success == 0)
837 success = -EIO;
838 return success;
839
840 failed:
841 return err;
842 }
843
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Tue 06-10-20 21:06:06, kernel test robot wrote: > Hi Jan, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on ext4/dev] > [also build test WARNING on linus/master v5.9-rc8 next-20201002] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] Thanks for report! I'll send v4 with sparse warning fixed up (it isn't a functional problem but it is good to have it fixed). Honza > > url: https://github.com/0day-ci/linux/commits/Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337 > base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev > config: i386-randconfig-s001-20201005 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > reproduce: > # apt-get install sparse > # sparse version: v0.6.2-201-g24bdaac6-dirty > # https://github.com/0day-ci/linux/commit/ca606dc0beaec78e8b2b6ec2f112b2192534d9e2 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337 > git checkout ca606dc0beaec78e8b2b6ec2f112b2192534d9e2 > # save the attached .config to linux build tree > make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > echo > echo "sparse warnings: (new ones prefixed by >>)" > echo > >> fs/jbd2/recovery.c:713:41: sparse: sparse: incorrect type in initializer (different base types) @@ expected restricted __be64 [usertype] commit_time @@ got unsigned long long @@ > fs/jbd2/recovery.c:713:41: sparse: expected restricted __be64 [usertype] commit_time > >> fs/jbd2/recovery.c:713:41: sparse: got unsigned long long > fs/jbd2/recovery.c:730:45: sparse: sparse: restricted __be64 degrades to integer > fs/jbd2/recovery.c:730:60: sparse: sparse: restricted __be64 degrades to integer > > vim +713 fs/jbd2/recovery.c > > 415 > 416 static int do_one_pass(journal_t *journal, > 417 struct recovery_info *info, enum passtype pass) > 418 { > 419 unsigned int first_commit_ID, next_commit_ID; > 420 unsigned long next_log_block; > 421 int err, success = 0; > 422 journal_superblock_t * sb; > 423 journal_header_t * tmp; > 424 struct buffer_head * bh; > 425 unsigned int sequence; > 426 int blocktype; > 427 int tag_bytes = journal_tag_bytes(journal); > 428 __u32 crc32_sum = ~0; /* Transactional Checksums */ > 429 int descr_csum_size = 0; > 430 int block_error = 0; > 431 bool need_check_commit_time = false; > 432 __be64 last_trans_commit_time = 0; > 433 > 434 /* > 435 * First thing is to establish what we expect to find in the log > 436 * (in terms of transaction IDs), and where (in terms of log > 437 * block offsets): query the superblock. > 438 */ > 439 > 440 sb = journal->j_superblock; > 441 next_commit_ID = be32_to_cpu(sb->s_sequence); > 442 next_log_block = be32_to_cpu(sb->s_start); > 443 > 444 first_commit_ID = next_commit_ID; > 445 if (pass == PASS_SCAN) > 446 info->start_transaction = first_commit_ID; > 447 > 448 jbd_debug(1, "Starting recovery pass %d\n", pass); > 449 > 450 /* > 451 * Now we walk through the log, transaction by transaction, > 452 * making sure that each transaction has a commit block in the > 453 * expected place. Each complete transaction gets replayed back > 454 * into the main filesystem. > 455 */ > 456 > 457 while (1) { > 458 int flags; > 459 char * tagp; > 460 journal_block_tag_t * tag; > 461 struct buffer_head * obh; > 462 struct buffer_head * nbh; > 463 > 464 cond_resched(); > 465 > 466 /* If we already know where to stop the log traversal, > 467 * check right now that we haven't gone past the end of > 468 * the log. */ > 469 > 470 if (pass != PASS_SCAN) > 471 if (tid_geq(next_commit_ID, info->end_transaction)) > 472 break; > 473 > 474 jbd_debug(2, "Scanning for sequence ID %u at %lu/%lu\n", > 475 next_commit_ID, next_log_block, journal->j_last); > 476 > 477 /* Skip over each chunk of the transaction looking > 478 * either the next descriptor block or the final commit > 479 * record. */ > 480 > 481 jbd_debug(3, "JBD2: checking block %ld\n", next_log_block); > 482 err = jread(&bh, journal, next_log_block); > 483 if (err) > 484 goto failed; > 485 > 486 next_log_block++; > 487 wrap(journal, next_log_block); > 488 > 489 /* What kind of buffer is it? > 490 * > 491 * If it is a descriptor block, check that it has the > 492 * expected sequence number. Otherwise, we're all done > 493 * here. */ > 494 > 495 tmp = (journal_header_t *)bh->b_data; > 496 > 497 if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER)) { > 498 brelse(bh); > 499 break; > 500 } > 501 > 502 blocktype = be32_to_cpu(tmp->h_blocktype); > 503 sequence = be32_to_cpu(tmp->h_sequence); > 504 jbd_debug(3, "Found magic %d, sequence %d\n", > 505 blocktype, sequence); > 506 > 507 if (sequence != next_commit_ID) { > 508 brelse(bh); > 509 break; > 510 } > 511 > 512 /* OK, we have a valid descriptor block which matches > 513 * all of the sequence number checks. What are we going > 514 * to do with it? That depends on the pass... */ > 515 > 516 switch(blocktype) { > 517 case JBD2_DESCRIPTOR_BLOCK: > 518 /* Verify checksum first */ > 519 if (jbd2_journal_has_csum_v2or3(journal)) > 520 descr_csum_size = > 521 sizeof(struct jbd2_journal_block_tail); > 522 if (descr_csum_size > 0 && > 523 !jbd2_descriptor_block_csum_verify(journal, > 524 bh->b_data)) { > 525 /* > 526 * PASS_SCAN can see stale blocks due to lazy > 527 * journal init. Don't error out on those yet. > 528 */ > 529 if (pass != PASS_SCAN) { > 530 pr_err("JBD2: Invalid checksum " > 531 "recovering block %lu in log\n", > 532 next_log_block); > 533 err = -EFSBADCRC; > 534 brelse(bh); > 535 goto failed; > 536 } > 537 need_check_commit_time = true; > 538 jbd_debug(1, > 539 "invalid descriptor block found in %lu\n", > 540 next_log_block); > 541 } > 542 > 543 /* If it is a valid descriptor block, replay it > 544 * in pass REPLAY; if journal_checksums enabled, then > 545 * calculate checksums in PASS_SCAN, otherwise, > 546 * just skip over the blocks it describes. */ > 547 if (pass != PASS_REPLAY) { > 548 if (pass == PASS_SCAN && > 549 jbd2_has_feature_checksum(journal) && > 550 !need_check_commit_time && > 551 !info->end_transaction) { > 552 if (calc_chksums(journal, bh, > 553 &next_log_block, > 554 &crc32_sum)) { > 555 put_bh(bh); > 556 break; > 557 } > 558 put_bh(bh); > 559 continue; > 560 } > 561 next_log_block += count_tags(journal, bh); > 562 wrap(journal, next_log_block); > 563 put_bh(bh); > 564 continue; > 565 } > 566 > 567 /* A descriptor block: we can now write all of > 568 * the data blocks. Yay, useful work is finally > 569 * getting done here! */ > 570 > 571 tagp = &bh->b_data[sizeof(journal_header_t)]; > 572 while ((tagp - bh->b_data + tag_bytes) > 573 <= journal->j_blocksize - descr_csum_size) { > 574 unsigned long io_block; > 575 > 576 tag = (journal_block_tag_t *) tagp; > 577 flags = be16_to_cpu(tag->t_flags); > 578 > 579 io_block = next_log_block++; > 580 wrap(journal, next_log_block); > 581 err = jread(&obh, journal, io_block); > 582 if (err) { > 583 /* Recover what we can, but > 584 * report failure at the end. */ > 585 success = err; > 586 printk(KERN_ERR > 587 "JBD2: IO error %d recovering " > 588 "block %ld in log\n", > 589 err, io_block); > 590 } else { > 591 unsigned long long blocknr; > 592 > 593 J_ASSERT(obh != NULL); > 594 blocknr = read_tag_block(journal, > 595 tag); > 596 > 597 /* If the block has been > 598 * revoked, then we're all done > 599 * here. */ > 600 if (jbd2_journal_test_revoke > 601 (journal, blocknr, > 602 next_commit_ID)) { > 603 brelse(obh); > 604 ++info->nr_revoke_hits; > 605 goto skip_write; > 606 } > 607 > 608 /* Look for block corruption */ > 609 if (!jbd2_block_tag_csum_verify( > 610 journal, tag, obh->b_data, > 611 be32_to_cpu(tmp->h_sequence))) { > 612 brelse(obh); > 613 success = -EFSBADCRC; > 614 printk(KERN_ERR "JBD2: Invalid " > 615 "checksum recovering " > 616 "data block %llu in " > 617 "log\n", blocknr); > 618 block_error = 1; > 619 goto skip_write; > 620 } > 621 > 622 /* Find a buffer for the new > 623 * data being restored */ > 624 nbh = __getblk(journal->j_fs_dev, > 625 blocknr, > 626 journal->j_blocksize); > 627 if (nbh == NULL) { > 628 printk(KERN_ERR > 629 "JBD2: Out of memory " > 630 "during recovery.\n"); > 631 err = -ENOMEM; > 632 brelse(bh); > 633 brelse(obh); > 634 goto failed; > 635 } > 636 > 637 lock_buffer(nbh); > 638 memcpy(nbh->b_data, obh->b_data, > 639 journal->j_blocksize); > 640 if (flags & JBD2_FLAG_ESCAPE) { > 641 *((__be32 *)nbh->b_data) = > 642 cpu_to_be32(JBD2_MAGIC_NUMBER); > 643 } > 644 > 645 BUFFER_TRACE(nbh, "marking dirty"); > 646 set_buffer_uptodate(nbh); > 647 mark_buffer_dirty(nbh); > 648 BUFFER_TRACE(nbh, "marking uptodate"); > 649 ++info->nr_replays; > 650 /* ll_rw_block(WRITE, 1, &nbh); */ > 651 unlock_buffer(nbh); > 652 brelse(obh); > 653 brelse(nbh); > 654 } > 655 > 656 skip_write: > 657 tagp += tag_bytes; > 658 if (!(flags & JBD2_FLAG_SAME_UUID)) > 659 tagp += 16; > 660 > 661 if (flags & JBD2_FLAG_LAST_TAG) > 662 break; > 663 } > 664 > 665 brelse(bh); > 666 continue; > 667 > 668 case JBD2_COMMIT_BLOCK: > 669 /* How to differentiate between interrupted commit > 670 * and journal corruption ? > 671 * > 672 * {nth transaction} > 673 * Checksum Verification Failed > 674 * | > 675 * ____________________ > 676 * | | > 677 * async_commit sync_commit > 678 * | | > 679 * | GO TO NEXT "Journal Corruption" > 680 * | TRANSACTION > 681 * | > 682 * {(n+1)th transanction} > 683 * | > 684 * _______|______________ > 685 * | | > 686 * Commit block found Commit block not found > 687 * | | > 688 * "Journal Corruption" | > 689 * _____________|_________ > 690 * | | > 691 * nth trans corrupt OR nth trans > 692 * and (n+1)th interrupted interrupted > 693 * before commit block > 694 * could reach the disk. > 695 * (Cannot find the difference in above > 696 * mentioned conditions. Hence assume > 697 * "Interrupted Commit".) > 698 */ > 699 > 700 /* > 701 * Found an expected commit block: if checksums > 702 * are present, verify them in PASS_SCAN; else not > 703 * much to do other than move on to the next sequence > 704 * number. > 705 */ > 706 if (pass == PASS_SCAN && > 707 jbd2_has_feature_checksum(journal)) { > 708 struct commit_header *cbh = > 709 (struct commit_header *)bh->b_data; > 710 unsigned found_chksum = > 711 be32_to_cpu(cbh->h_chksum[0]); > 712 __be64 commit_time = > > 713 be64_to_cpu(cbh->h_commit_sec); > 714 > 715 if (info->end_transaction) { > 716 journal->j_failed_commit = > 717 info->end_transaction; > 718 brelse(bh); > 719 break; > 720 } > 721 > 722 /* > 723 * If need_check_commit_time is set, it means > 724 * csum verify failed before, if commit_time is > 725 * increasing, it's same journal, otherwise it > 726 * is stale journal block, just end this > 727 * recovery. > 728 */ > 729 if (need_check_commit_time) { > 730 if (commit_time >= last_trans_commit_time) { > 731 pr_err("JBD2: Invalid checksum found in transaction %u\n", > 732 next_commit_ID); > 733 err = -EFSBADCRC; > 734 brelse(bh); > 735 goto failed; > 736 } > 737 /* > 738 * It likely does not belong to same > 739 * journal, just end this recovery with > 740 * success. > 741 */ > 742 jbd_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n", > 743 next_commit_ID); > 744 err = 0; > 745 brelse(bh); > 746 goto done; > 747 } > 748 > 749 /* Neither checksum match nor unused? */ > 750 if (!((crc32_sum == found_chksum && > 751 cbh->h_chksum_type == > 752 JBD2_CRC32_CHKSUM && > 753 cbh->h_chksum_size == > 754 JBD2_CRC32_CHKSUM_SIZE) || > 755 (cbh->h_chksum_type == 0 && > 756 cbh->h_chksum_size == 0 && > 757 found_chksum == 0))) > 758 goto chksum_error; > 759 > 760 crc32_sum = ~0; > 761 last_trans_commit_time = commit_time; > 762 } > 763 if (pass == PASS_SCAN && > 764 !jbd2_commit_block_csum_verify(journal, > 765 bh->b_data)) { > 766 chksum_error: > 767 info->end_transaction = next_commit_ID; > 768 > 769 if (!jbd2_has_feature_async_commit(journal)) { > 770 journal->j_failed_commit = > 771 next_commit_ID; > 772 brelse(bh); > 773 break; > 774 } > 775 } > 776 brelse(bh); > 777 next_commit_ID++; > 778 continue; > 779 > 780 case JBD2_REVOKE_BLOCK: > 781 /* > 782 * Check revoke block crc in pass_scan, if csum verify > 783 * failed, check commit block time later. > 784 */ > 785 if (pass == PASS_SCAN && > 786 !jbd2_descriptor_block_csum_verify(journal, > 787 bh->b_data)) { > 788 jbd_debug(1, "JBD2: invalid revoke block found in %lu\n", > 789 next_log_block); > 790 need_check_commit_time = true; > 791 } > 792 /* If we aren't in the REVOKE pass, then we can > 793 * just skip over this block. */ > 794 if (pass != PASS_REVOKE) { > 795 brelse(bh); > 796 continue; > 797 } > 798 > 799 err = scan_revoke_records(journal, bh, > 800 next_commit_ID, info); > 801 brelse(bh); > 802 if (err) > 803 goto failed; > 804 continue; > 805 > 806 default: > 807 jbd_debug(3, "Unrecognised magic %d, end of scan.\n", > 808 blocktype); > 809 brelse(bh); > 810 goto done; > 811 } > 812 } > 813 > 814 done: > 815 /* > 816 * We broke out of the log scan loop: either we came to the > 817 * known end of the log or we found an unexpected block in the > 818 * log. If the latter happened, then we know that the "current" > 819 * transaction marks the end of the valid log. > 820 */ > 821 > 822 if (pass == PASS_SCAN) { > 823 if (!info->end_transaction) > 824 info->end_transaction = next_commit_ID; > 825 } else { > 826 /* It's really bad news if different passes end up at > 827 * different places (but possible due to IO errors). */ > 828 if (info->end_transaction != next_commit_ID) { > 829 printk(KERN_ERR "JBD2: recovery pass %d ended at " > 830 "transaction %u, expected %u\n", > 831 pass, next_commit_ID, info->end_transaction); > 832 if (!success) > 833 success = -EIO; > 834 } > 835 } > 836 if (block_error && success == 0) > 837 success = -EIO; > 838 return success; > 839 > 840 failed: > 841 return err; > 842 } > 843 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c index faa97d748474..3f352a20c122 100644 --- a/fs/jbd2/recovery.c +++ b/fs/jbd2/recovery.c @@ -428,6 +428,8 @@ static int do_one_pass(journal_t *journal, __u32 crc32_sum = ~0; /* Transactional Checksums */ int descr_csum_size = 0; int block_error = 0; + bool need_check_commit_time = false; + __be64 last_trans_commit_time = 0; /* * First thing is to establish what we expect to find in the log @@ -520,12 +522,22 @@ static int do_one_pass(journal_t *journal, 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 +547,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, @@ -684,16 +697,20 @@ static int do_one_pass(journal_t *journal, * "Interrupted Commit".) */ - /* 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 = (struct commit_header *)bh->b_data; unsigned found_chksum = be32_to_cpu(cbh->h_chksum[0]); + __be64 commit_time = + be64_to_cpu(cbh->h_commit_sec); if (info->end_transaction) { journal->j_failed_commit = @@ -702,6 +719,33 @@ static int do_one_pass(journal_t *journal, break; } + /* + * If need_check_commit_time is set, it means + * csum verify failed before, if commit_time is + * increasing, it's 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; + } + /* + * 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; + } + /* Neither checksum match nor unused? */ if (!((crc32_sum == found_chksum && cbh->h_chksum_type == @@ -714,6 +758,7 @@ static int do_one_pass(journal_t *journal, goto chksum_error; crc32_sum = ~0; + last_trans_commit_time = commit_time; } if (pass == PASS_SCAN && !jbd2_commit_block_csum_verify(journal, @@ -733,6 +778,17 @@ static int do_one_pass(journal_t *journal, 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 +856,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)