linux-kernel - Re: [f2fs-dev] [PATCH v3] f2fs: prevent the current section from being selected as a victim during GC

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] [day] [month] [year] [list]

Message-ID: <20250403070151.2894-1-yohan.joung@sk.com>
Date: Thu,  3 Apr 2025 16:01:51 +0900
From: "yohan.joung" <yohan.joung@...com>
To: linux-f2fs-devel@...ts.sourceforge.net
Cc: chao@...nel.org,
	jaegeuk@...nel.org,
	linux-kernel@...r.kernel.org,
	pilhyun.kim@...com,
	yohan.joung@...com
Subject: Re: [f2fs-dev] [PATCH v3] f2fs: prevent the current section from being selected as a victim during GC

On Thu, 3 Apr 2025 14:26:59 +0800 Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@...ts.sourceforge.net> wrote:
> On 2025/4/2 16:04, yohan.joung wrote:
> > When selecting a victim using next_victim_seg in a large section, the
> > selected section might already have been cleared and designated as the
> > new current section, making it actively in use.
> > This behavior causes inconsistency between the SIT and SSA.
> > 
> > F2FS-fs (dm-54): Inconsistent segment (70961) type [0, 1] in SSA and SIT
> > Call trace:
> > dump_backtrace+0xe8/0x10c
> > show_stack+0x18/0x28
> > dump_stack_lvl+0x50/0x6c
> > dump_stack+0x18/0x28
> > f2fs_stop_checkpoint+0x1c/0x3c
> > do_garbage_collect+0x41c/0x271c
> > f2fs_gc+0x27c/0x828
> > gc_thread_func+0x290/0x88c
> > kthread+0x11c/0x164
> > ret_from_fork+0x10/0x20
> > 
> > issue scenario
> > segs_per_sec=2
> > - seg#0 and seg#1 are all dirty
> > - all valid blocks are removed in seg#1
> > - gc select this sec and next_victim_seg=seg#0
> > - migrate seg#0, next_victim_seg=seg#1
> > - checkpoint -> sec(seg#0, seg#1)  becomes free
> > - allocator assigns sec(seg#0, seg#1) to curseg
> > - gc tries to migrate seg#1
> > 
> > Signed-off-by: yohan.joung <yohan.joung@...com>
> > Signed-off-by: Chao Yu <chao@...nel.org>
> > ---
> >   fs/f2fs/segment.h | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > index 0465dc00b349..129df633d656 100644
> > --- a/fs/f2fs/segment.h
> > +++ b/fs/f2fs/segment.h
> > @@ -476,6 +476,12 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> >   		if (next >= start_segno + usable_segs) {
> >   			if (test_and_clear_bit(secno, free_i->free_secmap))
> >   				free_i->free_sections++;
> > +
> > +			if (GET_SEC_FROM_SEG(sbi->next_victim_seg[BG_GC]) == secno)
> > +				sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> > +
> > +			if (GET_SEC_FROM_SEG(sbi->next_victim_seg[FG_GC]) == secno)
> > +				sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> 
> Only need to reset next_victim_seg[] when setting section free?
Ah, I missed that. I'll put up the patch again right away.
Thanks. 
> 
> if (!test_and_clear_bit(secno, free_i->free_secmap))
> 	goto skip_free;
> 
> free_i->free_sections++;
> 
> if (GET_SEC_FROM_SEG(sbi->next_victim_seg[BG_GC]) == secno)
> 	sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> if (GET_SEC_FROM_SEG(sbi->next_victim_seg[FG_GC]) == secno)
> 	sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> 
> Thanks,
> 
> >   		}
> >   	}
> >   skip_free:
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel