On 09.08.23 11:15, 'Michael Adler' via EFI Boot Guard wrote: > 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]> > --- > Makefile.am | 3 +- > tools/ebgpart.c | 63 +++++++--------- > tools/fat.c | 179 +++++++++++++++++++++++++++++++++++++++++++++ > tools/fat.h | 25 +++++++ > tools/linux_util.h | 59 +++++++++++++++ > 5 files changed, 291 insertions(+), 38 deletions(-) > create mode 100644 tools/fat.c > create mode 100644 tools/fat.h > create mode 100644 tools/linux_util.h > > diff --git a/Makefile.am b/Makefile.am > index ef18ffd..f1c777e 100644 > --- a/Makefile.am > +++ b/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) \ > diff --git a/tools/ebgpart.c b/tools/ebgpart.c > index e058793..18f440f 100644 > --- a/tools/ebgpart.c > +++ b/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,42 @@ 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; > + memset(&header, 0, sizeof(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: > diff --git a/tools/fat.c b/tools/fat.c > new file mode 100644 > index 0000000..0ac8ff5 > --- /dev/null > +++ b/tools/fat.c > @@ -0,0 +1,179 @@ > +/* > + * 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, ...) VERBOSE(stderr, __VA_ARGS__) > +#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; > + } > + > + /* 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; > + } > +} > diff --git a/tools/fat.h b/tools/fat.h > new file mode 100644 > index 0000000..4e77757 > --- /dev/null > +++ b/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); > diff --git a/tools/linux_util.h b/tools/linux_util.h > new file mode 100644 > index 0000000..fcdc1fa > --- /dev/null > +++ b/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)); > +}
Thanks, applied. Jan -- Siemens AG, Technology Linux Expert Center -- You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/5f84b0ec-91e7-4441-92af-e3c695b06f0e%40siemens.com.
