[PATCH 1/4] exfat: redefine PBR as boot_sector

[PATCH 1/4] exfat: redefine PBR as boot_sector

[Tetsuhiro Kohada]

Aggregate PBR related definitions and redefine as "boot_sector" to comply
with the exFAT specification.
And, rename variable names including 'pbr'.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
 fs/exfat/exfat_fs.h  |  2 +-
 fs/exfat/exfat_raw.h | 79 +++++++++++++++--------------------------
 fs/exfat/super.c     | 84 ++++++++++++++++++++++----------------------
 3 files changed, 72 insertions(+), 93 deletions(-)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 294aa7792bc3..b0e5b9afc56c 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -231,7 +231,7 @@ struct exfat_sb_info {
 	unsigned int root_dir; /* root dir cluster */
 	unsigned int dentries_per_clu; /* num of dentries per cluster */
 	unsigned int vol_flag; /* volume dirty flag */
-	struct buffer_head *pbr_bh; /* buffer_head of PBR sector */
+	struct buffer_head *boot_bh; /* buffer_head of BOOT sector */
 
 	unsigned int map_clu; /* allocation bitmap start cluster */
 	unsigned int map_sectors; /* num of allocation bitmap sectors */
diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h
index 8d6c64a7546d..b373dc4e099f 100644
--- a/fs/exfat/exfat_raw.h
+++ b/fs/exfat/exfat_raw.h
@@ -8,7 +8,8 @@
 
 #include <linux/types.h>
 
-#define PBR_SIGNATURE		0xAA55
+#define BOOT_SIGNATURE		0xAA55
+#define EXBOOT_SIGNATURE	0xAA550000
 
 #define EXFAT_MAX_FILE_LEN	255
 
@@ -55,7 +56,7 @@
 
 /* checksum types */
 #define CS_DIR_ENTRY		0
-#define CS_PBR_SECTOR		1
+#define CS_BOOT_SECTOR		1
 #define CS_DEFAULT		2
 
 /* file attributes */
@@ -69,57 +70,35 @@
 #define ATTR_RWMASK		(ATTR_HIDDEN | ATTR_SYSTEM | ATTR_VOLUME | \
 				 ATTR_SUBDIR | ATTR_ARCHIVE)
 
-#define PBR64_JUMP_BOOT_LEN		3
-#define PBR64_OEM_NAME_LEN		8
-#define PBR64_RESERVED_LEN		53
+#define BOOTSEC_JUMP_BOOT_LEN		3
+#define BOOTSEC_OEM_NAME_LEN		8
+#define BOOTSEC_OLDBPB_LEN		53
 
 #define EXFAT_FILE_NAME_LEN		15
 
-/* EXFAT BIOS parameter block (64 bytes) */
-struct bpb64 {
-	__u8 jmp_boot[PBR64_JUMP_BOOT_LEN];
-	__u8 oem_name[PBR64_OEM_NAME_LEN];
-	__u8 res_zero[PBR64_RESERVED_LEN];
-} __packed;
-
-/* EXFAT EXTEND BIOS parameter block (56 bytes) */
-struct bsx64 {
-	__le64 vol_offset;
-	__le64 vol_length;
-	__le32 fat_offset;
-	__le32 fat_length;
-	__le32 clu_offset;
-	__le32 clu_count;
-	__le32 root_cluster;
-	__le32 vol_serial;
-	__u8 fs_version[2];
-	__le16 vol_flags;
-	__u8 sect_size_bits;
-	__u8 sect_per_clus_bits;
-	__u8 num_fats;
-	__u8 phy_drv_no;
-	__u8 perc_in_use;
-	__u8 reserved2[7];
-} __packed;
-
-/* EXFAT PBR[BPB+BSX] (120 bytes) */
-struct pbr64 {
-	struct bpb64 bpb;
-	struct bsx64 bsx;
-} __packed;
-
-/* Common PBR[Partition Boot Record] (512 bytes) */
-struct pbr {
-	union {
-		__u8 raw[64];
-		struct bpb64 f64;
-	} bpb;
-	union {
-		__u8 raw[56];
-		struct bsx64 f64;
-	} bsx;
-	__u8 boot_code[390];
-	__le16 signature;
+/* EXFAT: Main and Backup Boot Sector (512 bytes) */
+struct boot_sector {
+	__u8	jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
+	__u8	oem_name[BOOTSEC_OEM_NAME_LEN];
+	__u8	must_be_zero[BOOTSEC_OLDBPB_LEN];
+	__le64	partition_offset;
+	__le64	vol_length;
+	__le32	fat_offset;
+	__le32	fat_length;
+	__le32	clu_offset;
+	__le32	clu_count;
+	__le32	root_cluster;
+	__le32	vol_serial;
+	__u8	fs_revision[2];
+	__le16	vol_flags;
+	__u8	sect_size_bits;
+	__u8	sect_per_clus_bits;
+	__u8	num_fats;
+	__u8	drv_sel;
+	__u8	percent_in_use;
+	__u8	reserved[7];
+	__u8	boot_code[390];
+	__le16	signature;
 } __packed;
 
 struct exfat_dentry {
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index c1f47f4071a8..e60d28e73ff0 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -49,7 +49,7 @@ static void exfat_put_super(struct super_block *sb)
 		sync_blockdev(sb->s_bdev);
 	exfat_set_vol_flags(sb, VOL_CLEAN);
 	exfat_free_bitmap(sbi);
-	brelse(sbi->pbr_bh);
+	brelse(sbi->boot_bh);
 	mutex_unlock(&sbi->s_lock);
 
 	call_rcu(&sbi->rcu, exfat_delayed_free);
@@ -101,7 +101,7 @@ static int exfat_statfs(struct dentry *dentry, struct kstatfs *buf)
 int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flag)
 {
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
-	struct pbr64 *bpb = (struct pbr64 *)sbi->pbr_bh->b_data;
+	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
 	bool sync;
 
 	/* flags are not changed */
@@ -116,18 +116,18 @@ int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flag)
 	if (sb_rdonly(sb))
 		return 0;
 
-	bpb->bsx.vol_flags = cpu_to_le16(new_flag);
+	p_boot->vol_flags = cpu_to_le16(new_flag);
 
-	if (new_flag == VOL_DIRTY && !buffer_dirty(sbi->pbr_bh))
+	if (new_flag == VOL_DIRTY && !buffer_dirty(sbi->boot_bh))
 		sync = true;
 	else
 		sync = false;
 
-	set_buffer_uptodate(sbi->pbr_bh);
-	mark_buffer_dirty(sbi->pbr_bh);
+	set_buffer_uptodate(sbi->boot_bh);
+	mark_buffer_dirty(sbi->boot_bh);
 
 	if (sync)
-		sync_dirty_buffer(sbi->pbr_bh);
+		sync_dirty_buffer(sbi->boot_bh);
 	return 0;
 }
 
@@ -366,13 +366,14 @@ static int exfat_read_root(struct inode *inode)
 	return 0;
 }
 
-static struct pbr *exfat_read_pbr_with_logical_sector(struct super_block *sb)
+static struct boot_sector *exfat_read_boot_with_logical_sector(
+		struct super_block *sb)
 {
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
-	struct pbr *p_pbr = (struct pbr *) (sbi->pbr_bh)->b_data;
+	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
 	unsigned short logical_sect = 0;
 
-	logical_sect = 1 << p_pbr->bsx.f64.sect_size_bits;
+	logical_sect = 1 << p_boot->sect_size_bits;
 
 	if (!is_power_of_2(logical_sect) ||
 	    logical_sect < 512 || logical_sect > 4096) {
@@ -387,49 +388,48 @@ static struct pbr *exfat_read_pbr_with_logical_sector(struct super_block *sb)
 	}
 
 	if (logical_sect > sb->s_blocksize) {
-		brelse(sbi->pbr_bh);
-		sbi->pbr_bh = NULL;
+		brelse(sbi->boot_bh);
+		sbi->boot_bh = NULL;
 
 		if (!sb_set_blocksize(sb, logical_sect)) {
 			exfat_err(sb, "unable to set blocksize %u",
 				  logical_sect);
 			return NULL;
 		}
-		sbi->pbr_bh = sb_bread(sb, 0);
-		if (!sbi->pbr_bh) {
+		sbi->boot_bh = sb_bread(sb, 0);
+		if (!sbi->boot_bh) {
 			exfat_err(sb, "unable to read boot sector (logical sector size = %lu)",
 				  sb->s_blocksize);
 			return NULL;
 		}
 
-		p_pbr = (struct pbr *)sbi->pbr_bh->b_data;
+		p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
 	}
-	return p_pbr;
+	return p_boot;
 }
 
 /* mount the file system volume */
 static int __exfat_fill_super(struct super_block *sb)
 {
 	int ret;
-	struct pbr *p_pbr;
-	struct pbr64 *p_bpb;
+	struct boot_sector *p_boot;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 
 	/* set block size to read super block */
 	sb_min_blocksize(sb, 512);
 
 	/* read boot sector */
-	sbi->pbr_bh = sb_bread(sb, 0);
-	if (!sbi->pbr_bh) {
+	sbi->boot_bh = sb_bread(sb, 0);
+	if (!sbi->boot_bh) {
 		exfat_err(sb, "unable to read boot sector");
 		return -EIO;
 	}
 
 	/* PRB is read */
-	p_pbr = (struct pbr *)sbi->pbr_bh->b_data;
+	p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
 
-	/* check the validity of PBR */
-	if (le16_to_cpu((p_pbr->signature)) != PBR_SIGNATURE) {
+	/* check the validity of BOOT */
+	if (le16_to_cpu((p_boot->signature)) != BOOT_SIGNATURE) {
 		exfat_err(sb, "invalid boot record signature");
 		ret = -EINVAL;
 		goto free_bh;
@@ -437,8 +437,8 @@ static int __exfat_fill_super(struct super_block *sb)
 
 
 	/* check logical sector size */
-	p_pbr = exfat_read_pbr_with_logical_sector(sb);
-	if (!p_pbr) {
+	p_boot = exfat_read_boot_with_logical_sector(sb);
+	if (!p_boot) {
 		ret = -EIO;
 		goto free_bh;
 	}
@@ -447,43 +447,43 @@ static int __exfat_fill_super(struct super_block *sb)
 	 * res_zero field must be filled with zero to prevent mounting
 	 * from FAT volume.
 	 */
-	if (memchr_inv(p_pbr->bpb.f64.res_zero, 0,
-			sizeof(p_pbr->bpb.f64.res_zero))) {
+	if (memchr_inv(p_boot->must_be_zero, 0,
+			sizeof(p_boot->must_be_zero))) {
 		ret = -EINVAL;
 		goto free_bh;
 	}
 
-	p_bpb = (struct pbr64 *)p_pbr;
-	if (!p_bpb->bsx.num_fats) {
+	p_boot = (struct boot_sector *)p_boot;
+	if (!p_boot->num_fats) {
 		exfat_err(sb, "bogus number of FAT structure");
 		ret = -EINVAL;
 		goto free_bh;
 	}
 
-	sbi->sect_per_clus = 1 << p_bpb->bsx.sect_per_clus_bits;
-	sbi->sect_per_clus_bits = p_bpb->bsx.sect_per_clus_bits;
+	sbi->sect_per_clus = 1 << p_boot->sect_per_clus_bits;
+	sbi->sect_per_clus_bits = p_boot->sect_per_clus_bits;
 	sbi->cluster_size_bits = sbi->sect_per_clus_bits + sb->s_blocksize_bits;
 	sbi->cluster_size = 1 << sbi->cluster_size_bits;
-	sbi->num_FAT_sectors = le32_to_cpu(p_bpb->bsx.fat_length);
-	sbi->FAT1_start_sector = le32_to_cpu(p_bpb->bsx.fat_offset);
-	sbi->FAT2_start_sector = p_bpb->bsx.num_fats == 1 ?
+	sbi->num_FAT_sectors = le32_to_cpu(p_boot->fat_length);
+	sbi->FAT1_start_sector = le32_to_cpu(p_boot->fat_offset);
+	sbi->FAT2_start_sector = p_boot->num_fats == 1 ?
 		sbi->FAT1_start_sector :
 			sbi->FAT1_start_sector + sbi->num_FAT_sectors;
-	sbi->data_start_sector = le32_to_cpu(p_bpb->bsx.clu_offset);
-	sbi->num_sectors = le64_to_cpu(p_bpb->bsx.vol_length);
+	sbi->data_start_sector = le32_to_cpu(p_boot->clu_offset);
+	sbi->num_sectors = le64_to_cpu(p_boot->vol_length);
 	/* because the cluster index starts with 2 */
-	sbi->num_clusters = le32_to_cpu(p_bpb->bsx.clu_count) +
+	sbi->num_clusters = le32_to_cpu(p_boot->clu_count) +
 		EXFAT_RESERVED_CLUSTERS;
 
-	sbi->root_dir = le32_to_cpu(p_bpb->bsx.root_cluster);
+	sbi->root_dir = le32_to_cpu(p_boot->root_cluster);
 	sbi->dentries_per_clu = 1 <<
 		(sbi->cluster_size_bits - DENTRY_SIZE_BITS);
 
-	sbi->vol_flag = le16_to_cpu(p_bpb->bsx.vol_flags);
+	sbi->vol_flag = le16_to_cpu(p_boot->vol_flags);
 	sbi->clu_srch_ptr = EXFAT_FIRST_CLUSTER;
 	sbi->used_clusters = EXFAT_CLUSTERS_UNTRACKED;
 
-	if (le16_to_cpu(p_bpb->bsx.vol_flags) & VOL_DIRTY) {
+	if (le16_to_cpu(p_boot->vol_flags) & VOL_DIRTY) {
 		sbi->vol_flag |= VOL_DIRTY;
 		exfat_warn(sb, "Volume was not properly unmounted. Some data may be corrupt. Please run fsck.");
 	}
@@ -517,7 +517,7 @@ static int __exfat_fill_super(struct super_block *sb)
 free_upcase_table:
 	exfat_free_upcase_table(sbi);
 free_bh:
-	brelse(sbi->pbr_bh);
+	brelse(sbi->boot_bh);
 	return ret;
 }
 
@@ -608,7 +608,7 @@ static int exfat_fill_super(struct super_block *sb, struct fs_context *fc)
 free_table:
 	exfat_free_upcase_table(sbi);
 	exfat_free_bitmap(sbi);
-	brelse(sbi->pbr_bh);
+	brelse(sbi->boot_bh);
 
 check_nls_io:
 	unload_nls(sbi->nls_io);
-- 
2.25.1

[PATCH 2/4] exfat: separate the boot sector analysis

[Tetsuhiro Kohada]

Separate the boot sector analysis to read_boot_sector().
Furthermore, add a strict consistency check, because overlapping areas
can cause serious corruption.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
 fs/exfat/exfat_raw.h |  1 +
 fs/exfat/super.c     | 96 +++++++++++++++++++++++---------------------
 2 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h
index b373dc4e099f..65f884785192 100644
--- a/fs/exfat/exfat_raw.h
+++ b/fs/exfat/exfat_raw.h
@@ -15,6 +15,7 @@
 
 #define VOL_CLEAN		0x0000
 #define VOL_DIRTY		0x0002
+#define ERR_MEDIUM		0x0004
 
 #define EXFAT_EOF_CLUSTER	0xFFFFFFFFu
 #define EXFAT_BAD_CLUSTER	0xFFFFFFF7u
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index e60d28e73ff0..95909b4d5e75 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -366,25 +366,20 @@ static int exfat_read_root(struct inode *inode)
 	return 0;
 }
 
-static struct boot_sector *exfat_read_boot_with_logical_sector(
-		struct super_block *sb)
+static int exfat_calibrate_blocksize(struct super_block *sb, int logical_sect)
 {
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
-	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
-	unsigned short logical_sect = 0;
-
-	logical_sect = 1 << p_boot->sect_size_bits;
 
 	if (!is_power_of_2(logical_sect) ||
 	    logical_sect < 512 || logical_sect > 4096) {
 		exfat_err(sb, "bogus logical sector size %u", logical_sect);
-		return NULL;
+		return -EIO;
 	}
 
 	if (logical_sect < sb->s_blocksize) {
 		exfat_err(sb, "logical sector size too small for device (logical sector size = %u)",
 			  logical_sect);
-		return NULL;
+		return -EIO;
 	}
 
 	if (logical_sect > sb->s_blocksize) {
@@ -394,24 +389,20 @@ static struct boot_sector *exfat_read_boot_with_logical_sector(
 		if (!sb_set_blocksize(sb, logical_sect)) {
 			exfat_err(sb, "unable to set blocksize %u",
 				  logical_sect);
-			return NULL;
+			return -EIO;
 		}
 		sbi->boot_bh = sb_bread(sb, 0);
 		if (!sbi->boot_bh) {
 			exfat_err(sb, "unable to read boot sector (logical sector size = %lu)",
 				  sb->s_blocksize);
-			return NULL;
+			return -EIO;
 		}
-
-		p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
 	}
-	return p_boot;
+	return 0;
 }
 
-/* mount the file system volume */
-static int __exfat_fill_super(struct super_block *sb)
+static int exfat_read_boot_sector(struct super_block *sb)
 {
-	int ret;
 	struct boot_sector *p_boot;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 
@@ -424,51 +415,36 @@ static int __exfat_fill_super(struct super_block *sb)
 		exfat_err(sb, "unable to read boot sector");
 		return -EIO;
 	}
-
-	/* PRB is read */
 	p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
 
 	/* check the validity of BOOT */
 	if (le16_to_cpu((p_boot->signature)) != BOOT_SIGNATURE) {
 		exfat_err(sb, "invalid boot record signature");
-		ret = -EINVAL;
-		goto free_bh;
-	}
-
-
-	/* check logical sector size */
-	p_boot = exfat_read_boot_with_logical_sector(sb);
-	if (!p_boot) {
-		ret = -EIO;
-		goto free_bh;
+		return -EINVAL;
 	}
 
 	/*
-	 * res_zero field must be filled with zero to prevent mounting
+	 * must_be_zero field must be filled with zero to prevent mounting
 	 * from FAT volume.
 	 */
-	if (memchr_inv(p_boot->must_be_zero, 0,
-			sizeof(p_boot->must_be_zero))) {
-		ret = -EINVAL;
-		goto free_bh;
-	}
+	if (memchr_inv(p_boot->must_be_zero, 0, sizeof(p_boot->must_be_zero)))
+		return -EINVAL;
 
-	p_boot = (struct boot_sector *)p_boot;
-	if (!p_boot->num_fats) {
+	if (p_boot->num_fats != 1 && p_boot->num_fats != 2) {
 		exfat_err(sb, "bogus number of FAT structure");
-		ret = -EINVAL;
-		goto free_bh;
+		return -EINVAL;
 	}
 
 	sbi->sect_per_clus = 1 << p_boot->sect_per_clus_bits;
 	sbi->sect_per_clus_bits = p_boot->sect_per_clus_bits;
-	sbi->cluster_size_bits = sbi->sect_per_clus_bits + sb->s_blocksize_bits;
+	sbi->cluster_size_bits = p_boot->sect_per_clus_bits +
+		p_boot->sect_size_bits;
 	sbi->cluster_size = 1 << sbi->cluster_size_bits;
 	sbi->num_FAT_sectors = le32_to_cpu(p_boot->fat_length);
 	sbi->FAT1_start_sector = le32_to_cpu(p_boot->fat_offset);
-	sbi->FAT2_start_sector = p_boot->num_fats == 1 ?
-		sbi->FAT1_start_sector :
-			sbi->FAT1_start_sector + sbi->num_FAT_sectors;
+	sbi->FAT2_start_sector = le32_to_cpu(p_boot->fat_offset);
+	if (p_boot->num_fats == 2)
+		sbi->FAT2_start_sector += sbi->num_FAT_sectors;
 	sbi->data_start_sector = le32_to_cpu(p_boot->clu_offset);
 	sbi->num_sectors = le64_to_cpu(p_boot->vol_length);
 	/* because the cluster index starts with 2 */
@@ -483,15 +459,45 @@ static int __exfat_fill_super(struct super_block *sb)
 	sbi->clu_srch_ptr = EXFAT_FIRST_CLUSTER;
 	sbi->used_clusters = EXFAT_CLUSTERS_UNTRACKED;
 
-	if (le16_to_cpu(p_boot->vol_flags) & VOL_DIRTY) {
-		sbi->vol_flag |= VOL_DIRTY;
-		exfat_warn(sb, "Volume was not properly unmounted. Some data may be corrupt. Please run fsck.");
+	/* check consistencies */
+	if (sbi->num_FAT_sectors << p_boot->sect_size_bits <
+	    sbi->num_clusters * 4) {
+		exfat_err(sb, "bogus fat length");
+		return -EINVAL;
+	}
+	if (sbi->data_start_sector <
+	    sbi->FAT1_start_sector + sbi->num_FAT_sectors * p_boot->num_fats) {
+		exfat_err(sb, "bogus data start sector");
+		return -EINVAL;
 	}
+	if (sbi->vol_flag & VOL_DIRTY)
+		exfat_warn(sb, "Volume was not properly unmounted. Some data may be corrupt. Please run fsck.");
+	if (sbi->vol_flag & ERR_MEDIUM)
+		exfat_warn(sb, "Medium has reported failures. Some data may be lost.");
 
 	/* exFAT file size is limited by a disk volume size */
 	sb->s_maxbytes = (u64)(sbi->num_clusters - EXFAT_RESERVED_CLUSTERS) <<
 		sbi->cluster_size_bits;
 
+	/* check logical sector size */
+	if (exfat_calibrate_blocksize(sb, 1 << p_boot->sect_size_bits))
+		return -EIO;
+
+	return 0;
+}
+
+/* mount the file system volume */
+static int __exfat_fill_super(struct super_block *sb)
+{
+	int ret;
+	struct exfat_sb_info *sbi = EXFAT_SB(sb);
+
+	ret = exfat_read_boot_sector(sb);
+	if (ret) {
+		exfat_err(sb, "failed to read boot sector");
+		goto free_bh;
+	}
+
 	ret = exfat_create_upcase_table(sb);
 	if (ret) {
 		exfat_err(sb, "failed to load upcase table");
-- 
2.25.1

[PATCH 3/4] exfat: add boot region verification

[Tetsuhiro Kohada]

Add Boot-Regions verification specified in exFAT specification.
Note that the checksum type is strongly related to the raw structure,
so the'u32 'type is used to clarify the number of bits.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
 fs/exfat/exfat_fs.h |  1 +
 fs/exfat/misc.c     | 14 +++++++++++++
 fs/exfat/super.c    | 50 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index b0e5b9afc56c..15817281b3c8 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -517,6 +517,7 @@ void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
 		u8 *tz, __le16 *time, __le16 *date, u8 *time_cs);
 unsigned short exfat_calc_chksum_2byte(void *data, int len,
 		unsigned short chksum, int type);
+u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type);
 void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync);
 void exfat_chain_set(struct exfat_chain *ec, unsigned int dir,
 		unsigned int size, unsigned char flags);
diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c
index ab7f88b1f6d3..b82d2dd5bd7c 100644
--- a/fs/exfat/misc.c
+++ b/fs/exfat/misc.c
@@ -151,6 +151,20 @@ unsigned short exfat_calc_chksum_2byte(void *data, int len,
 	return chksum;
 }
 
+u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type)
+{
+	int i;
+	u8 *c = (u8 *)data;
+
+	for (i = 0; i < len; i++, c++) {
+		if (unlikely(type == CS_BOOT_SECTOR &&
+			     (i == 106 || i == 107 || i == 112)))
+			continue;
+		chksum = ((chksum << 31) | (chksum >> 1)) + *c;
+	}
+	return chksum;
+}
+
 void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync)
 {
 	set_bit(EXFAT_SB_DIRTY, &EXFAT_SB(sb)->s_state);
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 95909b4d5e75..42b3bd3df020 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -486,6 +486,50 @@ static int exfat_read_boot_sector(struct super_block *sb)
 	return 0;
 }
 
+static int exfat_verify_boot_region(struct super_block *sb)
+{
+	struct buffer_head *bh = NULL;
+	u32 chksum = 0, *p_sig, *p_chksum;
+	int sn, i;
+
+	/* read boot sector sub-regions */
+	for (sn = 0; sn < 11; sn++) {
+		bh = sb_bread(sb, sn);
+		if (!bh)
+			return -EIO;
+
+		if (sn != 0 && sn <= 8) {
+			/* extended boot sector sub-regions */
+			p_sig = (u32 *)&bh->b_data[sb->s_blocksize - 4];
+			if (le32_to_cpu(*p_sig) != EXBOOT_SIGNATURE) {
+				exfat_err(sb, "no exboot-signature");
+				brelse(bh);
+				return -EINVAL;
+			}
+		}
+
+		chksum = exfat_calc_chksum32(bh->b_data, sb->s_blocksize,
+			chksum, sn ? CS_DEFAULT : CS_BOOT_SECTOR);
+		brelse(bh);
+	}
+
+	/* boot checksum sub-regions */
+	bh = sb_bread(sb, sn);
+	if (!bh)
+		return -EIO;
+
+	for (i = 0; i < sb->s_blocksize; i += sizeof(u32)) {
+		p_chksum = (u32 *)&bh->b_data[i];
+		if (le32_to_cpu(*p_chksum) != chksum) {
+			exfat_err(sb, "mismatch checksum");
+			brelse(bh);
+			return -EINVAL;
+		}
+	}
+	brelse(bh);
+	return 0;
+}
+
 /* mount the file system volume */
 static int __exfat_fill_super(struct super_block *sb)
 {
@@ -498,6 +542,12 @@ static int __exfat_fill_super(struct super_block *sb)
 		goto free_bh;
 	}
 
+	ret = exfat_verify_boot_region(sb);
+	if (ret) {
+		exfat_err(sb, "invalid boot region");
+		goto free_bh;
+	}
+
 	ret = exfat_create_upcase_table(sb);
 	if (ret) {
 		exfat_err(sb, "failed to load upcase table");
-- 
2.25.1

RE: [PATCH 3/4] exfat: add boot region verification

[Namjae Jeon]

[snip]
> +static int exfat_verify_boot_region(struct super_block *sb) {
> +	struct buffer_head *bh = NULL;
> +	u32 chksum = 0, *p_sig, *p_chksum;
> +	int sn, i;
> +
> +	/* read boot sector sub-regions */
> +	for (sn = 0; sn < 11; sn++) {
> +		bh = sb_bread(sb, sn);
> +		if (!bh)
> +			return -EIO;
> +
> +		if (sn != 0 && sn <= 8) {
> +			/* extended boot sector sub-regions */
> +			p_sig = (u32 *)&bh->b_data[sb->s_blocksize - 4];
> +			if (le32_to_cpu(*p_sig) != EXBOOT_SIGNATURE) {
> +				exfat_err(sb, "no exboot-signature");
                                exfat_warn(sb, "Invalid exboot-signature(sector = %d): 0x%08x", sn, *p_sig);
> +				brelse(bh);
> +				return -EINVAL;
Don't make mount error, Just print warning message.
> +			}
> +		}
> +
> +		chksum = exfat_calc_chksum32(bh->b_data, sb->s_blocksize,
> +			chksum, sn ? CS_DEFAULT : CS_BOOT_SECTOR);
> +		brelse(bh);
> +	}
> +
> +	/* boot checksum sub-regions */
> +	bh = sb_bread(sb, sn);
> +	if (!bh)
> +		return -EIO;
> +
> +	for (i = 0; i < sb->s_blocksize; i += sizeof(u32)) {
> +		p_chksum = (u32 *)&bh->b_data[i];
> +		if (le32_to_cpu(*p_chksum) != chksum) {
> +			exfat_err(sb, "mismatch checksum");
Print invalid checksum value also.
> +			brelse(bh);
> +			return -EINVAL;
> +		}
> +	}
> +	brelse(bh);
> +	return 0;
> +}
> +
>  /* mount the file system volume */
>  static int __exfat_fill_super(struct super_block *sb)  { @@ -498,6 +542,12 @@ static int
> __exfat_fill_super(struct super_block *sb)
>  		goto free_bh;
>  	}
> 
> +	ret = exfat_verify_boot_region(sb);
> +	if (ret) {
> +		exfat_err(sb, "invalid boot region");
> +		goto free_bh;
> +	}
> +
>  	ret = exfat_create_upcase_table(sb);
>  	if (ret) {
>  		exfat_err(sb, "failed to load upcase table");
> --
> 2.25.1

Re: [PATCH 3/4] exfat: add boot region verification

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3329 bytes --]

Hi Tetsuhiro,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20200519]
[cannot apply to linus/master v5.7-rc7 v5.7-rc6 v5.7-rc5 v5.7-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tetsuhiro-Kohada/exfat-redefine-PBR-as-boot_sector/20200525-195329
base:    fb57b1fabcb28f358901b2df90abd2b48abc1ca8
config: c6x-randconfig-s032-20200529 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-243-gc100a7ab-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=c6x CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> fs/exfat/super.c:485:29: sparse: sparse: cast to restricted __le32
>> fs/exfat/super.c:485:29: sparse: sparse: cast to restricted __le32
>> fs/exfat/super.c:485:29: sparse: sparse: cast to restricted __le32
>> fs/exfat/super.c:485:29: sparse: sparse: cast to restricted __le32
>> fs/exfat/super.c:485:29: sparse: sparse: cast to restricted __le32
>> fs/exfat/super.c:485:29: sparse: sparse: cast to restricted __le32
   fs/exfat/super.c:504:21: sparse: sparse: cast to restricted __le32
   fs/exfat/super.c:504:21: sparse: sparse: cast to restricted __le32
   fs/exfat/super.c:504:21: sparse: sparse: cast to restricted __le32
   fs/exfat/super.c:504:21: sparse: sparse: cast to restricted __le32
   fs/exfat/super.c:504:21: sparse: sparse: cast to restricted __le32
   fs/exfat/super.c:504:21: sparse: sparse: cast to restricted __le32

vim +485 fs/exfat/super.c

   469	
   470	static int exfat_verify_boot_region(struct super_block *sb)
   471	{
   472		struct buffer_head *bh = NULL;
   473		u32 chksum = 0, *p_sig, *p_chksum;
   474		int sn, i;
   475	
   476		/* read boot sector sub-regions */
   477		for (sn = 0; sn < 11; sn++) {
   478			bh = sb_bread(sb, sn);
   479			if (!bh)
   480				return -EIO;
   481	
   482			if (sn != 0 && sn <= 8) {
   483				/* extended boot sector sub-regions */
   484				p_sig = (u32 *)&bh->b_data[sb->s_blocksize - 4];
 > 485				if (le32_to_cpu(*p_sig) != EXBOOT_SIGNATURE) {
   486					exfat_err(sb, "no exboot-signature");
   487					brelse(bh);
   488					return -EINVAL;
   489				}
   490			}
   491	
   492			chksum = exfat_calc_chksum32(bh->b_data, sb->s_blocksize,
   493				chksum, sn ? CS_DEFAULT : CS_BOOT_SECTOR);
   494			brelse(bh);
   495		}
   496	
   497		/* boot checksum sub-regions */
   498		bh = sb_bread(sb, sn);
   499		if (!bh)
   500			return -EIO;
   501	
   502		for (i = 0; i < sb->s_blocksize; i += sizeof(u32)) {
   503			p_chksum = (u32 *)&bh->b_data[i];
   504			if (le32_to_cpu(*p_chksum) != chksum) {
   505				exfat_err(sb, "mismatch checksum");
   506				brelse(bh);
   507				return -EINVAL;
   508			}
   509		}
   510		brelse(bh);
   511		return 0;
   512	}
   513	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28873 bytes --]

[PATCH 4/4] exfat: standardize checksum calculation

[Tetsuhiro Kohada]

To clarify that it is a 16-bit checksum, the parts related to the 16-bit
checksum are renamed and change type to u16.
Furthermore, replace checksum calculation in exfat_load_upcase_table()
with exfat_calc_checksum32().

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
 fs/exfat/dir.c      | 12 ++++++------
 fs/exfat/exfat_fs.h |  5 ++---
 fs/exfat/misc.c     | 10 ++++------
 fs/exfat/nls.c      | 19 +++++++------------
 4 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index b5a237c33d50..b673362a895c 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -496,7 +496,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
 	int ret = 0;
 	int i, num_entries;
 	sector_t sector;
-	unsigned short chksum;
+	u16 chksum;
 	struct exfat_dentry *ep, *fep;
 	struct buffer_head *fbh, *bh;
 
@@ -505,7 +505,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
 		return -EIO;
 
 	num_entries = fep->dentry.file.num_ext + 1;
-	chksum = exfat_calc_chksum_2byte(fep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
+	chksum = exfat_calc_chksum16(fep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
 
 	for (i = 1; i < num_entries; i++) {
 		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh, NULL);
@@ -513,7 +513,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
 			ret = -EIO;
 			goto release_fbh;
 		}
-		chksum = exfat_calc_chksum_2byte(ep, DENTRY_SIZE, chksum,
+		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
 				CS_DEFAULT);
 		brelse(bh);
 	}
@@ -600,10 +600,10 @@ int exfat_update_dir_chksum_with_entry_set(struct super_block *sb,
 	int chksum_type = CS_DIR_ENTRY, i, num_entries = es->num_entries;
 	unsigned int buf_off = (off - es->offset);
 	unsigned int remaining_byte_in_sector, copy_entries, clu;
-	unsigned short chksum = 0;
+	u16 chksum = 0;
 
 	for (i = 0; i < num_entries; i++) {
-		chksum = exfat_calc_chksum_2byte(&es->entries[i], DENTRY_SIZE,
+		chksum = exfat_calc_chksum16(&es->entries[i], DENTRY_SIZE,
 			chksum, chksum_type);
 		chksum_type = CS_DEFAULT;
 	}
@@ -1047,7 +1047,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
 			}
 
 			if (entry_type == TYPE_STREAM) {
-				unsigned short name_hash;
+				u16 name_hash;
 
 				if (step != DIRENT_STEP_STRM) {
 					step = DIRENT_STEP_FILE;
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 15817281b3c8..993d13bbebec 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -139,7 +139,7 @@ struct exfat_dentry_namebuf {
 struct exfat_uni_name {
 	/* +3 for null and for converting */
 	unsigned short name[MAX_NAME_LENGTH + 3];
-	unsigned short name_hash;
+	u16 name_hash;
 	unsigned char name_len;
 };
 
@@ -515,8 +515,7 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
 void exfat_truncate_atime(struct timespec64 *ts);
 void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
 		u8 *tz, __le16 *time, __le16 *date, u8 *time_cs);
-unsigned short exfat_calc_chksum_2byte(void *data, int len,
-		unsigned short chksum, int type);
+u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type);
 u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type);
 void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync);
 void exfat_chain_set(struct exfat_chain *ec, unsigned int dir,
diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c
index b82d2dd5bd7c..17d41f3d3709 100644
--- a/fs/exfat/misc.c
+++ b/fs/exfat/misc.c
@@ -136,17 +136,15 @@ void exfat_truncate_atime(struct timespec64 *ts)
 	ts->tv_nsec = 0;
 }
 
-unsigned short exfat_calc_chksum_2byte(void *data, int len,
-		unsigned short chksum, int type)
+u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type)
 {
 	int i;
-	unsigned char *c = (unsigned char *)data;
+	u8 *c = (u8 *)data;
 
 	for (i = 0; i < len; i++, c++) {
-		if (((i == 2) || (i == 3)) && (type == CS_DIR_ENTRY))
+		if (unlikely(type == CS_DIR_ENTRY && (i == 2 || i == 3)))
 			continue;
-		chksum = (((chksum & 1) << 15) | ((chksum & 0xFFFE) >> 1)) +
-			(unsigned short)*c;
+		chksum = ((chksum << 15) | (chksum >> 1)) + *c;
 	}
 	return chksum;
 }
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c
index 1ebda90cbdd7..19321773dd07 100644
--- a/fs/exfat/nls.c
+++ b/fs/exfat/nls.c
@@ -527,7 +527,7 @@ static int exfat_utf8_to_utf16(struct super_block *sb,
 
 	*uniname = '\0';
 	p_uniname->name_len = unilen;
-	p_uniname->name_hash = exfat_calc_chksum_2byte(upname, unilen << 1, 0,
+	p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
 			CS_DEFAULT);
 
 	if (p_lossy)
@@ -623,7 +623,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb,
 
 	*uniname = '\0';
 	p_uniname->name_len = unilen;
-	p_uniname->name_hash = exfat_calc_chksum_2byte(upname, unilen << 1, 0,
+	p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
 			CS_DEFAULT);
 
 	if (p_lossy)
@@ -655,7 +655,8 @@ static int exfat_load_upcase_table(struct super_block *sb,
 {
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	unsigned int sect_size = sb->s_blocksize;
-	unsigned int i, index = 0, checksum = 0;
+	unsigned int i, index = 0;
+	u32 chksum = 0;
 	int ret;
 	unsigned char skip = false;
 	unsigned short *upcase_table;
@@ -681,13 +682,6 @@ static int exfat_load_upcase_table(struct super_block *sb,
 		for (i = 0; i < sect_size && index <= 0xFFFF; i += 2) {
 			unsigned short uni = get_unaligned_le16(bh->b_data + i);
 
-			checksum = ((checksum & 1) ? 0x80000000 : 0) +
-				(checksum >> 1) +
-				*(((unsigned char *)bh->b_data) + i);
-			checksum = ((checksum & 1) ? 0x80000000 : 0) +
-				(checksum >> 1) +
-				*(((unsigned char *)bh->b_data) + (i + 1));
-
 			if (skip) {
 				index += uni;
 				skip = false;
@@ -701,13 +695,14 @@ static int exfat_load_upcase_table(struct super_block *sb,
 			}
 		}
 		brelse(bh);
+		chksum = exfat_calc_chksum32(bh->b_data, i, chksum, CS_DEFAULT);
 	}
 
-	if (index >= 0xFFFF && utbl_checksum == checksum)
+	if (index >= 0xFFFF && utbl_checksum == chksum)
 		return 0;
 
 	exfat_err(sb, "failed to load upcase table (idx : 0x%08x, chksum : 0x%08x, utbl_chksum : 0x%08x)",
-		  index, checksum, utbl_checksum);
+		  index, chksum, utbl_checksum);
 	ret = -EINVAL;
 free_table:
 	exfat_free_upcase_table(sbi);
-- 
2.25.1

RE: [PATCH 4/4] exfat: standardize checksum calculation

[Namjae Jeon]

> To clarify that it is a 16-bit checksum, the parts related to the 16-bit checksum are renamed and
> change type to u16.
> Furthermore, replace checksum calculation in exfat_load_upcase_table() with exfat_calc_checksum32().
> 
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>

I can not apply this patch to exfat dev tree. Could you please check it ?
patching file fs/exfat/dir.c
Hunk #1 succeeded at 491 (offset -5 lines).
Hunk #2 succeeded at 500 (offset -5 lines).
Hunk #3 succeeded at 508 (offset -5 lines).
Hunk #4 FAILED at 600.
Hunk #5 succeeded at 1000 (offset -47 lines).
1 out of 5 hunks FAILED -- saving rejects to file fs/exfat/dir.c.rej
patching file fs/exfat/exfat_fs.h
Hunk #1 succeeded at 137 (offset -2 lines).
Hunk #2 succeeded at 512 (offset -3 lines).
patching file fs/exfat/misc.c
patching file fs/exfat/nls.c

Thanks!
> ---
>  fs/exfat/dir.c      | 12 ++++++------
>  fs/exfat/exfat_fs.h |  5 ++---
>  fs/exfat/misc.c     | 10 ++++------
>  fs/exfat/nls.c      | 19 +++++++------------
>  4 files changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index b5a237c33d50..b673362a895c 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -496,7 +496,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
>  	int ret = 0;
>  	int i, num_entries;
>  	sector_t sector;
> -	unsigned short chksum;
> +	u16 chksum;
>  	struct exfat_dentry *ep, *fep;
>  	struct buffer_head *fbh, *bh;
> 
> @@ -505,7 +505,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
>  		return -EIO;
> 
>  	num_entries = fep->dentry.file.num_ext + 1;
> -	chksum = exfat_calc_chksum_2byte(fep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
> +	chksum = exfat_calc_chksum16(fep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
> 
>  	for (i = 1; i < num_entries; i++) {
>  		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh, NULL); @@ -513,7 +513,7 @@ int
> exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
>  			ret = -EIO;
>  			goto release_fbh;
>  		}
> -		chksum = exfat_calc_chksum_2byte(ep, DENTRY_SIZE, chksum,
> +		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
>  				CS_DEFAULT);
>  		brelse(bh);
>  	}
> @@ -600,10 +600,10 @@ int exfat_update_dir_chksum_with_entry_set(struct super_block *sb,
>  	int chksum_type = CS_DIR_ENTRY, i, num_entries = es->num_entries;
>  	unsigned int buf_off = (off - es->offset);
>  	unsigned int remaining_byte_in_sector, copy_entries, clu;
> -	unsigned short chksum = 0;
> +	u16 chksum = 0;
> 
>  	for (i = 0; i < num_entries; i++) {
> -		chksum = exfat_calc_chksum_2byte(&es->entries[i], DENTRY_SIZE,
> +		chksum = exfat_calc_chksum16(&es->entries[i], DENTRY_SIZE,
>  			chksum, chksum_type);
>  		chksum_type = CS_DEFAULT;
>  	}
> @@ -1047,7 +1047,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
>  			}
> 
>  			if (entry_type == TYPE_STREAM) {
> -				unsigned short name_hash;
> +				u16 name_hash;
> 
>  				if (step != DIRENT_STEP_STRM) {
>  					step = DIRENT_STEP_FILE;
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index 15817281b3c8..993d13bbebec 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -139,7 +139,7 @@ struct exfat_dentry_namebuf {  struct exfat_uni_name {
>  	/* +3 for null and for converting */
>  	unsigned short name[MAX_NAME_LENGTH + 3];
> -	unsigned short name_hash;
> +	u16 name_hash;
>  	unsigned char name_len;
>  };
> 
> @@ -515,8 +515,7 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,  void
> exfat_truncate_atime(struct timespec64 *ts);  void exfat_set_entry_time(struct exfat_sb_info *sbi,
> struct timespec64 *ts,
>  		u8 *tz, __le16 *time, __le16 *date, u8 *time_cs); -unsigned short
> exfat_calc_chksum_2byte(void *data, int len,
> -		unsigned short chksum, int type);
> +u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type);
>  u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type);  void exfat_update_bh(struct
> super_block *sb, struct buffer_head *bh, int sync);  void exfat_chain_set(struct exfat_chain *ec,
> unsigned int dir, diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c index b82d2dd5bd7c..17d41f3d3709
> 100644
> --- a/fs/exfat/misc.c
> +++ b/fs/exfat/misc.c
> @@ -136,17 +136,15 @@ void exfat_truncate_atime(struct timespec64 *ts)
>  	ts->tv_nsec = 0;
>  }
> 
> -unsigned short exfat_calc_chksum_2byte(void *data, int len,
> -		unsigned short chksum, int type)
> +u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type)
>  {
>  	int i;
> -	unsigned char *c = (unsigned char *)data;
> +	u8 *c = (u8 *)data;
> 
>  	for (i = 0; i < len; i++, c++) {
> -		if (((i == 2) || (i == 3)) && (type == CS_DIR_ENTRY))
> +		if (unlikely(type == CS_DIR_ENTRY && (i == 2 || i == 3)))
>  			continue;
> -		chksum = (((chksum & 1) << 15) | ((chksum & 0xFFFE) >> 1)) +
> -			(unsigned short)*c;
> +		chksum = ((chksum << 15) | (chksum >> 1)) + *c;
>  	}
>  	return chksum;
>  }
> diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 1ebda90cbdd7..19321773dd07 100644
> --- a/fs/exfat/nls.c
> +++ b/fs/exfat/nls.c
> @@ -527,7 +527,7 @@ static int exfat_utf8_to_utf16(struct super_block *sb,
> 
>  	*uniname = '\0';
>  	p_uniname->name_len = unilen;
> -	p_uniname->name_hash = exfat_calc_chksum_2byte(upname, unilen << 1, 0,
> +	p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
>  			CS_DEFAULT);
> 
>  	if (p_lossy)
> @@ -623,7 +623,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb,
> 
>  	*uniname = '\0';
>  	p_uniname->name_len = unilen;
> -	p_uniname->name_hash = exfat_calc_chksum_2byte(upname, unilen << 1, 0,
> +	p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
>  			CS_DEFAULT);
> 
>  	if (p_lossy)
> @@ -655,7 +655,8 @@ static int exfat_load_upcase_table(struct super_block *sb,  {
>  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
>  	unsigned int sect_size = sb->s_blocksize;
> -	unsigned int i, index = 0, checksum = 0;
> +	unsigned int i, index = 0;
> +	u32 chksum = 0;
>  	int ret;
>  	unsigned char skip = false;
>  	unsigned short *upcase_table;
> @@ -681,13 +682,6 @@ static int exfat_load_upcase_table(struct super_block *sb,
>  		for (i = 0; i < sect_size && index <= 0xFFFF; i += 2) {
>  			unsigned short uni = get_unaligned_le16(bh->b_data + i);
> 
> -			checksum = ((checksum & 1) ? 0x80000000 : 0) +
> -				(checksum >> 1) +
> -				*(((unsigned char *)bh->b_data) + i);
> -			checksum = ((checksum & 1) ? 0x80000000 : 0) +
> -				(checksum >> 1) +
> -				*(((unsigned char *)bh->b_data) + (i + 1));
> -
>  			if (skip) {
>  				index += uni;
>  				skip = false;
> @@ -701,13 +695,14 @@ static int exfat_load_upcase_table(struct super_block *sb,
>  			}
>  		}
>  		brelse(bh);
> +		chksum = exfat_calc_chksum32(bh->b_data, i, chksum, CS_DEFAULT);
>  	}
> 
> -	if (index >= 0xFFFF && utbl_checksum == checksum)
> +	if (index >= 0xFFFF && utbl_checksum == chksum)
>  		return 0;
> 
>  	exfat_err(sb, "failed to load upcase table (idx : 0x%08x, chksum : 0x%08x, utbl_chksum :
> 0x%08x)",
> -		  index, checksum, utbl_checksum);
> +		  index, chksum, utbl_checksum);
>  	ret = -EINVAL;
>  free_table:
>  	exfat_free_upcase_table(sbi);
> --
> 2.25.1

Re: [PATCH 4/4] exfat: standardize checksum calculation

[Tetsuhiro Kohada]

Thank you for your comment.

> I can not apply this patch to exfat dev tree. Could you please check it ?
> patching file fs/exfat/dir.c
> Hunk #1 succeeded at 491 (offset -5 lines).
> Hunk #2 succeeded at 500 (offset -5 lines).
> Hunk #3 succeeded at 508 (offset -5 lines).
> Hunk #4 FAILED at 600.
> Hunk #5 succeeded at 1000 (offset -47 lines).
> 1 out of 5 hunks FAILED -- saving rejects to file fs/exfat/dir.c.rej
> patching file fs/exfat/exfat_fs.h
> Hunk #1 succeeded at 137 (offset -2 lines).
> Hunk #2 succeeded at 512 (offset -3 lines).
> patching file fs/exfat/misc.c
> patching file fs/exfat/nls.c

II tried applying patch to dev-tree (4c4dbb6ad8e8).
-The .patch file I sent
-mbox file downloaded from archive
But I can't reproduce the error. (Both succeed)
How do you reproduce the error?

BR

Re: [PATCH 4/4] exfat: standardize checksum calculation

[Namjae Jeon]

2020-05-27 16:39 GMT+09:00, Tetsuhiro Kohada <kohada.t2@gmail.com>:
> Thank you for your comment.
>
>> I can not apply this patch to exfat dev tree. Could you please check it ?
>> patching file fs/exfat/dir.c
>> Hunk #1 succeeded at 491 (offset -5 lines).
>> Hunk #2 succeeded at 500 (offset -5 lines).
>> Hunk #3 succeeded at 508 (offset -5 lines).
>> Hunk #4 FAILED at 600.
>> Hunk #5 succeeded at 1000 (offset -47 lines).
>> 1 out of 5 hunks FAILED -- saving rejects to file fs/exfat/dir.c.rej
>> patching file fs/exfat/exfat_fs.h
>> Hunk #1 succeeded at 137 (offset -2 lines).
>> Hunk #2 succeeded at 512 (offset -3 lines).
>> patching file fs/exfat/misc.c
>> patching file fs/exfat/nls.c
>
> II tried applying patch to dev-tree (4c4dbb6ad8e8).
> -The .patch file I sent
> -mbox file downloaded from archive
> But I can't reproduce the error. (Both succeed)
> How do you reproduce the error?
I tried to appy your patches in the following order.
1. [PATCH] exfat: optimize dir-cache
2. [PATCH 1/4] exfat: redefine PBR as boot_sector
3. [PATCH 2/4] exfat: separate the boot sector analysis
4. [PATCH 3/4] exfat: add boot region verification
5. [PATCH 4/4] exfat: standardize checksum calculation

Thanks!
>
> BR
>

Re: [PATCH 4/4] exfat: standardize checksum calculation

[Tetsuhiro Kohada]

>> II tried applying patch to dev-tree (4c4dbb6ad8e8).
>> -The .patch file I sent
>> -mbox file downloaded from archive
>> But I can't reproduce the error. (Both succeed)
>> How do you reproduce the error?
> I tried to appy your patches in the following order.
> 1. [PATCH] exfat: optimize dir-cache
> 2. [PATCH 1/4] exfat: redefine PBR as boot_sector
> 3. [PATCH 2/4] exfat: separate the boot sector analysis
> 4. [PATCH 3/4] exfat: add boot region verification
> 5. [PATCH 4/4] exfat: standardize checksum calculation

I was able to reproduce it.

The dir-cache patch was created based on the HEAD of dev-tree.
The 4 patches for boot_sector were also created based on the HEAD of dev-tree.
(at physically separated place)

I'm sorry I didn't check any conflicts with these patches.

I'll repost the patch, based on the dir-cache patched dev-tree.
If dir-cache patch will merge into dev-tree, should I wait until then?

BR

RE: [PATCH 4/4] exfat: standardize checksum calculation

[Namjae Jeon]

> >> II tried applying patch to dev-tree (4c4dbb6ad8e8).
> >> -The .patch file I sent
> >> -mbox file downloaded from archive
> >> But I can't reproduce the error. (Both succeed) How do you reproduce
> >> the error?
> > I tried to appy your patches in the following order.
> > 1. [PATCH] exfat: optimize dir-cache
> > 2. [PATCH 1/4] exfat: redefine PBR as boot_sector 3. [PATCH 2/4]
> > exfat: separate the boot sector analysis 4. [PATCH 3/4] exfat: add
> > boot region verification 5. [PATCH 4/4] exfat: standardize checksum
> > calculation
> 
> I was able to reproduce it.
> 
> The dir-cache patch was created based on the HEAD of dev-tree.
> The 4 patches for boot_sector were also created based on the HEAD of dev-tree.
> (at physically separated place)
> 
> I'm sorry I didn't check any conflicts with these patches.
> 
> I'll repost the patch, based on the dir-cache patched dev-tree.
> If dir-cache patch will merge into dev-tree, should I wait until then?
I will apply them after testing at once if you send updated 5 patches again.
Thanks!
> 
> BR

Re: [PATCH 4/4] exfat: standardize checksum calculation

[Tetsuhiro Kohada]

>> I'll repost the patch, based on the dir-cache patched dev-tree.
>> If dir-cache patch will merge into dev-tree, should I wait until then?
> I will apply them after testing at once if you send updated 5 patches again.

I resend patches for boot_sector.
However, the dir-cache patch hasn't changed, so I haven't reposted it.

BR

Re: [PATCH 4/4] exfat: standardize checksum calculation

[Namjae Jeon]

2020-05-28 19:09 GMT+09:00, Tetsuhiro Kohada <kohada.t2@gmail.com>:
>>> I'll repost the patch, based on the dir-cache patched dev-tree.
>>> If dir-cache patch will merge into dev-tree, should I wait until then?
>> I will apply them after testing at once if you send updated 5 patches
>> again.
>
> I resend patches for boot_sector.
> However, the dir-cache patch hasn't changed, so I haven't reposted it.
Well, I leave a comment for this patch.
>
> BR
>

RE: [PATCH 1/4] exfat: redefine PBR as boot_sector

[Sungjong Seo]

> Aggregate PBR related definitions and redefine as "boot_sector" to comply
> with the exFAT specification.
> And, rename variable names including 'pbr'.
> 
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
> ---
>  fs/exfat/exfat_fs.h  |  2 +-
>  fs/exfat/exfat_raw.h | 79 +++++++++++++++--------------------------
>  fs/exfat/super.c     | 84 ++++++++++++++++++++++----------------------
>  3 files changed, 72 insertions(+), 93 deletions(-)
> 
[snip]
> +/* EXFAT: Main and Backup Boot Sector (512 bytes) */ struct boot_sector
> +{
> +	__u8	jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
> +	__u8	oem_name[BOOTSEC_OEM_NAME_LEN];

According to the exFAT specification, fs_name and BOOTSEC_FS_NAME_LEN look
better.

> +	__u8	must_be_zero[BOOTSEC_OLDBPB_LEN];
> +	__le64	partition_offset;
> +	__le64	vol_length;
> +	__le32	fat_offset;
> +	__le32	fat_length;
> +	__le32	clu_offset;
> +	__le32	clu_count;
> +	__le32	root_cluster;
> +	__le32	vol_serial;
> +	__u8	fs_revision[2];
> +	__le16	vol_flags;
> +	__u8	sect_size_bits;
> +	__u8	sect_per_clus_bits;
> +	__u8	num_fats;
> +	__u8	drv_sel;
> +	__u8	percent_in_use;
> +	__u8	reserved[7];
> +	__u8	boot_code[390];
> +	__le16	signature;
>  } __packed;

Re: [PATCH 1/4] exfat: redefine PBR as boot_sector

[Tetsuhiro Kohada]

> [snip]
>> +/* EXFAT: Main and Backup Boot Sector (512 bytes) */ struct boot_sector
>> +{
>> +	__u8	jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
>> +	__u8	oem_name[BOOTSEC_OEM_NAME_LEN];
> 
> According to the exFAT specification, fs_name and BOOTSEC_FS_NAME_LEN look
> better.

Oops.
I sent v2 patches, before I noticed this comment,

I'll make another small patch, OK?

BTW
I have a concern about fs_name.
The exfat specification says that this field is "EXFAT".

I think it's a important field for determining the filesystem.
However, in this patch, I gave up checking this field.
Because there is no similar check in FATFS.
Do you know why Linux FATFS does not check this filed?
And, what do you think of checking this field?

BR

RE: [PATCH 1/4] exfat: redefine PBR as boot_sector

[Sungjong Seo]

> > [snip]
> >> +/* EXFAT: Main and Backup Boot Sector (512 bytes) */ struct
> >> +boot_sector {
> >> +	__u8	jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
> >> +	__u8	oem_name[BOOTSEC_OEM_NAME_LEN];
> >
> > According to the exFAT specification, fs_name and BOOTSEC_FS_NAME_LEN
> > look better.
> 
> Oops.
> I sent v2 patches, before I noticed this comment,
> 
> I'll make another small patch, OK?

No, It make sense to make v3, because you have renamed the variables in
boot_sector on this patch.

> BTW
> I have a concern about fs_name.
> The exfat specification says that this field is "EXFAT".
> 
> I think it's a important field for determining the filesystem.
> However, in this patch, I gave up checking this field.
> Because there is no similar check in FATFS.
> Do you know why Linux FATFS does not check this filed?
> And, what do you think of checking this field?

FATFS has the same field named "oem_name" and whatever is okay for its value.
However, in case of exFAT, it is an important field to determine filesystem.

I think it would be better to check this field for exFAT-fs.
Would you like to contribute new patch for checking it?

> 
> BR

Re: [PATCH 1/4] exfat: redefine PBR as boot_sector

[Tetsuhiro Kohada]

>> I'll make another small patch, OK?
> 
> No, It make sense to make v3, because you have renamed the variables in
> boot_sector on this patch.

OK.


>> BTW
>> I have a concern about fs_name.
>> The exfat specification says that this field is "EXFAT".
>>
>> I think it's a important field for determining the filesystem.
>> However, in this patch, I gave up checking this field.
>> Because there is no similar check in FATFS.
>> Do you know why Linux FATFS does not check this filed?
>> And, what do you think of checking this field?
> 
> FATFS has the same field named "oem_name" and whatever is okay for its value.
> However, in case of exFAT, it is an important field to determine filesystem.
> 
> I think it would be better to check this field for exFAT-fs.
> Would you like to contribute new patch for checking it?

I already have the code, so I'll add it to [PATCH 2/4 v3].

BR