[PATCH] f2fs: fix to avoid potential deadloop in f2fs_fsync_node_pages()

Chao Yu posted 1 patch 1 month ago
fs/f2fs/node.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] f2fs: fix to avoid potential deadloop in f2fs_fsync_node_pages()
Posted by Chao Yu 1 month ago
There is potential deadloop in race condition:

Thread A				Thread B
- fsync
 - f2fs_do_sync_file
  - f2fs_fsync_node_pages
   - last_fsync_dnode
    - folio_get(last_folio)
					- f2fs_setattr
					 - f2fs_truncate
					  - f2fs_truncate_blocks
					   - f2fs_do_truncate_blocks
					    - f2fs_truncate_inode_blocks
					     - truncate_dnode
					      - truncate_node
					       - invalidate_mapping_pages
					        - folio->mapping = NULL
   - is_node_folio alwasy return false
   - atomic && !marked is always true,
     then goto retry

Cc: stable@kernel.org
Fixes: 608514deba38 ("f2fs: set fsync mark only for the last dnode")
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/node.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 968e5ed38816..86c2e67e43b6 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2016,6 +2016,11 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
 		f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
 			   ino, last_folio->index);
 		folio_lock(last_folio);
+		if (unlikely(!is_node_folio(last_folio))) {
+			f2fs_folio_put(last_folio, true);
+			ret = -EAGAIN;
+			goto out;
+		}
 		f2fs_folio_wait_writeback(last_folio, NODE, true, true);
 		folio_mark_dirty(last_folio);
 		folio_unlock(last_folio);
-- 
2.49.0
Re: [f2fs-dev] [PATCH] f2fs: fix to avoid potential deadloop in f2fs_fsync_node_pages()
Posted by Zhiguo Niu 2 weeks, 5 days ago
Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
于2026年8月24日周一 21:19写道:
>
> There is potential deadloop in race condition:
>
> Thread A                                Thread B
> - fsync
>  - f2fs_do_sync_file
>   - f2fs_fsync_node_pages
>    - last_fsync_dnode
>     - folio_get(last_folio)
>                                         - f2fs_setattr
>                                          - f2fs_truncate
>                                           - f2fs_truncate_blocks
>                                            - f2fs_do_truncate_blocks
>                                             - f2fs_truncate_inode_blocks
>                                              - truncate_dnode
>                                               - truncate_node
>                                                - invalidate_mapping_pages
>                                                 - folio->mapping = NULL
>    - is_node_folio alwasy return false
>    - atomic && !marked is always true,
>      then goto retry
>
> Cc: stable@kernel.org
> Fixes: 608514deba38 ("f2fs: set fsync mark only for the last dnode")
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/node.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 968e5ed38816..86c2e67e43b6 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2016,6 +2016,11 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
>                 f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
>                            ino, last_folio->index);
>                 folio_lock(last_folio);
> +               if (unlikely(!is_node_folio(last_folio))) {
Hi Chao,
is_node_folio->NODE_MAPPING(F2FS_F_SB(folio))->F2FS_M_SB(folio->mappping)->F2FS_I_SB(mapping->host)
if mapping==NULL, panic will occur here?
just like  commit msg in  mai: "f2fs: parameterize node helpers and macros"
thanks!

> +                       f2fs_folio_put(last_folio, true);
> +                       ret = -EAGAIN;
> +                       goto out;
> +               }
>                 f2fs_folio_wait_writeback(last_folio, NODE, true, true);
>                 folio_mark_dirty(last_folio);
>                 folio_unlock(last_folio);
> --
> 2.49.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 to avoid potential deadloop in f2fs_fsync_node_pages()
Posted by Chao Yu 2 weeks, 5 days ago
On 9/9/26 09:12, Zhiguo Niu wrote:
> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> 于2026年8月24日周一 21:19写道:
>>
>> There is potential deadloop in race condition:
>>
>> Thread A                                Thread B
>> - fsync
>>  - f2fs_do_sync_file
>>   - f2fs_fsync_node_pages
>>    - last_fsync_dnode
>>     - folio_get(last_folio)
>>                                         - f2fs_setattr
>>                                          - f2fs_truncate
>>                                           - f2fs_truncate_blocks
>>                                            - f2fs_do_truncate_blocks
>>                                             - f2fs_truncate_inode_blocks
>>                                              - truncate_dnode
>>                                               - truncate_node
>>                                                - invalidate_mapping_pages
>>                                                 - folio->mapping = NULL
>>    - is_node_folio alwasy return false
>>    - atomic && !marked is always true,
>>      then goto retry
>>
>> Cc: stable@kernel.org
>> Fixes: 608514deba38 ("f2fs: set fsync mark only for the last dnode")
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>  fs/f2fs/node.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index 968e5ed38816..86c2e67e43b6 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -2016,6 +2016,11 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
>>                 f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
>>                            ino, last_folio->index);
>>                 folio_lock(last_folio);
>> +               if (unlikely(!is_node_folio(last_folio))) {
> Hi Chao,
> is_node_folio->NODE_MAPPING(F2FS_F_SB(folio))->F2FS_M_SB(folio->mappping)->F2FS_I_SB(mapping->host)
> if mapping==NULL, panic will occur here?
> just like  commit msg in  mai: "f2fs: parameterize node helpers and macros"

Hi Zhiguo,

Ah, seems we need to revert ("f2fs: introduce is_{meta,node}_folio") which
introduce this bug? IIUC.

Thanks,

> thanks!
> 
>> +                       f2fs_folio_put(last_folio, true);
>> +                       ret = -EAGAIN;
>> +                       goto out;
>> +               }
>>                 f2fs_folio_wait_writeback(last_folio, NODE, true, true);
>>                 folio_mark_dirty(last_folio);
>>                 folio_unlock(last_folio);
>> --
>> 2.49.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 to avoid potential deadloop in f2fs_fsync_node_pages()
Posted by Zhiguo Niu 2 weeks, 5 days ago
Chao Yu <chao@kernel.org> 于2026年9月9日周三 10:03写道:
>
> On 9/9/26 09:12, Zhiguo Niu wrote:
> > Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> > 于2026年8月24日周一 21:19写道:
> >>
> >> There is potential deadloop in race condition:
> >>
> >> Thread A                                Thread B
> >> - fsync
> >>  - f2fs_do_sync_file
> >>   - f2fs_fsync_node_pages
> >>    - last_fsync_dnode
> >>     - folio_get(last_folio)
> >>                                         - f2fs_setattr
> >>                                          - f2fs_truncate
> >>                                           - f2fs_truncate_blocks
> >>                                            - f2fs_do_truncate_blocks
> >>                                             - f2fs_truncate_inode_blocks
> >>                                              - truncate_dnode
> >>                                               - truncate_node
> >>                                                - invalidate_mapping_pages
> >>                                                 - folio->mapping = NULL
> >>    - is_node_folio alwasy return false
> >>    - atomic && !marked is always true,
> >>      then goto retry
> >>
> >> Cc: stable@kernel.org
> >> Fixes: 608514deba38 ("f2fs: set fsync mark only for the last dnode")
> >> Signed-off-by: Chao Yu <chao@kernel.org>
> >> ---
> >>  fs/f2fs/node.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >> index 968e5ed38816..86c2e67e43b6 100644
> >> --- a/fs/f2fs/node.c
> >> +++ b/fs/f2fs/node.c
> >> @@ -2016,6 +2016,11 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
> >>                 f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
> >>                            ino, last_folio->index);
> >>                 folio_lock(last_folio);
> >> +               if (unlikely(!is_node_folio(last_folio))) {
> > Hi Chao,
> > is_node_folio->NODE_MAPPING(F2FS_F_SB(folio))->F2FS_M_SB(folio->mappping)->F2FS_I_SB(mapping->host)
> > if mapping==NULL, panic will occur here?
> > just like  commit msg in  mai: "f2fs: parameterize node helpers and macros"
>
> Hi Zhiguo,
>
> Ah, seems we need to revert ("f2fs: introduce is_{meta,node}_folio") which
> introduce this bug? IIUC.
Hi Chao,
seems yes,  we can use sbi directly.
thanks!
>
> Thanks,
>
> > thanks!
> >
> >> +                       f2fs_folio_put(last_folio, true);
> >> +                       ret = -EAGAIN;
> >> +                       goto out;
> >> +               }
> >>                 f2fs_folio_wait_writeback(last_folio, NODE, true, true);
> >>                 folio_mark_dirty(last_folio);
> >>                 folio_unlock(last_folio);
> >> --
> >> 2.49.0
> >>
> >>
> >>
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> Linux-f2fs-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>