Re: [PATCH] ext4: fix circular lock dependency in ext4_ext_migrate

[Jan Kara]

On Tue 09-06-26 16:40:07, Yun Zhou wrote:
> Move iput(tmp_inode) after ext4_writepages_up_write() to avoid a
> circular lock dependency between s_writepages_rwsem and sb_internal
> (freeze protection).
> 
> The deadlock scenario:
> 
>   CPU0 (EXT4_IOC_MIGRATE)        CPU1 (orphan cleanup during mount)
>   ----                           ----
>   ext4_ext_migrate()
>     ext4_writepages_down_write()
>       s_writepages_rwsem (write)
>                                  ext4_evict_inode()
>                                    sb_start_intwrite()   [sb_internal]
>                                    ...
>                                      ext4_writepages()
>                                        s_writepages_rwsem (read) [BLOCKED]
>     iput(tmp_inode)
>       ext4_evict_inode()
>         sb_start_intwrite()         [BLOCKED]
> 
> The tmp_inode is a temporary inode with nlink=0 created solely for
> building the extent tree.  Its eviction does not require
> s_writepages_rwsem protection, so deferring iput() until after
> releasing the rwsem is safe.
> 
> Reported-by: syzbot+f0b58a1f5075a90dd9a5@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=f0b58a1f5075a90dd9a5
> Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
> Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

Just one nit below:

> @@ -591,9 +592,10 @@ int ext4_ext_migrate(struct inode *inode)
>  	ext4_journal_stop(handle);
>  out_tmp_inode:
>  	unlock_new_inode(tmp_inode);
> -	iput(tmp_inode);
>  out_unlock:
>  	ext4_writepages_up_write(inode->i_sb, alloc_ctx);
> +	if (tmp_inode)
> +		iput(tmp_inode);

iput(NULL) is properly handled so you don't need the if (tmp_inode) check
here.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR