[PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature
From: Chao Yu
Date: Mon Apr 22 2019 - 05:35:13 EST
For large_nat_bitmap feature, there is a design flaw:
Previous:
struct f2fs_checkpoint layout:
+--------------------------+ 0x0000
| checkpoint_ver |
| ...... |
| checksum_offset |------+
| ...... | |
| sit_nat_version_bitmap[] |<-----|-------+
| ...... | | |
| checksum_value |<-----+ |
+--------------------------+ 0x1000 |
| | nat_bitmap + sit_bitmap
| payload blocks | |
| | |
+--------------------------|<-------------+
Obviously, if nat_bitmap size + sit_bitmap size is larger than
MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
checkpoint checksum's position, once checkpoint() is triggered
from kernel, nat or sit bitmap will be damaged by checksum field.
In order to fix this, let's relocate checksum_value's position
to the head of sit_nat_version_bitmap as below, then nat/sit
bitmap and chksum value update will become safe.
After:
struct f2fs_checkpoint layout:
+--------------------------+ 0x0000
| checkpoint_ver |
| ...... |
| checksum_offset |------+
| ...... | |
| sit_nat_version_bitmap[] |<-----+
| ...... |<-------------+
| | |
+--------------------------+ 0x1000 |
| | nat_bitmap + sit_bitmap
| payload blocks | |
| | |
+--------------------------|<-------------+
Related report and discussion:
https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/
In addition, during writing checkpoint, if large_nat_bitmap feature is
enabled, we need to set CP_LARGE_NAT_BITMAP_FLAG flag in checkpoint.
Reported-by: Park Ju Hyung <qkrwngud825@xxxxxxxxx>
Signed-off-by: Chao Yu <yuchao0@xxxxxxxxxx>
---
fsck/f2fs.h | 6 +++++-
fsck/fsck.c | 16 ++++++++++++++++
fsck/fsck.h | 1 +
fsck/main.c | 2 ++
fsck/mount.c | 42 +++++++++++++++++++++++++++---------------
include/f2fs_fs.h | 9 +++++++--
mkfs/f2fs_format.c | 5 ++++-
7 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/fsck/f2fs.h b/fsck/f2fs.h
index 93f01e5..6a6c4a5 100644
--- a/fsck/f2fs.h
+++ b/fsck/f2fs.h
@@ -272,7 +272,11 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
if (is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG)) {
offset = (flag == SIT_BITMAP) ?
le32_to_cpu(ckpt->nat_ver_bitmap_bytesize) : 0;
- return &ckpt->sit_nat_version_bitmap + offset;
+ /*
+ * if large_nat_bitmap feature is enabled, leave checksum
+ * protection for all nat/sit bitmaps.
+ */
+ return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
}
if (le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload) > 0) {
diff --git a/fsck/fsck.c b/fsck/fsck.c
index 7ecdee1..8d7deb5 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -1917,6 +1917,18 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
return 0;
}
+void fsck_chk_checkpoint(struct f2fs_sb_info *sbi)
+{
+ struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
+
+ if (get_cp(ckpt_flags) & CP_LARGE_NAT_BITMAP_FLAG) {
+ if (get_cp(checksum_offset) != CP_MIN_CHKSUM_OFFSET) {
+ ASSERT_MSG("Deprecated layout of large_nat_bitmap, "
+ "chksum_offset:%u", get_cp(checksum_offset));
+ }
+ }
+}
+
void fsck_init(struct f2fs_sb_info *sbi)
{
struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
@@ -2038,6 +2050,10 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
flags |= CP_TRIMMED_FLAG;
if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
flags |= CP_DISABLED_FLAG;
+ if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
+ flags |= CP_LARGE_NAT_BITMAP_FLAG;
+ set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
+ }
if (flags & CP_UMOUNT_FLAG)
cp_blocks = 8;
diff --git a/fsck/fsck.h b/fsck/fsck.h
index 376ced7..dd831de 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -154,6 +154,7 @@ extern int fsck_chk_dentry_blk(struct f2fs_sb_info *, u32, struct child_info *,
int, int);
int fsck_chk_inline_dentries(struct f2fs_sb_info *, struct f2fs_node *,
struct child_info *);
+void fsck_chk_checkpoint(struct f2fs_sb_info *sbi);
int fsck_chk_meta(struct f2fs_sb_info *sbi);
int fsck_chk_curseg_info(struct f2fs_sb_info *);
int convert_encrypted_name(unsigned char *, u32, unsigned char *, int);
diff --git a/fsck/main.c b/fsck/main.c
index d3f0c0d..e61bb91 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -616,6 +616,8 @@ static void do_fsck(struct f2fs_sb_info *sbi)
c.fix_on = 1;
}
+ fsck_chk_checkpoint(sbi);
+
fsck_chk_quota_node(sbi);
/* Traverse all block recursively from root inode */
diff --git a/fsck/mount.c b/fsck/mount.c
index da1d4c9..843742e 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -769,15 +769,33 @@ int init_sb_info(struct f2fs_sb_info *sbi)
return 0;
}
+static int verify_checksum_chksum(struct f2fs_checkpoint *cp)
+{
+ unsigned int chksum_offset = get_cp(checksum_offset);
+ unsigned int crc, cal_crc;
+
+ if (chksum_offset < CP_MIN_CHKSUM_OFFSET ||
+ chksum_offset > CP_CHKSUM_OFFSET) {
+ MSG(0, "\tInvalid CP CRC offset: %u\n", chksum_offset);
+ return -1;
+ }
+
+ crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + chksum_offset));
+ cal_crc = f2fs_checkpoint_chksum(cp);
+ if (cal_crc != crc) {
+ MSG(0, "\tInvalid CP CRC: offset:%u, crc:0x%x, calc:0x%x\n",
+ chksum_offset, crc, cal_crc);
+ return -1;
+ }
+ return 0;
+}
+
void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
unsigned long long *version)
{
void *cp_page_1, *cp_page_2;
struct f2fs_checkpoint *cp;
- unsigned long blk_size = sbi->blocksize;
unsigned long long cur_version = 0, pre_version = 0;
- unsigned int crc = 0;
- size_t crc_offset;
/* Read the 1st cp block in this CP pack */
cp_page_1 = malloc(PAGE_SIZE);
@@ -787,12 +805,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
goto invalid_cp1;
cp = (struct f2fs_checkpoint *)cp_page_1;
- crc_offset = get_cp(checksum_offset);
- if (crc_offset > (blk_size - sizeof(__le32)))
- goto invalid_cp1;
-
- crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
- if (f2fs_crc_valid(crc, cp, crc_offset))
+ if (verify_checksum_chksum(cp))
goto invalid_cp1;
if (get_cp(cp_pack_total_block_count) > sbi->blocks_per_seg)
@@ -810,12 +823,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
goto invalid_cp2;
cp = (struct f2fs_checkpoint *)cp_page_2;
- crc_offset = get_cp(checksum_offset);
- if (crc_offset > (blk_size - sizeof(__le32)))
- goto invalid_cp2;
-
- crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
- if (f2fs_crc_valid(crc, cp, crc_offset))
+ if (verify_checksum_chksum(cp))
goto invalid_cp2;
cur_version = get_cp(checkpoint_ver);
@@ -2365,6 +2373,10 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
flags |= CP_TRIMMED_FLAG;
if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
flags |= CP_DISABLED_FLAG;
+ if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
+ flags |= CP_LARGE_NAT_BITMAP_FLAG;
+ set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
+ }
set_cp(free_segment_count, get_free_segments(sbi));
set_cp(valid_block_count, sbi->total_valid_block_count);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 1bd935c..89b8a0c 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -692,10 +692,15 @@ struct f2fs_checkpoint {
unsigned char sit_nat_version_bitmap[1];
} __attribute__((packed));
+#define CP_BITMAP_OFFSET \
+ (offsetof(struct f2fs_checkpoint, sit_nat_version_bitmap))
+#define CP_MIN_CHKSUM_OFFSET CP_BITMAP_OFFSET
+
+#define MIN_NAT_BITMAP_SIZE 64
#define MAX_SIT_BITMAP_SIZE_IN_CKPT \
- (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
+ (CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET - MIN_NAT_BITMAP_SIZE)
#define MAX_BITMAP_SIZE_IN_CKPT \
- (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
+ (CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET)
/*
* For orphan inode management
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index a534293..eabffce 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -706,7 +706,10 @@ static int f2fs_write_check_point_pack(void)
set_cp(nat_ver_bitmap_bytesize, ((get_sb(segment_count_nat) / 2) <<
get_sb(log_blocks_per_seg)) / 8);
- set_cp(checksum_offset, CP_CHKSUM_OFFSET);
+ if (c.large_nat_bitmap)
+ set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
+ else
+ set_cp(checksum_offset, CP_CHKSUM_OFFSET);
crc = f2fs_checkpoint_chksum(cp);
*((__le32 *)((unsigned char *)cp + get_cp(checksum_offset))) =
--
2.18.0.rc1