On Thu 27-03-25 23:18:09, 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>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
One style nit below:
> 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;
Bogus indentation here.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
