[PATCH] ext4: fix circular lock dependency in ext4_ext_migrate

[PATCH] ext4: fix circular lock dependency in ext4_ext_migrate

[Yun Zhou]

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@syzkaller.appspotmail.com
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@windriver.com>
---
 fs/ext4/migrate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 477d43d7e294..25368eb44e85 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -464,6 +464,7 @@ int ext4_ext_migrate(struct inode *inode)
 	if (IS_ERR(tmp_inode)) {
 		retval = PTR_ERR(tmp_inode);
 		ext4_journal_stop(handle);
+		tmp_inode = NULL;
 		goto out_unlock;
 	}
 	/*
@@ -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);
 	return retval;
 }
 
-- 
2.43.0

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@syzkaller.appspotmail.com
> 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@windriver.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

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@suse.com>
SUSE Labs, CR

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

[Zhou, Yun]

On 6/9/26 20:05, Jan Kara wrote:
> Looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> 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.
Hi Jan,

Thank you for your careful review. Should I remove this redundant check 
in v2?

BR,
Yun

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

[Jan Kara]

On Wed 10-06-26 15:04:33, Zhou, Yun wrote:
> 
> 
> On 6/9/26 20:05, Jan Kara wrote:
> > Looks good. Feel free to add:
> > 
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > 
> > 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.
> Hi Jan,
> 
> Thank you for your careful review. Should I remove this redundant check in
> v2?

Yes, please. Thank you!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Yun Zhou]

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@syzkaller.appspotmail.com
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@windriver.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
v2: remove redundant null pointer check for iput(tmp_inode)

 fs/ext4/migrate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 477d43d7e294..5d60ef10fe11 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -464,6 +464,7 @@ int ext4_ext_migrate(struct inode *inode)
 	if (IS_ERR(tmp_inode)) {
 		retval = PTR_ERR(tmp_inode);
 		ext4_journal_stop(handle);
+		tmp_inode = NULL;
 		goto out_unlock;
 	}
 	/*
@@ -591,9 +592,9 @@ 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);
+	iput(tmp_inode);
 	return retval;
 }
 
-- 
2.43.0

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

[Yun Zhou]

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+212e8f62790f8e0bc63b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=212e8f62790f8e0bc63b
Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
v3: fixes Reported-by tag and Closes tag.

v2: remove redundant null pointer check for iput(tmp_inode).

 fs/ext4/migrate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 477d43d7e294..5d60ef10fe11 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -464,6 +464,7 @@ int ext4_ext_migrate(struct inode *inode)
 	if (IS_ERR(tmp_inode)) {
 		retval = PTR_ERR(tmp_inode);
 		ext4_journal_stop(handle);
+		tmp_inode = NULL;
 		goto out_unlock;
 	}
 	/*
@@ -591,9 +592,9 @@ 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);
+	iput(tmp_inode);
 	return retval;
 }
 
-- 
2.43.0

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

[Zhou, Yun]

> 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+212e8f62790f8e0bc63b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=212e8f62790f8e0bc63b
> Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
> v3: fixes Reported-by tag and Closes tag.
>
> v2: remove redundant null pointer check for iput(tmp_inode).
>
>   fs/ext4/migrate.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
Hi Honza,

Thank you very much for taking the time to review these patches and 
providing your valuable suggestions. I am eager to solve these 
long-standing deadlock issues on Syzkaller, but I do not have much 
community experience. I'd like to know, regarding this patch, should I 
launch a new RR thread or continue waiting? BR, Yun

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

[Jan Kara]

Hello Yun!

On Tue 16-06-26 15:51:13, Zhou, Yun 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+212e8f62790f8e0bc63b@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=212e8f62790f8e0bc63b
> > Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
> > Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > ---
> > v3: fixes Reported-by tag and Closes tag.
> > 
> > v2: remove redundant null pointer check for iput(tmp_inode).
> > 
> >   fs/ext4/migrate.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Thank you very much for taking the time to review these patches and
> providing your valuable suggestions. I am eager to solve these long-standing
> deadlock issues on Syzkaller, but I do not have much community experience.
> I'd like to know, regarding this patch, should I launch a new RR thread or
> continue waiting? BR, Yun

Please keep waiting. On Sunday the merge window for 7.2-rc1 has opened.
That means that about week before and at least for the next week,
maintainers often aren't taking any patches in their trees. I expect Ted to
pick your patch later when he collects fixes to send for 7.2-rc2 or so - he
sends email about that as a reply to the patch. If nothing happens for next
two weeks, I suggest you send a ping asking whether the patch didn't get
lost as a reply to your patch submission. Thanks for your fixes!

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Zhou, Yun]

Friendly ping.

On 6/12/26 08:53, 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+212e8f62790f8e0bc63b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=212e8f62790f8e0bc63b
> Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
> v3: fixes Reported-by tag and Closes tag.
> 
> v2: remove redundant null pointer check for iput(tmp_inode).
> 
>   fs/ext4/migrate.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 477d43d7e294..5d60ef10fe11 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -464,6 +464,7 @@ int ext4_ext_migrate(struct inode *inode)
>   	if (IS_ERR(tmp_inode)) {
>   		retval = PTR_ERR(tmp_inode);
>   		ext4_journal_stop(handle);
> +		tmp_inode = NULL;
>   		goto out_unlock;
>   	}
>   	/*
> @@ -591,9 +592,9 @@ 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);
> +	iput(tmp_inode);
>   	return retval;
>   }
>   

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

[Zhou, Yun]

Hi Ted,

Gentle ping on this patch. It has been reviewed and received
Reviewed-by tags from Jan Kara a few weeks ago. I haven't heard back 
since then, so I wanted to check if there's
anything else needed from my side.

BR,
Yun

On 6/12/26 08:53, 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+212e8f62790f8e0bc63b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=212e8f62790f8e0bc63b
> Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
> v3: fixes Reported-by tag and Closes tag.
> 
> v2: remove redundant null pointer check for iput(tmp_inode).
> 
>   fs/ext4/migrate.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 477d43d7e294..5d60ef10fe11 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -464,6 +464,7 @@ int ext4_ext_migrate(struct inode *inode)
>   	if (IS_ERR(tmp_inode)) {
>   		retval = PTR_ERR(tmp_inode);
>   		ext4_journal_stop(handle);
> +		tmp_inode = NULL;
>   		goto out_unlock;
>   	}
>   	/*
> @@ -591,9 +592,9 @@ 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);
> +	iput(tmp_inode);
>   	return retval;
>   }
>