Message ID | 20230810085417.1501293-8-yi.zhang@huaweicloud.com |
---|---|
State | Superseded |
Headers | show |
Series | ext4,jbd2: cleanup journal load and initialization process | expand |
On Thu 10-08-23 16:54:12, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@huawei.com> > > If JBD2_FEATURE_INCOMPAT_FAST_COMMIT bit is set, it means the journal > have fast commit records need to recover, so the fast commit size > should not be too large, and the leftover normal journal size should > never less than JBD2_MIN_JOURNAL_BLOCKS. If it happens, the > journal->j_last is likely to be wrong and will probably lead to > incorrect journal recovery. So add a check into the > journal_check_superblock(), and drop the pointless check when > initializing the fastcommit parameters. > > Signed-off-by: Zhang Yi <yi.zhang@huawei.com> Just one small note below. With that fixed feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> > @@ -1389,6 +1390,14 @@ static int journal_check_superblock(journal_t *journal) > return err; > } > > + num_fc_blks = jbd2_has_feature_fast_commit(journal) ? > + jbd2_journal_get_num_fc_blks(sb) : 0; > + if (be32_to_cpu(sb->s_maxlen) < JBD2_MIN_JOURNAL_BLOCKS + num_fc_blks) { To avoid possible overflow of the right hand side, we should probably do the check like: if (be32_to_cpu(sb->s_maxlen) < JBD2_MIN_JOURNAL_BLOCKS || be32_to_cpu(sb->s_maxlen) - JBD2_MIN_JOURNAL_BLOCKS < num_fc_blks) { ... } Honza
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c index a8d17070073b..5e2206e7a5ba 100644 --- a/fs/jbd2/journal.c +++ b/fs/jbd2/journal.c @@ -1347,6 +1347,7 @@ static void journal_fail_superblock(journal_t *journal) static int journal_check_superblock(journal_t *journal) { journal_superblock_t *sb = journal->j_superblock; + int num_fc_blks; int err = -EINVAL; if (sb->s_header.h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER) || @@ -1389,6 +1390,14 @@ static int journal_check_superblock(journal_t *journal) return err; } + num_fc_blks = jbd2_has_feature_fast_commit(journal) ? + jbd2_journal_get_num_fc_blks(sb) : 0; + if (be32_to_cpu(sb->s_maxlen) < JBD2_MIN_JOURNAL_BLOCKS + num_fc_blks) { + printk(KERN_ERR "JBD2: journal file too short %u,%d\n", + be32_to_cpu(sb->s_maxlen), num_fc_blks); + return err; + } + if (jbd2_has_feature_csum2(journal) && jbd2_has_feature_csum3(journal)) { /* Can't have checksum v2 and v3 at the same time! */ @@ -1454,7 +1463,6 @@ static int journal_load_superblock(journal_t *journal) int err; struct buffer_head *bh; journal_superblock_t *sb; - int num_fc_blocks; bh = getblk_unmovable(journal->j_dev, journal->j_blk_offset, journal->j_blocksize); @@ -1492,9 +1500,8 @@ static int journal_load_superblock(journal_t *journal) if (jbd2_has_feature_fast_commit(journal)) { journal->j_fc_last = be32_to_cpu(sb->s_maxlen); - num_fc_blocks = jbd2_journal_get_num_fc_blks(sb); - if (journal->j_last - num_fc_blocks >= JBD2_MIN_JOURNAL_BLOCKS) - journal->j_last = journal->j_fc_last - num_fc_blocks; + journal->j_last = journal->j_fc_last - + jbd2_journal_get_num_fc_blks(sb); journal->j_fc_first = journal->j_last + 1; journal->j_fc_off = 0; }