linux-kernel - Re: [f2fs-dev] [PATCH] f2fs: fix to avoid migrating empty section

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHJ8P3+NJfE+F-Tsbw4Rz6PyT9o2F74kyrJncLqyQv+YeOSvgA@mail.gmail.com>
Date: Wed, 3 Sep 2025 08:04:22 +0800
From: Zhiguo Niu <niuzhiguo84@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: jaegeuk@...nel.org, Daeho Jeong <daehojeong@...gle.com>, linux-kernel@...r.kernel.org, 
	linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to avoid migrating empty section

Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@...ts.sourceforge.net>
于2025年9月1日周一 10:08写道：
>
> It reports a bug from device w/ zufs:
>
> F2FS-fs (dm-64): Inconsistent segment (173822) type [1, 0] in SSA and SIT
> F2FS-fs (dm-64): Stopped filesystem due to reason: 4
>
> Thread A                                Thread B
> - f2fs_expand_inode_data
>  - f2fs_allocate_pinning_section
>   - f2fs_gc_range
>    - do_garbage_collect w/ segno #x
>                                         - writepage
>                                          - f2fs_allocate_data_block
>                                           - new_curseg
>                                            - allocate segno #x
>
> The root cause is: fallocate on pinning file may race w/ block allocation
> as above, result in do_garbage_collect() from fallocate() may migrate
> segment which is just allocated by a log, the log will update segment type
> in its in-memory structure, however GC will get segment type from on-disk
> SSA block, once segment type changes by log, we can detect such
> inconsistency, then shutdown filesystem.
>
> In this case, on-disk SSA shows type of segno #173822 is 1 (SUM_TYPE_NODE),
> however segno #173822 was just allocated as data type segment, so in-memory
> SIT shows type of segno #173822 is 0 (SUM_TYPE_DATA).
>
> Change as below to fix this issue:
> - check whether current section is empty before gc
> - add sanity checks on do_garbage_collect() to avoid any race case, result
> in migrating segment used by log.
> - btw, it fixes misc issue in printed logs: "SSA and SIT" -> "SIT and SSA".
>
> Fixes: 9703d69d9d15 ("f2fs: support file pinning for zoned devices")
> Cc: Daeho Jeong <daehojeong@...gle.com>
> Signed-off-by: Chao Yu <chao@...nel.org>
> ---
>  fs/f2fs/gc.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index ed3acbfc83ca..a7708cf80c04 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1794,6 +1794,13 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>                 struct folio *sum_folio = filemap_get_folio(META_MAPPING(sbi),
>                                         GET_SUM_BLOCK(sbi, segno));
>
> +               if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno))) {
> +                       f2fs_err(sbi, "%s: segment %u is used by log",
> +                                                       __func__, segno);
> +                       f2fs_bug_on(sbi, 1);
Hi Chao,
Do we need f2fs_bug_on here?  or it is enough to skip current segno
and let gc continue?
Thanks!
> +                       goto skip;
> +               }
> +
>                 if (get_valid_blocks(sbi, segno, false) == 0)
>                         goto freed;
>                 if (gc_type == BG_GC && __is_large_section(sbi) &&
> @@ -1805,7 +1812,7 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>
>                 sum = folio_address(sum_folio);
>                 if (type != GET_SUM_TYPE((&sum->footer))) {
> -                       f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SSA and SIT",
> +                       f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SIT and SSA",
>                                  segno, type, GET_SUM_TYPE((&sum->footer)));
>                         f2fs_stop_checkpoint(sbi, false,
>                                 STOP_CP_REASON_CORRUPTED_SUMMARY);
> @@ -2068,6 +2075,13 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,
>                         .iroot = RADIX_TREE_INIT(gc_list.iroot, GFP_NOFS),
>                 };
>
> +               /*
> +                * avoid migrating empty section, as it can be allocated by
> +                * log in parallel.
> +                */
> +               if (!get_valid_blocks(sbi, segno, true))
> +                       continue;
> +
>                 if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno)))
>                         continue;
>
> --
> 2.49.0
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel