linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH v1] f2fs: fix uninitialized one_time_gc in victim_sel_policy
@ 2025-10-29  5:18 Xiaole He
  2025-10-29 10:48 ` Chao Yu via Linux-f2fs-devel
  2025-11-11 22:50 ` patchwork-bot+f2fs--- via Linux-f2fs-devel
  0 siblings, 2 replies; 3+ messages in thread
From: Xiaole He @ 2025-10-29  5:18 UTC (permalink / raw)
  To: linux-f2fs-devel; +Cc: Xiaole He, daehojeong, linux-kernel, jaegeuk, stable

The one_time_gc field in struct victim_sel_policy is conditionally
initialized but unconditionally read, leading to undefined behavior
that triggers UBSAN warnings.

In f2fs_get_victim() at fs/f2fs/gc.c:774, the victim_sel_policy
structure is declared without initialization:

    struct victim_sel_policy p;

The field p.one_time_gc is only assigned when the 'one_time' parameter
is true (line 789):

    if (one_time) {
        p.one_time_gc = one_time;
        ...
    }

However, this field is unconditionally read in subsequent get_gc_cost()
at line 395:

    if (p->one_time_gc && (valid_thresh_ratio < 100) && ...)

When one_time is false, p.one_time_gc contains uninitialized stack
memory. Hence p.one_time_gc is an invalid bool value.

UBSAN detects this invalid bool value:

    UBSAN: invalid-load in fs/f2fs/gc.c:395:7
    load of value 77 is not a valid value for type '_Bool'
    CPU: 3 UID: 0 PID: 1297 Comm: f2fs_gc-252:16 Not tainted 6.18.0-rc3
    #5 PREEMPT(voluntary)
    Hardware name: OpenStack Foundation OpenStack Nova,
    BIOS 1.13.0-1ubuntu1.1 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x70/0x90
     dump_stack+0x14/0x20
     __ubsan_handle_load_invalid_value+0xb3/0xf0
     ? dl_server_update+0x2e/0x40
     ? update_curr+0x147/0x170
     f2fs_get_victim.cold+0x66/0x134 [f2fs]
     ? sched_balance_newidle+0x2ca/0x470
     ? finish_task_switch.isra.0+0x8d/0x2a0
     f2fs_gc+0x2ba/0x8e0 [f2fs]
     ? _raw_spin_unlock_irqrestore+0x12/0x40
     ? __timer_delete_sync+0x80/0xe0
     ? timer_delete_sync+0x14/0x20
     ? schedule_timeout+0x82/0x100
     gc_thread_func+0x38b/0x860 [f2fs]
     ? gc_thread_func+0x38b/0x860 [f2fs]
     ? __pfx_autoremove_wake_function+0x10/0x10
     kthread+0x10b/0x220
     ? __pfx_gc_thread_func+0x10/0x10 [f2fs]
     ? _raw_spin_unlock_irq+0x12/0x40
     ? __pfx_kthread+0x10/0x10
     ret_from_fork+0x11a/0x160
     ? __pfx_kthread+0x10/0x10
     ret_from_fork_asm+0x1a/0x30
     </TASK>

This issue is reliably reproducible with the following steps on a
100GB SSD /dev/vdb:

    mkfs.f2fs -f /dev/vdb
    mount /dev/vdb /mnt/f2fs_test
    fio --name=gc --directory=/mnt/f2fs_test --rw=randwrite \
        --bs=4k --size=8G --numjobs=12 --fsync=4 --runtime=10 \
        --time_based
    echo 1 > /sys/fs/f2fs/vdb/gc_urgent

The uninitialized value causes incorrect GC victim selection, leading
to unpredictable garbage collection behavior.

Fix by zero-initializing the entire victim_sel_policy structure to
ensure all fields have defined values.

Fixes: e791d00bd06c ("f2fs: add valid block ratio not to do excessive GC for one time GC")
Cc: stable@kernel.org
Signed-off-by: Xiaole He <hexiaole1994@126.com>
---
 fs/f2fs/gc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index a7708cf80c04..56e372c23a78 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -771,7 +771,7 @@ int f2fs_get_victim(struct f2fs_sb_info *sbi, unsigned int *result,
 {
 	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
 	struct sit_info *sm = SIT_I(sbi);
-	struct victim_sel_policy p;
+	struct victim_sel_policy p = {0};
 	unsigned int secno, last_victim;
 	unsigned int last_segment;
 	unsigned int nsearched;
-- 
2.34.1



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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [f2fs-dev] [PATCH v1] f2fs: fix uninitialized one_time_gc in victim_sel_policy
  2025-10-29  5:18 [f2fs-dev] [PATCH v1] f2fs: fix uninitialized one_time_gc in victim_sel_policy Xiaole He
@ 2025-10-29 10:48 ` Chao Yu via Linux-f2fs-devel
  2025-11-11 22:50 ` patchwork-bot+f2fs--- via Linux-f2fs-devel
  1 sibling, 0 replies; 3+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-10-29 10:48 UTC (permalink / raw)
  To: Xiaole He, linux-f2fs-devel; +Cc: jaegeuk, stable, daehojeong, linux-kernel

On 10/29/25 13:18, Xiaole He wrote:
> The one_time_gc field in struct victim_sel_policy is conditionally
> initialized but unconditionally read, leading to undefined behavior
> that triggers UBSAN warnings.
> 
> In f2fs_get_victim() at fs/f2fs/gc.c:774, the victim_sel_policy
> structure is declared without initialization:
> 
>     struct victim_sel_policy p;
> 
> The field p.one_time_gc is only assigned when the 'one_time' parameter
> is true (line 789):
> 
>     if (one_time) {
>         p.one_time_gc = one_time;
>         ...
>     }
> 
> However, this field is unconditionally read in subsequent get_gc_cost()
> at line 395:
> 
>     if (p->one_time_gc && (valid_thresh_ratio < 100) && ...)
> 
> When one_time is false, p.one_time_gc contains uninitialized stack
> memory. Hence p.one_time_gc is an invalid bool value.
> 
> UBSAN detects this invalid bool value:
> 
>     UBSAN: invalid-load in fs/f2fs/gc.c:395:7
>     load of value 77 is not a valid value for type '_Bool'
>     CPU: 3 UID: 0 PID: 1297 Comm: f2fs_gc-252:16 Not tainted 6.18.0-rc3
>     #5 PREEMPT(voluntary)
>     Hardware name: OpenStack Foundation OpenStack Nova,
>     BIOS 1.13.0-1ubuntu1.1 04/01/2014
>     Call Trace:
>      <TASK>
>      dump_stack_lvl+0x70/0x90
>      dump_stack+0x14/0x20
>      __ubsan_handle_load_invalid_value+0xb3/0xf0
>      ? dl_server_update+0x2e/0x40
>      ? update_curr+0x147/0x170
>      f2fs_get_victim.cold+0x66/0x134 [f2fs]
>      ? sched_balance_newidle+0x2ca/0x470
>      ? finish_task_switch.isra.0+0x8d/0x2a0
>      f2fs_gc+0x2ba/0x8e0 [f2fs]
>      ? _raw_spin_unlock_irqrestore+0x12/0x40
>      ? __timer_delete_sync+0x80/0xe0
>      ? timer_delete_sync+0x14/0x20
>      ? schedule_timeout+0x82/0x100
>      gc_thread_func+0x38b/0x860 [f2fs]
>      ? gc_thread_func+0x38b/0x860 [f2fs]
>      ? __pfx_autoremove_wake_function+0x10/0x10
>      kthread+0x10b/0x220
>      ? __pfx_gc_thread_func+0x10/0x10 [f2fs]
>      ? _raw_spin_unlock_irq+0x12/0x40
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork+0x11a/0x160
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork_asm+0x1a/0x30
>      </TASK>
> 
> This issue is reliably reproducible with the following steps on a
> 100GB SSD /dev/vdb:
> 
>     mkfs.f2fs -f /dev/vdb
>     mount /dev/vdb /mnt/f2fs_test
>     fio --name=gc --directory=/mnt/f2fs_test --rw=randwrite \
>         --bs=4k --size=8G --numjobs=12 --fsync=4 --runtime=10 \
>         --time_based
>     echo 1 > /sys/fs/f2fs/vdb/gc_urgent
> 
> The uninitialized value causes incorrect GC victim selection, leading
> to unpredictable garbage collection behavior.
> 
> Fix by zero-initializing the entire victim_sel_policy structure to
> ensure all fields have defined values.
> 
> Fixes: e791d00bd06c ("f2fs: add valid block ratio not to do excessive GC for one time GC")
> Cc: stable@kernel.org
> Signed-off-by: Xiaole He <hexiaole1994@126.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,



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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [f2fs-dev] [PATCH v1] f2fs: fix uninitialized one_time_gc in victim_sel_policy
  2025-10-29  5:18 [f2fs-dev] [PATCH v1] f2fs: fix uninitialized one_time_gc in victim_sel_policy Xiaole He
  2025-10-29 10:48 ` Chao Yu via Linux-f2fs-devel
@ 2025-11-11 22:50 ` patchwork-bot+f2fs--- via Linux-f2fs-devel
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+f2fs--- via Linux-f2fs-devel @ 2025-11-11 22:50 UTC (permalink / raw)
  To: Xiaole He; +Cc: linux-kernel, jaegeuk, daehojeong, stable, linux-f2fs-devel

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Wed, 29 Oct 2025 13:18:07 +0800 you wrote:
> The one_time_gc field in struct victim_sel_policy is conditionally
> initialized but unconditionally read, leading to undefined behavior
> that triggers UBSAN warnings.
> 
> In f2fs_get_victim() at fs/f2fs/gc.c:774, the victim_sel_policy
> structure is declared without initialization:
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v1] f2fs: fix uninitialized one_time_gc in victim_sel_policy
    https://git.kernel.org/jaegeuk/f2fs/c/f10e76889502

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-11-11 22:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29  5:18 [f2fs-dev] [PATCH v1] f2fs: fix uninitialized one_time_gc in victim_sel_policy Xiaole He
2025-10-29 10:48 ` Chao Yu via Linux-f2fs-devel
2025-11-11 22:50 ` patchwork-bot+f2fs--- via Linux-f2fs-devel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).