Re: [f2fs-dev] [RFC PATCH v2 3/3] f2fs-tools: introduce sb checksum

2018-08-08 Thread Chao Yu 
On 2018/8/8 18:27, Junling Zheng wrote:
> This patch introduced superblock checksum.

It needs to update sb_chksum in sanity_check_area_boundary()?

To avoid missing updating sb_chksum, how about packaging a function to commit
superblock like what we did in kernel side, so that we can update chksum in
packaged function. IMO, we can add one other patch to do that before this one.

Thanks,

> 
> Signed-off-by: Junling Zheng 
> ---
> v1 -> v2:
>  - Rename CHKSUM_OFFSET_SB to SB_CHKSUM_OFFSET.
>  - Introduce update_sb_chksum() to update superblock checksum.
>  fsck/fsck.h|  1 +
>  fsck/mount.c   | 44 
>  fsck/resize.c  |  3 +++
>  include/f2fs_fs.h  |  6 +-
>  mkfs/f2fs_format.c |  9 +
>  5 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/fsck/fsck.h b/fsck/fsck.h
> index 6042e68..2172cac 100644
> --- a/fsck/fsck.h
> +++ b/fsck/fsck.h
> @@ -170,6 +170,7 @@ extern int fsck_verify(struct f2fs_sb_info *);
>  extern void fsck_free(struct f2fs_sb_info *);
>  extern int f2fs_do_mount(struct f2fs_sb_info *);
>  extern void f2fs_do_umount(struct f2fs_sb_info *);
> +extern void update_sb_chksum(struct f2fs_super_block *);
>  
>  extern void flush_journal_entries(struct f2fs_sb_info *);
>  extern void zero_journal_entries(struct f2fs_sb_info *);
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 58ef3e6..1dfabba 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
>   DISP_u32(sb, node_ino);
>   DISP_u32(sb, meta_ino);
>   DISP_u32(sb, cp_payload);
> + DISP_u32(sb, crc);
>   DISP("%-.256s", sb, version);
>   printf("\n");
>  }
> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>   if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
>   MSG(0, "%s", " lost_found");
>   }
> + if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
> + MSG(0, "%s", " sb_checksum");
> + }
>   MSG(0, "\n");
>   MSG(0, "Info: superblock encrypt level = %d, salt = ",
>   sb->encryption_level);
> @@ -558,10 +562,29 @@ static inline int sanity_check_area_boundary(struct 
> f2fs_super_block *sb,
>   return 0;
>  }
>  
> +static int verify_sb_chksum(struct f2fs_super_block *sb)
> +{
> + if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
> + MSG(0, "\tInvalid SB CRC offset: %u\n",
> + get_sb(checksum_offset));
> + return -1;
> + }
> + if (f2fs_crc_valid(get_sb(crc), sb,
> + get_sb(checksum_offset))) {
> + MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
> + return -1;
> + }
> + return 0;
> +}
> +
>  int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
>  {
>   unsigned int blocksize;
>  
> + if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
> + verify_sb_chksum(sb))
> + return -1;
> +
>   if (F2FS_SUPER_MAGIC != get_sb(magic))
>   return -1;
>  
> @@ -605,6 +628,18 @@ int sanity_check_raw_super(struct f2fs_super_block *sb, 
> u64 offset)
>   return 0;
>  }
>  
> +void update_sb_chksum(struct f2fs_super_block *sb)
> +{
> + u32 old_crc, new_crc;
> +
> + old_crc = get_sb(crc);
> + new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> + SB_CHKSUM_OFFSET);
> + set_sb(crc, new_crc);
> + MSG(0, "Info: update SB CRC successfully "
> + "(0x%x --> 0x%x)\n", old_crc, new_crc);
> +}
> +
>  int validate_super_block(struct f2fs_sb_info *sbi, int block)
>  {
>   u64 offset;
> @@ -646,6 +681,10 @@ int validate_super_block(struct f2fs_sb_info *sbi, int 
> block)
>  
>   memcpy(sbi->raw_super->version,
>   c.version, VERSION_LEN);
> + if (le32_to_cpu(sbi->raw_super->feature) &
> + F2FS_FEATURE_SB_CHKSUM)
> + update_sb_chksum(sbi->raw_super);
> +
>   ret = dev_write(sbi->raw_super, offset,
>   sizeof(struct f2fs_super_block));
>   ASSERT(ret >= 0);
> @@ -2402,6 +2441,9 @@ static int check_sector_size(struct f2fs_super_block 
> *sb)
>   set_sb(log_sectorsize, log_sectorsize);
>   set_sb(log_sectors_per_block, log_sectors_per_block);
>  
> + if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)
> + update_sb_chksum(sb);
> +
>   memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>   DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
>   for (index = 0; index < 2; index++) {
> @@ -2434,6 +2476,8 @@ static void tune_sb_features(struct f2fs_sb_info *sbi)
>   if (!sb_changed)
>   return;
>  
> + if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)

[f2fs-dev] [RFC PATCH v2 3/3] f2fs-tools: introduce sb checksum

2018-08-08 Thread Junling Zheng 
This patch introduced superblock checksum.

Signed-off-by: Junling Zheng 
---
v1 -> v2:
 - Rename CHKSUM_OFFSET_SB to SB_CHKSUM_OFFSET.
 - Introduce update_sb_chksum() to update superblock checksum.
 fsck/fsck.h|  1 +
 fsck/mount.c   | 44 
 fsck/resize.c  |  3 +++
 include/f2fs_fs.h  |  6 +-
 mkfs/f2fs_format.c |  9 +
 5 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/fsck/fsck.h b/fsck/fsck.h
index 6042e68..2172cac 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -170,6 +170,7 @@ extern int fsck_verify(struct f2fs_sb_info *);
 extern void fsck_free(struct f2fs_sb_info *);
 extern int f2fs_do_mount(struct f2fs_sb_info *);
 extern void f2fs_do_umount(struct f2fs_sb_info *);
+extern void update_sb_chksum(struct f2fs_super_block *);
 
 extern void flush_journal_entries(struct f2fs_sb_info *);
 extern void zero_journal_entries(struct f2fs_sb_info *);
diff --git a/fsck/mount.c b/fsck/mount.c
index 58ef3e6..1dfabba 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
DISP_u32(sb, node_ino);
DISP_u32(sb, meta_ino);
DISP_u32(sb, cp_payload);
+   DISP_u32(sb, crc);
DISP("%-.256s", sb, version);
printf("\n");
 }
@@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
MSG(0, "%s", " lost_found");
}
+   if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
+   MSG(0, "%s", " sb_checksum");
+   }
MSG(0, "\n");
MSG(0, "Info: superblock encrypt level = %d, salt = ",
sb->encryption_level);
@@ -558,10 +562,29 @@ static inline int sanity_check_area_boundary(struct 
f2fs_super_block *sb,
return 0;
 }
 
+static int verify_sb_chksum(struct f2fs_super_block *sb)
+{
+   if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
+   MSG(0, "\tInvalid SB CRC offset: %u\n",
+   get_sb(checksum_offset));
+   return -1;
+   }
+   if (f2fs_crc_valid(get_sb(crc), sb,
+   get_sb(checksum_offset))) {
+   MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
+   return -1;
+   }
+   return 0;
+}
+
 int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
 {
unsigned int blocksize;
 
+   if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
+   verify_sb_chksum(sb))
+   return -1;
+
if (F2FS_SUPER_MAGIC != get_sb(magic))
return -1;
 
@@ -605,6 +628,18 @@ int sanity_check_raw_super(struct f2fs_super_block *sb, 
u64 offset)
return 0;
 }
 
+void update_sb_chksum(struct f2fs_super_block *sb)
+{
+   u32 old_crc, new_crc;
+
+   old_crc = get_sb(crc);
+   new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
+   SB_CHKSUM_OFFSET);
+   set_sb(crc, new_crc);
+   MSG(0, "Info: update SB CRC successfully "
+   "(0x%x --> 0x%x)\n", old_crc, new_crc);
+}
+
 int validate_super_block(struct f2fs_sb_info *sbi, int block)
 {
u64 offset;
@@ -646,6 +681,10 @@ int validate_super_block(struct f2fs_sb_info *sbi, int 
block)
 
memcpy(sbi->raw_super->version,
c.version, VERSION_LEN);
+   if (le32_to_cpu(sbi->raw_super->feature) &
+   F2FS_FEATURE_SB_CHKSUM)
+   update_sb_chksum(sbi->raw_super);
+
ret = dev_write(sbi->raw_super, offset,
sizeof(struct f2fs_super_block));
ASSERT(ret >= 0);
@@ -2402,6 +2441,9 @@ static int check_sector_size(struct f2fs_super_block *sb)
set_sb(log_sectorsize, log_sectorsize);
set_sb(log_sectors_per_block, log_sectors_per_block);
 
+   if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)
+   update_sb_chksum(sb);
+
memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
for (index = 0; index < 2; index++) {
@@ -2434,6 +2476,8 @@ static void tune_sb_features(struct f2fs_sb_info *sbi)
if (!sb_changed)
return;
 
+   if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)
+   update_sb_chksum(sb);
write_superblock(sb);
 }
 
diff --git a/fsck/resize.c b/fsck/resize.c
index e9612b3..ca8c6f9 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -160,6 +160,9 @@ safe_resize:
(get_sb(segment_count_main) - 2));
return -1;
}
+   if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)
+   update_sb_chksum(sb);
+
return 0;
 }
 
diff --git a/include/f2fs_fs.h