On 11/8/23 07:12, christian.taedcke-...@weidmueller.com wrote:
From: Christian Taedcke <christian.taed...@weidmueller.com>
This fixes an issue where the FAT type (FAT12, FAT16) is not
correctly detected, e.g. when the BPB field BS_FilSysType contains the
valid value "FAT ".
According to the FAT spec the field BS_FilSysType has only
informational character and does not determine the FAT type.
The logic of this code is based on the linux kernel implementation
from the file fs/fat/inode.c function fat_fill_super().
For details about FAT see http://elm-chan.org/docs/fat_e.html
Signed-off-by: Christian Taedcke <christian.taed...@weidmueller.com>
Have you run bloat-o-meter on this? FAT is used in SPL a lot.
Would it be possible to write a test for this, especially for a FAT12
filesystem with
4084 data sectors and a FAT16 filesystem with 4085?
---
fs/fat/fat.c | 58 +++++++++++++++++++++++-----------------
include/fat.h | 6 -----
test/image/spl_load_fs.c | 2 +-
3 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 8a0f4e4e6c..9179257ff2 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -27,6 +27,9 @@
#define FATU8ARRAY2CPU16(x) (((x)[1] << 8) + (x)[0])
+/* maximum number of clusters for FAT12 */
+#define MAX_FAT12 0xFF4
+
/*
* Convert a string to lowercase. Converts at most 'len' characters,
* 'len' may be larger than the length of 'str' if 'str' is NULL
@@ -485,6 +488,32 @@ static __u8 mkcksum(struct nameext *nameext)
return ret;
}
+/*
+ * Determine the FAT type
+ *
+ * Based on fat_fill_super() from the Linux kernel's fs/fat/inode.c
+ */
+static int
+determine_fat_bits(const boot_sector *bs)
+{
+ if (!bs->fat_length && bs->fat32_length) {
+ return 32;
+ } else {
+ u16 fat_start = bs->reserved;
+ u32 dir_start = fat_start + bs->fats * bs->fat_length;
+ u32 rootdir_sectors = FATU8ARRAY2CPU16(bs->dir_entries) *
+ sizeof(dir_entry) /
+ FATU8ARRAY2CPU16(bs->sector_size);
Note that technically we are supposed to round up.
+ u32 data_start = dir_start + rootdir_sectors;
+ u16 sectors = FATU8ARRAY2CPU16(bs->sectors);
+ u32 total_sectors = sectors ? sectors : bs->total_sect;
+ u32 total_clusters = (total_sectors - data_start) /
+ bs->cluster_size;
+
+ return (total_clusters > MAX_FAT12) ? 16 : 12;
+ }
+}
+
This should be merged with...
/*
* Read boot sector and volume info from a FAT filesystem
*/
@@ -493,7 +522,6 @@ read_bootsectandvi(boot_sector *bs, volume_info *volinfo,
int *fatsize)
{
__u8 *block;
volume_info *vistart;
- int ret = 0;
if (cur_dev == NULL) {
debug("Error: no device selected\n");
@@ -508,7 +536,8 @@ read_bootsectandvi(boot_sector *bs, volume_info *volinfo,
int *fatsize)
if (disk_read(0, 1, block) < 0) {
debug("Error: reading block\n");
- goto fail;
+ free(block);
+ return -1;
}
memcpy(bs, block, sizeof(boot_sector));
@@ -527,33 +556,15 @@ read_bootsectandvi(boot_sector *bs, volume_info *volinfo,
int *fatsize)
bs->info_sector = FAT2CPU16(bs->info_sector);
bs->backup_boot = FAT2CPU16(bs->backup_boot);
vistart = (volume_info *)(block + sizeof(boot_sector));
- *fatsize = 32;
} else {
vistart = (volume_info *)&(bs->fat32_length);
- *fatsize = 0;
}
... these if statements (above)
memcpy(volinfo, vistart, sizeof(volume_info));
- if (*fatsize == 32) {
- if (strncmp(FAT32_SIGN, vistart->fs_type, SIGNLEN) == 0)
- goto exit;
- } else {
- if (strncmp(FAT12_SIGN, vistart->fs_type, SIGNLEN) == 0) {
- *fatsize = 12;
- goto exit;
- }
- if (strncmp(FAT16_SIGN, vistart->fs_type, SIGNLEN) == 0) {
- *fatsize = 16;
- goto exit;
- }
- }
I think we should still verify these as long as vistart->ext_boot_sign is 0x29.
+ *fatsize = determine_fat_bits(bs);
- debug("Error: broken fs_type sign\n");
-fail:
- ret = -1;
-exit:
free(block);
- return ret;
+ return 0;
}
static int get_fs_info(fsdata *mydata)
@@ -1158,9 +1169,8 @@ int file_fat_detectfs(void)
memcpy(vol_label, volinfo.volume_label, 11);
vol_label[11] = '\0';
- volinfo.fs_type[5] = '\0';
- printf("Filesystem: %s \"%s\"\n", volinfo.fs_type, vol_label);
+ printf("Filesystem: FAT%d \"%s\"\n", fatsize, vol_label);
return 0;
}
diff --git a/include/fat.h b/include/fat.h
index a9756fb4cd..3dce99a23c 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -34,12 +34,6 @@ struct disk_partition;
/* Maximum number of entry for long file name according to spec */
#define MAX_LFN_SLOT 20
-/* Filesystem identifiers */
-#define FAT12_SIGN "FAT12 "
-#define FAT16_SIGN "FAT16 "
-#define FAT32_SIGN "FAT32 "
-#define SIGNLEN 8
-
/* File attributes */
#define ATTR_RO 1
#define ATTR_HIDDEN 2
diff --git a/test/image/spl_load_fs.c b/test/image/spl_load_fs.c
index 297ab08a82..90e640b5de 100644
--- a/test/image/spl_load_fs.c
+++ b/test/image/spl_load_fs.c
@@ -220,7 +220,7 @@ static size_t create_fat(void *dst, size_t size, const char
*filename,
bs->root_cluster = cpu_to_le32(root_sector);
vi->ext_boot_sign = 0x29;
- memcpy(vi->fs_type, FAT32_SIGN, sizeof(vi->fs_type));
+ memcpy(vi->fs_type, "FAT32 ", sizeof(vi->fs_type));
memcpy(dst + 0x1fe, "\x55\xAA", 2);
--Sean
