Re: [PATCH v5 1/3] ext4: skip extra isize expansion during mount to prevent deadlock

[Jan Kara]

On Mon 15-06-26 19:53:15, Yun Zhou wrote:
> ext4_try_to_expand_extra_isize() is called from __ext4_mark_inode_dirty()
> while holding an active jbd2 handle.  During mount (!SB_ACTIVE), the
> expand path may move xattrs to external blocks and release ea_inodes via
> iput().  When !SB_ACTIVE, iput() calls write_inode_now() which acquires
> s_writepages_rwsem, creating a circular lock dependency:
> 
>   s_writepages_rwsem --> jbd2_handle --> xattr_sem --> s_writepages_rwsem
> 
> This can be triggered via:
> 
>   ext4_process_orphan() -> ext4_truncate() -> ext4_mark_inode_dirty()
>     -> ext4_try_to_expand_extra_isize()
> 
> or:
> 
>   ext4_evict_inode() -> ext4_mark_inode_dirty()
>     -> ext4_try_to_expand_extra_isize()
> 
> Skip expansion when !SB_ACTIVE.  This is a minor loss of functionality
> (extra isize won't grow for these inodes during mount), which e2fsck
> can resolve later if needed.
> 
> Reported-by: syzbot+5d19358d7eb30ffb0cc5@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=5d19358d7eb30ffb0cc5
> Fixes: c8585c6fcaf2 ("ext4: fix races between changing inode journal mode and ext4_writepages")
> Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>

Yeah, looks as a sensible compromise. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/ext4/inode.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index cd7588a3fa45..be1e3eaa4f23 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -6479,6 +6479,16 @@ static int ext4_try_to_expand_extra_isize(struct inode *inode,
>  	if (ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND))
>  		return -EOVERFLOW;
>  
> +	/*
> +	 * Skip expansion during mount (!SB_ACTIVE).  Expanding extra isize
> +	 * may move xattrs to external blocks and release ea_inodes via iput.
> +	 * When !SB_ACTIVE, iput triggers write_inode_now() which acquires
> +	 * s_writepages_rwsem, causing a deadlock with the caller's active
> +	 * jbd2 handle (lock order: s_writepages_rwsem -> jbd2_handle).
> +	 */
> +	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE)))
> +		return -EBUSY;
> +
>  	/*
>  	 * In nojournal mode, we can immediately attempt to expand
>  	 * the inode.  When journaled, we first need to obtain extra
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR