[f2fs-dev] [PATCH] f2fs: fix potential deadlock in f2fs_convert_inline_inode

[f2fs-dev] [PATCH] f2fs: fix potential deadlock in f2fs_convert_inline_inode

[Greg Kroah-Hartman]

f2fs_convert_inline_inode() holds the page lock of the inline data page
and then calls f2fs_lock_op(), which acquires cp_rwsem in read mode.
At the same time, f2fs_write_checkpoint() can acquire cp_rwsem in write
mode and then will wait for page locks, like during
f2fs_write_node_pages() or data flushing, leading to a deadlock.

Fix this by acquiring the lock_op before locking the page. This ensures
the correct lock ordering, op before page, and avoids the deadlock.

Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Chao Yu <chao@kernel.org>
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

This issue was found by running a tool to compare a past kernel CVE to
try to find any potential places in the existing codebase that was
missed with the original fix.  I do not know if this patch or text
really is correct, but the code paths seems sane.

Note that the majority of the changelog text came from an untrusted and
experimental LLM model that is known for making crap up.  So it might be
totally lying here, and if so, I am very sorry for wasting anyone's time
and I'll just go back to running this on code that I actually understand
and know how to verify myself, but I figured it was worth at least
asking you all about it.

fs/f2fs/inline.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 0a1052d5ee62..98bc920a4f35 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -232,12 +232,14 @@ int f2fs_convert_inline_inode(struct inode *inode)
 	if (err)
 		return err;
 
-	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
-	if (IS_ERR(folio))
-		return PTR_ERR(folio);
-
 	f2fs_lock_op(sbi, &lc);
 
+	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
+	if (IS_ERR(folio)) {
+		f2fs_unlock_op(sbi, &lc);
+		return PTR_ERR(folio);
+	}
+
 	ifolio = f2fs_get_inode_folio(sbi, inode->i_ino);
 	if (IS_ERR(ifolio)) {
 		err = PTR_ERR(ifolio);
-- 
2.53.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix potential deadlock in f2fs_convert_inline_inode

[Chao Yu via Linux-f2fs-devel]

On 2/24/26 09:04, Greg Kroah-Hartman wrote:
> f2fs_convert_inline_inode() holds the page lock of the inline data page
> and then calls f2fs_lock_op(), which acquires cp_rwsem in read mode.
> At the same time, f2fs_write_checkpoint() can acquire cp_rwsem in write
> mode and then will wait for page locks, like during
> f2fs_write_node_pages() or data flushing, leading to a deadlock.

- f2fs_convert_inline_inode		- f2fs_write_checkpoint()
 - f2fs_grab_cache_folio page #0
					 - block_operations
					  - f2fs_lock_all
 - f2fs_lock_op
					  - f2fs_sync_node_pages
					   - flush_inline_data
					    - f2fs_filemap_get_folio page #0

It seems true, although I didn't hit such deadlock.

To Jaegeuk, what do you think?

> 
> Fix this by acquiring the lock_op before locking the page. This ensures
> the correct lock ordering, op before page, and avoids the deadlock.
> 
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: Chao Yu <chao@kernel.org>
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 
> This issue was found by running a tool to compare a past kernel CVE to
> try to find any potential places in the existing codebase that was
> missed with the original fix.  I do not know if this patch or text
> really is correct, but the code paths seems sane.
> 
> Note that the majority of the changelog text came from an untrusted and
> experimental LLM model that is known for making crap up.  So it might be
> totally lying here, and if so, I am very sorry for wasting anyone's time
> and I'll just go back to running this on code that I actually understand
> and know how to verify myself, but I figured it was worth at least
> asking you all about it.
> 
> fs/f2fs/inline.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 0a1052d5ee62..98bc920a4f35 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -232,12 +232,14 @@ int f2fs_convert_inline_inode(struct inode *inode)
>  	if (err)
>  		return err;
>  
> -	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
> -	if (IS_ERR(folio))
> -		return PTR_ERR(folio);
> -
>  	f2fs_lock_op(sbi, &lc);
>  
> +	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
> +	if (IS_ERR(folio)) {
> +		f2fs_unlock_op(sbi, &lc);
> +		return PTR_ERR(folio);
> +	}
> +

It will be better to adjust the unlock order in between cp_rwsem and folio lock?

out:
	f2fs_folio_put(folio, true);
	f2fs_unlock_op(sbi, &lc);

Thanks,

>  	ifolio = f2fs_get_inode_folio(sbi, inode->i_ino);
>  	if (IS_ERR(ifolio)) {
>  		err = PTR_ERR(ifolio);



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix potential deadlock in f2fs_convert_inline_inode

[Jaegeuk Kim via Linux-f2fs-devel]

On 03/09, Chao Yu wrote:
> On 2/24/26 09:04, Greg Kroah-Hartman wrote:
> > f2fs_convert_inline_inode() holds the page lock of the inline data page
> > and then calls f2fs_lock_op(), which acquires cp_rwsem in read mode.
> > At the same time, f2fs_write_checkpoint() can acquire cp_rwsem in write
> > mode and then will wait for page locks, like during
> > f2fs_write_node_pages() or data flushing, leading to a deadlock.
> 
> - f2fs_convert_inline_inode		- f2fs_write_checkpoint()
>  - f2fs_grab_cache_folio page #0
> 					 - block_operations
> 					  - f2fs_lock_all
>  - f2fs_lock_op
> 					  - f2fs_sync_node_pages
> 					   - flush_inline_data
> 					    - f2fs_filemap_get_folio page #0

flush_inline_data -> f2fs_filemap_get_folio(FGP_LOCK|FGP_NOWAIT) does not
wait for the lock, right?

> 
> It seems true, although I didn't hit such deadlock.
> 
> To Jaegeuk, what do you think?
> 
> > 
> > Fix this by acquiring the lock_op before locking the page. This ensures
> > the correct lock ordering, op before page, and avoids the deadlock.
> > 
> > Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> > Cc: Chao Yu <chao@kernel.org>
> > Cc: stable <stable@kernel.org>
> > Assisted-by: gkh_clanker_2000
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > 
> > This issue was found by running a tool to compare a past kernel CVE to
> > try to find any potential places in the existing codebase that was
> > missed with the original fix.  I do not know if this patch or text
> > really is correct, but the code paths seems sane.
> > 
> > Note that the majority of the changelog text came from an untrusted and
> > experimental LLM model that is known for making crap up.  So it might be
> > totally lying here, and if so, I am very sorry for wasting anyone's time
> > and I'll just go back to running this on code that I actually understand
> > and know how to verify myself, but I figured it was worth at least
> > asking you all about it.
> > 
> > fs/f2fs/inline.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > index 0a1052d5ee62..98bc920a4f35 100644
> > --- a/fs/f2fs/inline.c
> > +++ b/fs/f2fs/inline.c
> > @@ -232,12 +232,14 @@ int f2fs_convert_inline_inode(struct inode *inode)
> >  	if (err)
> >  		return err;
> >  
> > -	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
> > -	if (IS_ERR(folio))
> > -		return PTR_ERR(folio);
> > -
> >  	f2fs_lock_op(sbi, &lc);
> >  
> > +	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
> > +	if (IS_ERR(folio)) {
> > +		f2fs_unlock_op(sbi, &lc);
> > +		return PTR_ERR(folio);
> > +	}
> > +
> 
> It will be better to adjust the unlock order in between cp_rwsem and folio lock?
> 
> out:
> 	f2fs_folio_put(folio, true);
> 	f2fs_unlock_op(sbi, &lc);
> 
> Thanks,
> 
> >  	ifolio = f2fs_get_inode_folio(sbi, inode->i_ino);
> >  	if (IS_ERR(ifolio)) {
> >  		err = PTR_ERR(ifolio);


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix potential deadlock in f2fs_convert_inline_inode

[Chao Yu via Linux-f2fs-devel]

On 2026/3/12 02:30, Jaegeuk Kim wrote:
> On 03/09, Chao Yu wrote:
>> On 2/24/26 09:04, Greg Kroah-Hartman wrote:
>>> f2fs_convert_inline_inode() holds the page lock of the inline data page
>>> and then calls f2fs_lock_op(), which acquires cp_rwsem in read mode.
>>> At the same time, f2fs_write_checkpoint() can acquire cp_rwsem in write
>>> mode and then will wait for page locks, like during
>>> f2fs_write_node_pages() or data flushing, leading to a deadlock.
>>
>> - f2fs_convert_inline_inode		- f2fs_write_checkpoint()
>>   - f2fs_grab_cache_folio page #0
>> 					 - block_operations
>> 					  - f2fs_lock_all
>>   - f2fs_lock_op
>> 					  - f2fs_sync_node_pages
>> 					   - flush_inline_data
>> 					    - f2fs_filemap_get_folio page #0
> 
> flush_inline_data -> f2fs_filemap_get_folio(FGP_LOCK|FGP_NOWAIT) does not
> wait for the lock, right?

Oh, I missed that, that explain why I never hit such deadlock. :)

Thanks,

> 
>>
>> It seems true, although I didn't hit such deadlock.
>>
>> To Jaegeuk, what do you think?
>>
>>>
>>> Fix this by acquiring the lock_op before locking the page. This ensures
>>> the correct lock ordering, op before page, and avoids the deadlock.
>>>
>>> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
>>> Cc: Chao Yu <chao@kernel.org>
>>> Cc: stable <stable@kernel.org>
>>> Assisted-by: gkh_clanker_2000
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>>
>>> This issue was found by running a tool to compare a past kernel CVE to
>>> try to find any potential places in the existing codebase that was
>>> missed with the original fix.  I do not know if this patch or text
>>> really is correct, but the code paths seems sane.
>>>
>>> Note that the majority of the changelog text came from an untrusted and
>>> experimental LLM model that is known for making crap up.  So it might be
>>> totally lying here, and if so, I am very sorry for wasting anyone's time
>>> and I'll just go back to running this on code that I actually understand
>>> and know how to verify myself, but I figured it was worth at least
>>> asking you all about it.
>>>
>>> fs/f2fs/inline.c | 10 ++++++----
>>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>>> index 0a1052d5ee62..98bc920a4f35 100644
>>> --- a/fs/f2fs/inline.c
>>> +++ b/fs/f2fs/inline.c
>>> @@ -232,12 +232,14 @@ int f2fs_convert_inline_inode(struct inode *inode)
>>>   	if (err)
>>>   		return err;
>>>   
>>> -	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
>>> -	if (IS_ERR(folio))
>>> -		return PTR_ERR(folio);
>>> -
>>>   	f2fs_lock_op(sbi, &lc);
>>>   
>>> +	folio = f2fs_grab_cache_folio(inode->i_mapping, 0, false);
>>> +	if (IS_ERR(folio)) {
>>> +		f2fs_unlock_op(sbi, &lc);
>>> +		return PTR_ERR(folio);
>>> +	}
>>> +
>>
>> It will be better to adjust the unlock order in between cp_rwsem and folio lock?
>>
>> out:
>> 	f2fs_folio_put(folio, true);
>> 	f2fs_unlock_op(sbi, &lc);
>>
>> Thanks,
>>
>>>   	ifolio = f2fs_get_inode_folio(sbi, inode->i_ino);
>>>   	if (IS_ERR(ifolio)) {
>>>   		err = PTR_ERR(ifolio);



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel