fix: correctly calculate FAT bit size based on cluster count

This commit addresses an issue in determining the bit size for FAT
filesystems. The previous approach was as follows:

1. Check the partition GUID against a hard-coded list of allowed values.
2. Assign 12-bit for BS_FilSysType FAT12, 16-bit for FAT16.
3. Assume 32-bit for all other cases.

This is problematic, because

1. the possible values of BS_FilSysType are not defined in the spec (and
   indeed vary across different filesystem creation tools).
2. The assumption that any other FS type must be FAT32 is incorrect and
   results in futile attempts to mount non-FAT filesystems using the FAT
   driver.

This commit aligns with the parsing algorithm used by the Linux kernel,
borrowing code from fs/fat to ensure maximum compatibility and
robustness. The borrowed code has been kept to a minimum and as close as
possible to upstream, making it easy to pull in bugfixes from future
versions of the Linux kernel. Both efibootguard and the Linux kernel are
GPL-2.0 licensed, permitting this approach.

For an in-depth understanding of the FAT file system, please refer to
http://elm-chan.org/docs/fat_e.html.

Signed-off-by: Michael Adler <[email protected]>
[Jan: style fix, add commenting sector_size = 0]
Signed-off-by: Jan Kiszka <[email protected]>

.gitignore

@@ -37,6 +37,7 @@ tools/tests/test_partitions
 tools/tests/test_environment
 tools/tests/test_api
 tools/tests/test_uservars
+tools/tests/test_fat
 tools/tests/*.log
 tools/tests/*.trs
 

Makefile.am

@@ -94,7 +94,8 @@ libebgenv_a_SOURCES = \
 	env/env_config_partitions.c \
 	env/env_disk_utils.c \
 	env/uservars.c \
-	tools/ebgpart.c
+	tools/ebgpart.c \
+	tools/fat.c
 
 libebgenv_a_CPPFLAGS = \
 	$(AM_CPPFLAGS) \

tools/ebgpart.c

@@ -18,6 +18,7 @@
 
 #include "ebgpart.h"
 #include <sys/sysmacros.h>
+#include "fat.h"
 
 static PedDevice *first_device = NULL;
 static PedDisk g_ped_dummy_disk;
@@ -93,54 +94,41 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
 			strerror(errno));
 		return false;
 	}
-	/* Look if it is a FAT12 or FAT16 */
-	off64_t dest = (off64_t)e->start_LBA * LB_SIZE + 0x36;
-	if (lseek64(fd, dest, SEEK_SET) == -1) {
-		VERBOSE(stderr, "Error seeking FAT12/16 Id String: %s\n",
+
+	/* seek to partition start */
+	off64_t offset_start = (off64_t)e->start_LBA * LB_SIZE;
+	if (lseek64(fd, offset_start, SEEK_SET) == -1) {
+		VERBOSE(stderr, "Error seeking to partition start: %s\n",
 			strerror(errno));
 		return false;
 	}
-	char FAT_id[9];
-	if (read(fd, FAT_id, 8) != 8) {
-		VERBOSE(stderr, "Error reading FAT12/16 Id String: %s\n",
+
+	/* read FAT header */
+	struct fat_boot_sector header;
+	if (read(fd, &header, sizeof(header)) != sizeof(header)) {
+		VERBOSE(stderr, "Error reading FAT header: %s\n",
 			strerror(errno));
 		return false;
-	};
-	FAT_id[8] = 0;
-	if (strcmp(FAT_id, "FAT12   ") != 0 &&
-	    strcmp(FAT_id, "FAT16   ") != 0) {
-		/* No FAT12/16 so read ID field for FAT32 */
-		dest = (off64_t)e->start_LBA * LB_SIZE + 0x52;
-		if (lseek64(fd, dest, SEEK_SET) == -1) {
-			VERBOSE(stderr, "Error seeking FAT32 Id String: %s\n",
-				strerror(errno));
-			return false;
-		}
-		if (read(fd, FAT_id, 8) != 8) {
-			VERBOSE(stderr, "Error reading FAT32 Id String: %s\n",
-				strerror(errno));
-			return false;
-		}
 	}
-	if (strcmp(FAT_id, "FAT12   ") == 0) {
-		if (asprintf(&pfst->name, "%s", "fat12") == -1) {
-			goto error_asprintf;
-		}
-	} else if (strcmp(FAT_id, "FAT16   ") == 0) {
-		if (asprintf(&pfst->name, "%s", "fat16") == -1) {
-			goto error_asprintf;
-		}
-	} else {
-		if (asprintf(&pfst->name, "%s", "fat32") == -1) {
-			goto error_asprintf;
-		}
-	}
-	VERBOSE(stdout, "GPT Partition #%u is %s.\n", i, pfst->name);
+
+	/* restore pos */
 	if (lseek64(fd, curr, SEEK_SET) == -1) {
 		VERBOSE(stderr, "Error restoring seek position (%s)",
 			strerror(errno));
 		return false;
 	}
+
+	int fat_bits = determine_FAT_bits(&header);
+	if (fat_bits <= 0) {
+		/* not a FAT header */
+		return false;
+	}
+	if (asprintf(&pfst->name, "fat%d", fat_bits) == -1) {
+		VERBOSE(stderr,
+			"Error in asprintf - possibly out of memory.\n");
+		return false;
+	}
+	VERBOSE(stdout, "GPT Partition #%u is %s.\n", i, pfst->name);
 	return true;
 
 error_asprintf:

tools/fat.c

@@ -0,0 +1,184 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2023
+ *
+ * Author: Michael Adler <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <linux/types.h>
+#include <linux/byteorder/little_endian.h>
+
+#include "fat.h"
+#include "linux_util.h"
+#include "ebgpart.h"
+
+static bool verbosity = true;
+
+#define fat_msg(sb, lvl, ...) do { VERBOSE(stderr, __VA_ARGS__); VERBOSE(stderr, "\n"); } while(0);
+#define KERN_ERR "ERROR"
+
+/******************************************************************************
+ * The below code was adopted from the Linux kernel:
+ *
+ * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/fat/inode.c?id=5d0c230f1de8c7515b6567d9afba1f196fb4e2f4
+ *
+ * The modifications were kept to a minimum to make it easy to sync these files.
+ */
+
+/*
+ * A deserialized copy of the on-disk structure laid out in struct
+ * fat_boot_sector.
+ */
+struct fat_bios_param_block {
+	u16	fat_sector_size;
+	u8	fat_sec_per_clus;
+	u16	fat_reserved;
+	u8	fat_fats;
+	u16	fat_dir_entries;
+	u16	fat_sectors;
+	u16	fat_fat_length;
+	u32	fat_total_sect;
+
+	u8	fat16_state;
+	u32	fat16_vol_id;
+
+	u32	fat32_length;
+	u32	fat32_root_cluster;
+	u16	fat32_info_sector;
+	u8	fat32_state;
+	u32	fat32_vol_id;
+};
+
+/* media of boot sector */
+static inline int fat_valid_media(u8 media)
+{
+	return 0xf8 <= media || media == 0xf0;
+}
+
+static int fat_read_bpb(void __attribute__((unused)) *sb,
+	struct fat_boot_sector *b, int silent, struct fat_bios_param_block *bpb)
+{
+	int error = -EINVAL;
+
+	/* Read in BPB ... */
+	memset(bpb, 0, sizeof(*bpb));
+	bpb->fat_sector_size = get_unaligned_le16(&b->sector_size);
+	bpb->fat_sec_per_clus = b->sec_per_clus;
+	bpb->fat_reserved = le16_to_cpu(b->reserved);
+	bpb->fat_fats = b->fats;
+	bpb->fat_dir_entries = get_unaligned_le16(&b->dir_entries);
+	bpb->fat_sectors = get_unaligned_le16(&b->sectors);
+	bpb->fat_fat_length = le16_to_cpu(b->fat_length);
+	bpb->fat_total_sect = le32_to_cpu(b->total_sect);
+
+	bpb->fat16_state = b->fat16.state;
+	bpb->fat16_vol_id = get_unaligned_le32(b->fat16.vol_id);
+
+	bpb->fat32_length = le32_to_cpu(b->fat32.length);
+	bpb->fat32_root_cluster = le32_to_cpu(b->fat32.root_cluster);
+	bpb->fat32_info_sector = le16_to_cpu(b->fat32.info_sector);
+	bpb->fat32_state = b->fat32.state;
+	bpb->fat32_vol_id = get_unaligned_le32(b->fat32.vol_id);
+
+	/* Validate this looks like a FAT filesystem BPB */
+	if (!bpb->fat_reserved) {
+		if (!silent)
+			fat_msg(sb, KERN_ERR,
+				"bogus number of reserved sectors");
+		goto out;
+	}
+	if (!bpb->fat_fats) {
+		if (!silent)
+			fat_msg(sb, KERN_ERR, "bogus number of FAT structure");
+		goto out;
+	}
+
+	/*
+	 * Earlier we checked here that b->secs_track and b->head are nonzero,
+	 * but it turns out valid FAT filesystems can have zero there.
+	 */
+
+	if (!fat_valid_media(b->media)) {
+		if (!silent)
+			fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",
+				(unsigned)b->media);
+		goto out;
+	}
+
+	if (!is_power_of_2(bpb->fat_sector_size)
+	    || (bpb->fat_sector_size < 512)
+	    || (bpb->fat_sector_size > 4096)) {
+		if (!silent)
+			fat_msg(sb, KERN_ERR, "bogus logical sector size %u",
+			       (unsigned)bpb->fat_sector_size);
+		goto out;
+	}
+
+	if (!is_power_of_2(bpb->fat_sec_per_clus)) {
+		if (!silent)
+			fat_msg(sb, KERN_ERR, "bogus sectors per cluster %u",
+				(unsigned)bpb->fat_sec_per_clus);
+		goto out;
+	}
+
+	if (bpb->fat_fat_length == 0 && bpb->fat32_length == 0) {
+		if (!silent)
+			fat_msg(sb, KERN_ERR, "bogus number of FAT sectors");
+		goto out;
+	}
+
+	error = 0;
+
+out:
+	return error;
+}
+
+ /* end of Linux kernel code */
+ /*****************************************************************************/
+
+int determine_FAT_bits(struct fat_boot_sector *sector)
+{
+	struct fat_bios_param_block bpb;
+	if (fat_read_bpb(NULL, sector, !verbosity, &bpb)) {
+		return 0;
+	}
+	/*
+	 * at this point, the following assertions are true:
+	 * bpb.fat_sec_per_clus > 0
+	 * bpb.fat_sector_size > 0
+	 */
+
+	/* based on fat_fill_super() from the Linux kernel's fs/fat/inode.c */
+	if (!bpb.fat_fat_length && bpb.fat32_length) {
+		return 32;
+	} else {
+		unsigned short fat_start = bpb.fat_reserved;
+		unsigned char fats = bpb.fat_fats;
+		unsigned short fat_length = bpb.fat_fat_length;
+		unsigned long dir_start = fat_start + fats * fat_length;
+		unsigned short dir_entries = bpb.fat_dir_entries;
+		unsigned long blocksize = bpb.fat_sector_size;
+		u32 total_sectors = bpb.fat_sectors;
+		if (total_sectors == 0) {
+			total_sectors = bpb.fat_total_sect;
+		}
+		unsigned short sec_per_clus = bpb.fat_sec_per_clus;
+		u32 rootdir_sectors = dir_entries *
+				      sizeof(struct msdos_dir_entry) /
+				      blocksize;
+		unsigned long data_start = dir_start + rootdir_sectors;
+		u32 total_clusters =
+			(total_sectors - data_start) / sec_per_clus;
+		return (total_clusters > MAX_FAT12) ? 16 : 12;
+	}
+}

tools/fat.h

@@ -0,0 +1,25 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2023
+ *
+ * Author: Michael Adler <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+#pragma once
+
+#include <linux/msdos_fs.h>
+#include "ebgpart.h"
+
+/**
+ * Determines the number of FAT bits (12, 16, or 32) based on the provided fat_boot_sector.
+ *
+ * The function performs the necessary checks and validations on the provided boot sector
+ * to ensure it is a valid FAT boot sector. If the provided boot sector is not valid or an error
+ * occurs during the determination process, the function returns a value less than or equal to 0.
+ */
+int determine_FAT_bits(struct fat_boot_sector *sector);

tools/linux_util.h

@@ -0,0 +1,59 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2023
+ *
+ * Author: Michael Adler <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+#pragma once
+
+#include <stdint.h>
+#include <stdbool.h>
+
+/* Utility functions taken from the Linux kernel */
+
+#define le16_to_cpu __le16_to_cpu
+#define le32_to_cpu __le32_to_cpu
+
+typedef uint8_t u8;
+typedef uint16_t u16;
+typedef uint32_t u32;
+
+static inline uint16_t __get_unaligned_le16(const uint8_t *p)
+{
+	return p[0] | p[1] << 8;
+}
+
+static inline uint16_t get_unaligned_le16(const void *p)
+{
+	return __get_unaligned_le16((const uint8_t *)p);
+}
+
+static inline u32 __get_unaligned_le32(const u8 *p)
+{
+	return p[0] | p[1] << 8 | p[2] << 16 | p[3] << 24;
+}
+
+static inline u32 get_unaligned_le32(const void *p)
+{
+	return __get_unaligned_le32(p);
+}
+
+/**
+ * is_power_of_2() - check if a value is a power of two
+ * @n: the value to check
+ *
+ * Determine whether some value is a power of two, where zero is
+ * *not* considered a power of two.
+ * Return: true if @n is a power of 2, otherwise false.
+ */
+static inline __attribute__((const))
+bool is_power_of_2(unsigned long n)
+{
+	return (n != 0 && ((n & (n - 1)) == 0));
+}