linux-kernel - 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: <20250402080015.2794-1-yohan.joung@sk.com>
Date: Wed,  2 Apr 2025 17:00:14 +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: f2fs: prevent the current section from being selected as a victim during GC

>On 4/2/25 08:52, 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..14d18bcf3559 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -460,6 +460,7 @@ static inline void __set_test_and_free(struct
>f2fs_sb_info *sbi,
>>  		unsigned int segno, bool inmem)
>>  {
>>  	struct free_segmap_info *free_i = FREE_I(sbi);
>> +	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>>  	unsigned int secno = GET_SEC_FROM_SEG(sbi, segno);
>>  	unsigned int start_segno = GET_SEG_FROM_SEC(sbi, secno);
>>  	unsigned int next;
>> @@ -476,6 +477,11 @@ 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 (test_and_clear_bit(secno, dirty_i->victim_secmap))
>{
>> +				sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>> +				sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>
>sbi->next_victim_seg[FG_GC] relies on sbi->cur_victim_sec?
right  
>
>If sbi->next_victim_seg[BG_GC] is not equal to secno, will we still need to
>nullify sbi->next_victim_seg[BG_GC]?
Changed to remove only when equal
>
>We have cleared bit in victim_secmap after we tag a section as prefree,
>right?
You are right 
change victim_secmap to not be used
In the end, I think I'll have to do this to directly compare segno, 
as you suggested.
I tested this and it worked fine.

Thanks
>
>- locate_dirty_segment
> - __locate_dirty_segment
> - __remove_dirty_segment
>  - clear_bit(GET_SEC_FROM_SEG(sbi, segno), dirty_i->victim_secmap);
>
>Thanks,
>
>> +			}
>>  		}
>>  	}
>>  skip_free: