In my test case, concurrent calls to f2fs shutdown report the following stack trace:
Oops: general protection fault, probably for non-canonical address 0xc6cfff63bb5513fc: 0000 [#1] PREEMPT SMP PTI CPU: 0 UID: 0 PID: 678 Comm: f2fs_rep_shutdo Not tainted 6.12.0-rc5-next-20241029-g6fb2fa9805c5-dirty #85 Call Trace: <TASK> ? show_regs+0x8b/0xa0 ? __die_body+0x26/0xa0 ? die_addr+0x54/0x90 ? exc_general_protection+0x24b/0x5c0 ? asm_exc_general_protection+0x26/0x30 ? kthread_stop+0x46/0x390 f2fs_stop_gc_thread+0x6c/0x110 f2fs_do_shutdown+0x309/0x3a0 f2fs_ioc_shutdown+0x150/0x1c0 __f2fs_ioctl+0xffd/0x2ac0 f2fs_ioctl+0x76/0xe0 vfs_ioctl+0x23/0x60 __x64_sys_ioctl+0xce/0xf0 x64_sys_call+0x2b1b/0x4540 do_syscall_64+0xa7/0x240 entry_SYSCALL_64_after_hwframe+0x76/0x7e The root cause is a race condition in f2fs_stop_gc_thread() called from different f2fs shutdown paths: [CPU0] [CPU1] ---------------------- ----------------------- f2fs_stop_gc_thread f2fs_stop_gc_thread gc_th = sbi->gc_thread gc_th = sbi->gc_thread kfree(gc_th) sbi->gc_thread = NULL < gc_th != NULL > kthread_stop(gc_th->f2fs_gc_task) //UAF The commit c7f114d864ac ("f2fs: fix to avoid use-after-free in f2fs_stop_gc_thread()") attempted to fix this issue by using a read semaphore to prevent races between shutdown and remount threads, but itfails to prevent all race conditions. While upgrading to s_umount write lock in f2fs_do_shutdown() would fix the current issue, however, using s_umount lock requires extreme caution to avoid lock recursion. A better solution is to introduce a semaphore to prevent races between concurrent f2fs_stop_gc_thread calls. Fixes: 7950e9ac638e ("f2fs: stop gc/discard thread after fs shutdown") Signed-off-by: Long Li <leo.lil...@huawei.com> --- fs/f2fs/f2fs.h | 1 + fs/f2fs/gc.c | 9 +++++++-- fs/f2fs/super.c | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 3c6f3cce5779..7ae1e2a4789f 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -1679,6 +1679,7 @@ struct f2fs_sb_info { * race between GC and GC or CP */ struct f2fs_gc_kthread *gc_thread; /* GC thread */ + struct semaphore gc_clean_lock; /* semaphore for clean GC thread */ struct atgc_management am; /* atgc management */ unsigned int cur_victim_sec; /* current victim section num */ unsigned int gc_mode; /* current GC state */ diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index e40bdd12e36d..e1b8bf98b5fa 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -232,14 +232,19 @@ int f2fs_start_gc_thread(struct f2fs_sb_info *sbi) void f2fs_stop_gc_thread(struct f2fs_sb_info *sbi) { - struct f2fs_gc_kthread *gc_th = sbi->gc_thread; + struct f2fs_gc_kthread *gc_th; - if (!gc_th) + down(&sbi->gc_clean_lock); + gc_th = sbi->gc_thread; + if (!gc_th) { + up(&sbi->gc_clean_lock); return; + } kthread_stop(gc_th->f2fs_gc_task); wake_up_all(&gc_th->fggc_wq); kfree(gc_th); sbi->gc_thread = NULL; + up(&sbi->gc_clean_lock); } static int select_gc_type(struct f2fs_sb_info *sbi, int gc_type) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 80a53dbf1c38..47a15050ea9c 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -4419,6 +4419,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) /* initialize locks within allocated memory */ init_f2fs_rwsem(&sbi->gc_lock); + sema_init(&sbi->gc_clean_lock, 1); mutex_init(&sbi->writepages); init_f2fs_rwsem(&sbi->cp_global_sem); init_f2fs_rwsem(&sbi->node_write); -- 2.39.2 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel