Re: [PATCH] [PATCH v5]jbd2: avoid transaction reuse after reformatting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Fengnan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tytso-fscrypt/master]
[cannot apply to ext4/dev linus/master v5.9 next-20201012]
[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/Fengnan-Chang/jbd2-avoid-transaction-reuse-after-reformatting/20201012-215959
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/fscrypt.git master
config: sparc64-randconfig-s032-20201012 (attached as .config)
compiler: sparc64-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.3-rc1-dirty
        # https://github.com/0day-ci/linux/commit/fa9e125afb86200f923868b057a3e77ec7dcb215
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Fengnan-Chang/jbd2-avoid-transaction-reuse-after-reformatting/20201012-215959
        git checkout fa9e125afb86200f923868b057a3e77ec7dcb215
        # 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=sparc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>


"sparse warnings: (new ones prefixed by >>)"
>> fs/jbd2/recovery.c:708:45: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be64 [usertype] commit_time @@     got unsigned long long [usertype] @@
   fs/jbd2/recovery.c:708:45: sparse:     expected restricted __be64 [usertype] commit_time
   fs/jbd2/recovery.c:708:45: sparse:     got unsigned long long [usertype]
   fs/jbd2/recovery.c:717:45: sparse: sparse: restricted __be64 degrades to integer
   fs/jbd2/recovery.c:718:49: sparse: sparse: restricted __be64 degrades to integer
   fs/jbd2/recovery.c:781:45: sparse: sparse: restricted __be64 degrades to integer
   fs/jbd2/recovery.c:782:49: sparse: sparse: restricted __be64 degrades to integer
   fs/jbd2/recovery.c:802:37: sparse: sparse: restricted __be64 degrades to integer
   fs/jbd2/recovery.c:802:51: sparse: sparse: restricted __be64 degrades to integer

vim +708 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, 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 recovering block %lu in log\n",
   531						       next_log_block);
   532						err = -EFSBADCRC;
   533						brelse(bh);
   534						goto failed;
   535					}
   536					need_check_commit_time = true;
   537					jbd_debug(1,
   538						"invalid descriptor block found in %lu\n",
   539						next_log_block);
   540				}
   541	
   542				/* If it is a valid descriptor block, replay it
   543				 * in pass REPLAY; if journal_checksums enabled, then
   544				 * calculate checksums in PASS_SCAN, otherwise,
   545				 * just skip over the blocks it describes. */
   546				if (pass != PASS_REPLAY) {
   547					if (pass == PASS_SCAN &&
   548					    jbd2_has_feature_checksum(journal) &&
   549					    !need_check_commit_time &&
   550					    !info->end_transaction) {
   551						if (calc_chksums(journal, bh,
   552								&next_log_block,
   553								&crc32_sum)) {
   554							put_bh(bh);
   555							break;
   556						}
   557						put_bh(bh);
   558						continue;
   559					}
   560					next_log_block += count_tags(journal, bh);
   561					wrap(journal, next_log_block);
   562					put_bh(bh);
   563					continue;
   564				}
   565	
   566				/* A descriptor block: we can now write all of
   567				 * the data blocks.  Yay, useful work is finally
   568				 * getting done here! */
   569	
   570				tagp = &bh->b_data[sizeof(journal_header_t)];
   571				while ((tagp - bh->b_data + tag_bytes)
   572				       <= journal->j_blocksize - descr_csum_size) {
   573					unsigned long io_block;
   574	
   575					tag = (journal_block_tag_t *) tagp;
   576					flags = be16_to_cpu(tag->t_flags);
   577	
   578					io_block = next_log_block++;
   579					wrap(journal, next_log_block);
   580					err = jread(&obh, journal, io_block);
   581					if (err) {
   582						/* Recover what we can, but
   583						 * report failure at the end. */
   584						success = err;
   585						printk(KERN_ERR
   586							"JBD2: IO error %d recovering "
   587							"block %ld in log\n",
   588							err, io_block);
   589					} else {
   590						unsigned long long blocknr;
   591	
   592						J_ASSERT(obh != NULL);
   593						blocknr = read_tag_block(journal,
   594									 tag);
   595	
   596						/* If the block has been
   597						 * revoked, then we're all done
   598						 * here. */
   599						if (jbd2_journal_test_revoke
   600						    (journal, blocknr,
   601						     next_commit_ID)) {
   602							brelse(obh);
   603							++info->nr_revoke_hits;
   604							goto skip_write;
   605						}
   606	
   607						/* Look for block corruption */
   608						if (!jbd2_block_tag_csum_verify(
   609							journal, tag, obh->b_data,
   610							be32_to_cpu(tmp->h_sequence))) {
   611							brelse(obh);
   612							success = -EFSBADCRC;
   613							printk(KERN_ERR "JBD2: Invalid "
   614							       "checksum recovering "
   615							       "data block %llu in "
   616							       "log\n", blocknr);
   617							block_error = 1;
   618							goto skip_write;
   619						}
   620	
   621						/* Find a buffer for the new
   622						 * data being restored */
   623						nbh = __getblk(journal->j_fs_dev,
   624								blocknr,
   625								journal->j_blocksize);
   626						if (nbh == NULL) {
   627							printk(KERN_ERR
   628							       "JBD2: Out of memory "
   629							       "during recovery.\n");
   630							err = -ENOMEM;
   631							brelse(bh);
   632							brelse(obh);
   633							goto failed;
   634						}
   635	
   636						lock_buffer(nbh);
   637						memcpy(nbh->b_data, obh->b_data,
   638								journal->j_blocksize);
   639						if (flags & JBD2_FLAG_ESCAPE) {
   640							*((__be32 *)nbh->b_data) =
   641							cpu_to_be32(JBD2_MAGIC_NUMBER);
   642						}
   643	
   644						BUFFER_TRACE(nbh, "marking dirty");
   645						set_buffer_uptodate(nbh);
   646						mark_buffer_dirty(nbh);
   647						BUFFER_TRACE(nbh, "marking uptodate");
   648						++info->nr_replays;
   649						/* ll_rw_block(WRITE, 1, &nbh); */
   650						unlock_buffer(nbh);
   651						brelse(obh);
   652						brelse(nbh);
   653					}
   654	
   655				skip_write:
   656					tagp += tag_bytes;
   657					if (!(flags & JBD2_FLAG_SAME_UUID))
   658						tagp += 16;
   659	
   660					if (flags & JBD2_FLAG_LAST_TAG)
   661						break;
   662				}
   663	
   664				brelse(bh);
   665				continue;
   666	
   667			case JBD2_COMMIT_BLOCK:
   668				/*     How to differentiate between interrupted commit
   669				 *               and journal corruption ?
   670				 *
   671				 * {nth transaction}
   672				 *        Checksum Verification Failed
   673				 *			 |
   674				 *		 ____________________
   675				 *		|		     |
   676				 * 	async_commit             sync_commit
   677				 *     		|                    |
   678				 *		| GO TO NEXT    "Journal Corruption"
   679				 *		| TRANSACTION
   680				 *		|
   681				 * {(n+1)th transanction}
   682				 *		|
   683				 * 	 _______|______________
   684				 * 	|	 	      |
   685				 * Commit block found	Commit block not found
   686				 *      |		      |
   687				 * "Journal Corruption"       |
   688				 *		 _____________|_________
   689				 *     		|	           	|
   690				 *	nth trans corrupt	OR   nth trans
   691				 *	and (n+1)th interrupted     interrupted
   692				 *	before commit block
   693				 *      could reach the disk.
   694				 *	(Cannot find the difference in above
   695				 *	 mentioned conditions. Hence assume
   696				 *	 "Interrupted Commit".)
   697				 */
   698				/*
   699				 * If need_check_commit_time is set, it means
   700				 * csum verify failed before, if commit_time is
   701				 * increasing, it's same journal, otherwise it
   702				 * is stale journal block, just end this
   703				 * recovery.
   704				 */
   705				if (pass == PASS_SCAN) {
   706					struct commit_header *cbh =
   707						(struct commit_header *)bh->b_data;
 > 708					commit_time =
   709						be64_to_cpu(cbh->h_commit_sec);
   710					/*
   711					 * When need check commit time, it means csum
   712					 * verify failed before, if commit time is
   713					 * increasing, it's same journal, otherwise
   714					 * not same journal, just end this recovery.
   715					 */
   716					if (need_check_commit_time) {
   717						if (commit_time >=
   718							last_trans_commit_time) {
   719							pr_err("JBD2: Invalid checksum found in transaction %u\n",
   720									next_commit_ID);
   721							err = -EFSBADCRC;
   722							brelse(bh);
   723							goto failed;
   724						}
   725						/*
   726						 * It likely does not belong to same
   727						 * journal, just end this recovery with
   728						 * success.
   729						 */
   730						jbd_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n",
   731								next_commit_ID);
   732						err = 0;
   733						brelse(bh);
   734						goto done;
   735					}
   736				}
   737				/* Found an expected commit block: if checksums
   738				 * are present verify them in PASS_SCAN; else not
   739				 * much to do other than move on to the next sequence
   740				 * number.
   741				 */
   742				if (pass == PASS_SCAN &&
   743				    jbd2_has_feature_checksum(journal)) {
   744					int chksum_err, chksum_seen;
   745					struct commit_header *cbh =
   746						(struct commit_header *)bh->b_data;
   747					unsigned found_chksum =
   748						be32_to_cpu(cbh->h_chksum[0]);
   749	
   750					chksum_err = chksum_seen = 0;
   751	
   752					if (info->end_transaction) {
   753						journal->j_failed_commit =
   754							info->end_transaction;
   755						brelse(bh);
   756						break;
   757					}
   758	
   759					if (crc32_sum == found_chksum &&
   760					    cbh->h_chksum_type == JBD2_CRC32_CHKSUM &&
   761					    cbh->h_chksum_size ==
   762							JBD2_CRC32_CHKSUM_SIZE)
   763					       chksum_seen = 1;
   764					else if (!(cbh->h_chksum_type == 0 &&
   765						     cbh->h_chksum_size == 0 &&
   766						     found_chksum == 0 &&
   767						     !chksum_seen))
   768					/*
   769					 * If fs is mounted using an old kernel and then
   770					 * kernel with journal_chksum is used then we
   771					 * get a situation where the journal flag has
   772					 * checksum flag set but checksums are not
   773					 * present i.e chksum = 0, in the individual
   774					 * commit blocks.
   775					 * Hence to avoid checksum failures, in this
   776					 * situation, this extra check is added.
   777					 */
   778							chksum_err = 1;
   779	
   780					if (chksum_err) {
   781						if (commit_time <
   782							last_trans_commit_time) {
   783							jbd_debug(1, "JBD2: Invalid commit checksum ignored in transaction %u, likely stale data\n",
   784								next_log_block);
   785							brelse(bh);
   786							goto done;
   787						}
   788						info->end_transaction = next_commit_ID;
   789	
   790						if (!jbd2_has_feature_async_commit(journal)) {
   791							journal->j_failed_commit =
   792								next_commit_ID;
   793							brelse(bh);
   794							break;
   795						}
   796					}
   797					crc32_sum = ~0;
   798				}
   799				if (pass == PASS_SCAN &&
   800				    !jbd2_commit_block_csum_verify(journal,
   801								   bh->b_data)) {
   802					if (commit_time < last_trans_commit_time) {
   803						jbd_debug(1, "JBD2: Invalid commit checksum ignored in transaction %u, likely stale data\n",
   804							next_log_block);
   805						brelse(bh);
   806						goto done;
   807					}
   808					info->end_transaction = next_commit_ID;
   809	
   810					if (!jbd2_has_feature_async_commit(journal)) {
   811						journal->j_failed_commit =
   812							next_commit_ID;
   813						brelse(bh);
   814						break;
   815					}
   816				}
   817				if (pass == PASS_SCAN)
   818					last_trans_commit_time = commit_time;
   819				brelse(bh);
   820				next_commit_ID++;
   821				continue;
   822	
   823			case JBD2_REVOKE_BLOCK:
   824				/*
   825				 * Check revoke block crc in pass_scan, if csum verify
   826				 * failed, check commit block time later.
   827				 */
   828				if (pass == PASS_SCAN) {
   829					if (!jbd2_descriptor_block_csum_verify(journal,
   830							bh->b_data)) {
   831						jbd_debug(1, "invalid revoke block found in %lu\n",
   832							next_log_block);
   833						need_check_commit_time = true;
   834					}
   835				}
   836				/* If we aren't in the REVOKE pass, then we can
   837				 * just skip over this block. */
   838				if (pass != PASS_REVOKE) {
   839					brelse(bh);
   840					continue;
   841				}
   842	
   843				err = scan_revoke_records(journal, bh,
   844							  next_commit_ID, info);
   845				brelse(bh);
   846				if (err)
   847					goto failed;
   848				continue;
   849	
   850			default:
   851				jbd_debug(3, "Unrecognised magic %d, end of scan.\n",
   852					  blocktype);
   853				brelse(bh);
   854				goto done;
   855			}
   856		}
   857	
   858	 done:
   859		/*
   860		 * We broke out of the log scan loop: either we came to the
   861		 * known end of the log or we found an unexpected block in the
   862		 * log.  If the latter happened, then we know that the "current"
   863		 * transaction marks the end of the valid log.
   864		 */
   865	
   866		if (pass == PASS_SCAN) {
   867			if (!info->end_transaction)
   868				info->end_transaction = next_commit_ID;
   869		} else {
   870			/* It's really bad news if different passes end up at
   871			 * different places (but possible due to IO errors). */
   872			if (info->end_transaction != next_commit_ID) {
   873				printk(KERN_ERR "JBD2: recovery pass %d ended at "
   874					"transaction %u, expected %u\n",
   875					pass, next_commit_ID, info->end_transaction);
   876				if (!success)
   877					success = -EIO;
   878			}
   879		}
   880		if (block_error && success == 0)
   881			success = -EIO;
   882		return success;
   883	
   884	 failed:
   885		return err;
   886	}
   887	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux