[PATCH] f2fs: check every cur_*_segno[] entry on readonly images

[PATCH] f2fs: check every cur_*_segno[] entry on readonly images

[Kaixuan Li]

sanity_check_ckpt() bounds-checks cur_node_segno[] and cur_data_segno[]
against main_segs, but on an image with the RO feature the goto that skips
the duplicate-segno cross-checks also leaves the enclosing loop, so only
index 0 of each array is ever checked.

An unchecked index reaches __set_sit_entry_type() through build_curseg() ->
reset_curseg(), which writes se->type through &sit_i->sentries[segno]. With
segment_count_main 120 and cur_data_segno[1] set to 200, KASAN reports
three out-of-bounds writes during mount and the mount then succeeds.

Use continue so every index is bounds-checked, and skip the node-vs-data
cross-check explicitly, which is the other thing the old goto skipped.

Fixes: a7d9fe3c3388 ("f2fs: support RO feature")
Signed-off-by: Kaixuan Li <kaixuanli0131@gmail.com>
---
Found on v7.2.4; the code is unchanged in mainline at commit
5dd1818b15d98d4a20806cd00b1b40320b06004f.

Before the patch, mounting such an image on v7.2.4 with KASAN:

	BUG: KASAN: slab-out-of-bounds in reset_curseg+0x48a/0x530
	Write of size 4 at addr ffff888009f8df40 by task init/1
	 reset_curseg+0x48a/0x530
	 f2fs_build_segment_manager+0x1f65/0x8de0
	 f2fs_fill_super+0x4348/0x7a10
	 vfs_get_tree+0x83/0x2f0
	 path_mount+0x570/0x1fb0
	The buggy address is located 3200 bytes to the right of
	 allocated 4800-byte region [ffff888009f8c000, ffff888009f8d2c0)

The 4800-byte region is sentries[], 120 entries. Two further writes of
size 8 follow at f2fs_build_segment_manager+0x479f and +0x480e, so
__set_sit_entry_type() is not the only sink the unchecked value reaches.
The mount completes afterwards, so nothing surfaces the corruption.

Reproducer: mkfs.f2fs on a 256 MB image gives segment_count_main = 120.
Then, in both superblock copies (offset 1024 and 1024 + F2FS_BLKSIZE) and
both checkpoint packs, set F2FS_FEATURE_RO (0x4000) in the feature word and
set cur_data_segno[1] and cur_node_segno[1] to 200; recompute each pack's
crc32 over checksum_offset bytes seeded with F2FS_SUPER_MAGIC and store it
at checksum_offset. Mount read-only. Scripts available on request.

Tested on v7.2.4 x86_64 with KASAN, three cases:

  - crafted image as above: rejected at checkpoint validation, no KASAN
    report (before the patch: three reports, and the mount succeeded);
  - plain mkfs.f2fs image: still mounts;
  - RO feature set with valid segnos: still mounts, which is why the
    node-vs-data cross-check is skipped explicitly rather than by letting
    the loop fall through to it.

fs/f2fs/super.c builds warning-free and checkpatch is clean apart from
"Unknown commit id" for the Fixes: tag, which is an artefact of the tree I
built in rather than the tag.
---
 fs/f2fs/super.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4239,7 +4239,7 @@
 			return 1;
 
 		if (f2fs_sb_has_readonly(sbi))
-			goto check_data;
+			continue;
 
 		for (j = i + 1; j < NR_CURSEG_NODE_TYPE; j++) {
 			if (le32_to_cpu(ckpt->cur_node_segno[i]) ==
@@ -4251,14 +4251,14 @@
 			}
 		}
 	}
-check_data:
+
 	for (i = 0; i < NR_CURSEG_DATA_TYPE; i++) {
 		if (le32_to_cpu(ckpt->cur_data_segno[i]) >= main_segs ||
 			le16_to_cpu(ckpt->cur_data_blkoff[i]) >= blocks_per_seg)
 			return 1;
 
 		if (f2fs_sb_has_readonly(sbi))
-			goto skip_cross;
+			continue;
 
 		for (j = i + 1; j < NR_CURSEG_DATA_TYPE; j++) {
 			if (le32_to_cpu(ckpt->cur_data_segno[i]) ==
@@ -4270,6 +4270,9 @@
 			}
 		}
 	}
+	if (f2fs_sb_has_readonly(sbi))
+		goto skip_cross;
+
 	for (i = 0; i < NR_CURSEG_NODE_TYPE; i++) {
 		for (j = 0; j < NR_CURSEG_DATA_TYPE; j++) {
 			if (le32_to_cpu(ckpt->cur_node_segno[i]) ==

Re: [PATCH] f2fs: check every cur_*_segno[] entry on readonly images

[Chao Yu]

On 9/19/26 00:55, Kaixuan Li wrote:
> sanity_check_ckpt() bounds-checks cur_node_segno[] and cur_data_segno[]
> against main_segs, but on an image with the RO feature the goto that skips
> the duplicate-segno cross-checks also leaves the enclosing loop, so only
> index 0 of each array is ever checked.
> 
> An unchecked index reaches __set_sit_entry_type() through build_curseg() ->
> reset_curseg(), which writes se->type through &sit_i->sentries[segno]. With
> segment_count_main 120 and cur_data_segno[1] set to 200, KASAN reports
> three out-of-bounds writes during mount and the mount then succeeds.
> 
> Use continue so every index is bounds-checked, and skip the node-vs-data
> cross-check explicitly, which is the other thing the old goto skipped.
> 

Cc: stable@kernel.org

> Fixes: a7d9fe3c3388 ("f2fs: support RO feature")
> Signed-off-by: Kaixuan Li <kaixuanli0131@gmail.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH] f2fs: check every cur_*_segno[] entry on readonly images

[patchwork-bot+f2fs]

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Sat, 19 Sep 2026 00:55:07 +0800 you wrote:
> sanity_check_ckpt() bounds-checks cur_node_segno[] and cur_data_segno[]
> against main_segs, but on an image with the RO feature the goto that skips
> the duplicate-segno cross-checks also leaves the enclosing loop, so only
> index 0 of each array is ever checked.
> 
> An unchecked index reaches __set_sit_entry_type() through build_curseg() ->
> reset_curseg(), which writes se->type through &sit_i->sentries[segno]. With
> segment_count_main 120 and cur_data_segno[1] set to 200, KASAN reports
> three out-of-bounds writes during mount and the mount then succeeds.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev] f2fs: check every cur_*_segno[] entry on readonly images
    https://git.kernel.org/jaegeuk/f2fs/c/9c4ee79190c8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html