linux-kernel - Re: [PATCH v5] 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: <87056483-6cf8-4b2e-82f1-dcda31a28afd@kernel.org>
Date: Mon, 7 Apr 2025 10:11:10 +0800
From: Chao Yu <chao@...nel.org>
To: "yohan.joung" <yohan.joung@...com>, Jaegeuk Kim <jaegeuk@...nel.org>
Cc: chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
 linux-kernel@...r.kernel.org, pilhyun.kim@...com
Subject: Re: [PATCH v5] f2fs: prevent the current section from being selected
 as a victim during GC

On 4/7/25 10:08, Chao Yu wrote:
> On 4/5/25 03:55, Jaegeuk Kim wrote:
>> Hi Yohan,
>>
>> I modified this patch after applying the clean up by
>>
>> https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
>>
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>>
>>         free_i->free_sections++;
>>
>> +       if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
>> +               sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>> +       if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
>> +               sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> 
> Reviewed-by: Chao Yu <chao@...nel.org>

Oh, can we add Fixes line to make it to be merged into stable kernel?

Thanks,

> 
> Thanks,
> 
>> +
>>  unlock_out:
>>         spin_unlock(&free_i->segmap_lock);
>>  }
>>
>> On 04/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 | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>> index 0465dc00b349..0773283babfa 100644
>>> --- a/fs/f2fs/segment.h
>>> +++ b/fs/f2fs/segment.h
>>> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>>>  		next = find_next_bit(free_i->free_segmap,
>>>  				start_segno + SEGS_PER_SEC(sbi), start_segno);
>>>  		if (next >= start_segno + usable_segs) {
>>> -			if (test_and_clear_bit(secno, free_i->free_secmap))
>>> +			if (test_and_clear_bit(secno, free_i->free_secmap)) {
>>>  				free_i->free_sections++;
>>> +
>>> +				if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
>>> +					sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>>> +
>>> +				if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
>>> +					sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>>> +			}
>>>  		}
>>>  	}
>>>  skip_free:
>>> -- 
>>> 2.33.0
>