[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@xxxxxxxxx>
---
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]) ==