Re: [PATCH 3/4] exfat: add boot region verification
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
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 *)>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 *)>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-...@lists.01.org
.config.gz
Description: application/gzip
RE: [PATCH 3/4] exfat: add boot region verification
[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 *)>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 *)>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
[PATCH 3/4] exfat: add boot region verification
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
---
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, _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 *)>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 *)>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
