On 2025/3/28 1:48, Ojaswin Mujoo wrote: > Block validity checks need to be skipped in case they are called > for journal blocks since they are part of system's protected > zone. > > Currently, this is done by checking inode->ino against > sbi->s_es->s_journal_inum, which is a direct read from the ext4 sb > buffer head. If someone modifies this underneath us then the > s_journal_inum field might get corrupted. To prevent against this, > change the check to directly compare the inode with journal->j_inode. > > **Slight change in behavior**: During journal init path, > check_block_validity etc might be called for journal inode when > sbi->s_journal is not set yet. In this case we now proceed with > ext4_inode_block_valid() instead of returning early. Since systems zones > have not been set yet, it is okay to proceed so we can perform basic > checks on the blocks. > > Suggested-by: Baokun Li <libaokun1@xxxxxxxxxx> > Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx> Thanks for the inprovement! Besides the indentation that Jan pointed out, it looks good to me. Reviewed-by: Zhang Yi <yi.zhang@xxxxxxxxxx> > --- > > ** Changes since v1 [1] ** > > - instead of using an sbi field direction check against jorunal->j_inode > - let block validity perform basic checks on journal blocks as well > during init path > - kvm-xfstests quick tests are passing > > [1] https://lore.kernel.org/linux-ext4/d1a9328a41029f6210a1924b192a59afcd3c5cee.1741952406.git.ojaswin@xxxxxxxxxxxxx/ > > fs/ext4/block_validity.c | 5 ++--- > fs/ext4/inode.c | 9 +++++---- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c > index 87ee3a17bd29..e8c5525afc67 100644 > --- a/fs/ext4/block_validity.c > +++ b/fs/ext4/block_validity.c > @@ -351,10 +351,9 @@ int ext4_check_blockref(const char *function, unsigned int line, > { > __le32 *bref = p; > unsigned int blk; > + journal_t *journal = EXT4_SB(inode->i_sb)->s_journal; > > - if (ext4_has_feature_journal(inode->i_sb) && > - (inode->i_ino == > - le32_to_cpu(EXT4_SB(inode->i_sb)->s_es->s_journal_inum))) > + if (journal && inode == journal->j_inode) > return 0; > > while (bref < p+max) { > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 365d31004bd0..8b048be14008 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -384,10 +384,11 @@ static int __check_block_validity(struct inode *inode, const char *func, > unsigned int line, > struct ext4_map_blocks *map) > { > - if (ext4_has_feature_journal(inode->i_sb) && > - (inode->i_ino == > - le32_to_cpu(EXT4_SB(inode->i_sb)->s_es->s_journal_inum))) > - return 0; > + journal_t *journal = EXT4_SB(inode->i_sb)->s_journal; > + > + if (journal && inode == journal->j_inode) > + return 0; > + > if (!ext4_inode_block_valid(inode, map->m_pblk, map->m_len)) { > ext4_error_inode(inode, func, line, map->m_pblk, > "lblock %lu mapped to illegal pblock %llu "
