Re: [f2fs-dev] [PATCH v2] fsck.f2fs: sanitize invalid segment type during block update

[Chao Yu <chao@kernel.org>]

On 8/4/26 02:21, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> If an invalid segment type (type < 0 || type >= NO_CHECK_TYPE) is read
> from SIT during block update, update_block() now validates the type,
> reports corruption via ASSERT_MSG(), fixes se->type directly, and falls
> back to a valid active curseg type based on its SSA summary block type
> when fix_on is enabled. find_next_free_block() also validates want_type to
> prevent out-of-bounds accesses.
> 
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> ---
> v2: used ASSERT_MSG() instead of ERR_MSG().
>      used the correct type info for fixed_type.
>      fixed se->type on top of fixing the local type only.
> ---
>   fsck/mount.c   |  5 +++++
>   fsck/segment.c | 20 ++++++++++++++++++--
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 85ed404..916dfbf 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -3059,6 +3059,11 @@ int find_next_free_block(struct f2fs_sb_info *sbi, u64 *to, int left,
>   	u64 end_blkaddr = (get_sb(segment_count_main) <<
>   			get_sb(log_blocks_per_seg)) + get_sb(main_blkaddr);
>   
> +	if (want_type < 0 || want_type >= NO_CHECK_TYPE) {
> +		ASSERT_MSG("Invalid segment type %d for block allocation", want_type);
> +		return -EINVAL;
> +	}
> +
>   	if (c.zoned_model == F2FS_ZONED_HM && !new_sec) {
>   		struct curseg_info *curseg = CURSEG_I(sbi, want_type);
>   		unsigned int segs_per_zone = sbi->segs_per_sec * sbi->secs_per_zone;
> diff --git a/fsck/segment.c b/fsck/segment.c
> index 46dc747..43f85ec 100644
> --- a/fsck/segment.c
> +++ b/fsck/segment.c
> @@ -770,15 +770,32 @@ int update_block(struct f2fs_sb_info *sbi, void *buf, u32 *blkaddr,
>   	struct seg_entry *se;
>   	struct f2fs_summary sum;
>   	u64 new_blkaddr, old_blkaddr = *blkaddr, offset;
> -	int ret, type;
> +	int ret, type, sum_type;
>   
>   	if (c.zoned_model != F2FS_ZONED_HM)
>   		return dev_write_block(buf, old_blkaddr, WRITE_LIFE_NONE);
>   
> +	sum_type = get_sum_entry(sbi, old_blkaddr, &sum);
> +
>   	/* update sit bitmap & valid_blocks && se->type for old block*/
>   	se = get_seg_entry(sbi, GET_SEGNO(sbi, old_blkaddr));
>   	offset = OFFSET_IN_SEG(sbi, old_blkaddr);
>   	type = se->type;
> +	if (type < 0 || type >= NO_CHECK_TYPE) {
> +		int fixed_type = (sum_type == SEG_TYPE_NODE ||
> +				sum_type == SEG_TYPE_CUR_NODE) ?
> +				CURSEG_HOT_NODE : CURSEG_HOT_DATA;
> +
> +		ASSERT_MSG("Invalid SIT segment type %d at blkaddr 0x%"PRIx64,
> +				type, old_blkaddr);
> +
> +		if (!c.fix_on)
> +			return -EINVAL;
> +
> +		FIX_MSG("Correct invalid segment type %d -> %d for blkaddr 0x%"PRIx64"\n",
> +				type, fixed_type, old_blkaddr);
> +		se->type = se->orig_type = type = fixed_type;

We should not change se->orig_type, because fsck uses it to record original
se->type, so that in check_sit_types(), it can be compared w/ se->type to know
which se->type is fixed.

Thanks,

> +	}
>   	se->valid_blocks--;
>   	f2fs_clear_bit(offset, (char *)se->cur_valid_map);
>   	if (need_fsync_data_record(sbi))
> @@ -811,7 +828,6 @@ int update_block(struct f2fs_sb_info *sbi, void *buf, u32 *blkaddr,
>   	f2fs_set_sit_bitmap(sbi, new_blkaddr);
>   
>   	/* update SSA */
> -	get_sum_entry(sbi, old_blkaddr, &sum);
>   	update_sum_entry(sbi, new_blkaddr, &sum);
>   
>   	if (IS_DATASEG(type)) {