[PATCH v2 1/2] fix: continue device probing after non-FAT partitions

[Michael Adler]

Resolves a regression from commit b23816ab9626 where efibootguard would
skip config file probing for an entire device upon encountering a
partition with an indeterminable FAT bit size.

Additionally, simplified the implementation and added documentation for
clarity.

Fixes: b23816ab9626 ("fix: correctly calculate FAT bit size based on cluster count")
Reported-by: Felix Moessbauer <felix.mo...@siemens.com>
Signed-off-by: Michael Adler <michae...@siemens.com>
---
tools/ebgpart.c | 69 ++++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index fc754df..9a9e179 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -44,7 +44,7 @@ static void add_block_dev(PedDevice *dev)
d->next = dev;
}

-static char *GUID_to_str(uint8_t *g)
+static char *GUID_to_str(const uint8_t *g)
{
(void)snprintf(buffer, 37,
"%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-"
@@ -73,26 +73,35 @@ static char *type_to_name(char t)
return "not supported";
}

-static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
- PedFileSystemType *pfst, uint32_t i)
+/**
+ * @brief Determines the FAT bit size of a disk or partition.
+ *
+ * @param fd File descriptor to inspect.
+ * @param e A pointer to the EFI partition entry.
+ * @param fat_size A pointer where the detected FAT bit size will be stored.
+ *
+ * @return 0 if the FAT bit size is successfully determined and stored in
+ * `fat_size`; otherwise, returns a non-zero error code.
+ */
+static int check_GPT_FAT_entry(int fd, const struct EFIpartitionentry *e,
+ int *fat_size)
{
char *guid_str = GUID_to_str(e->type_GUID);
if (strcmp(GPT_PARTITION_GUID_FAT_NTFS, guid_str) != 0 &&
strcmp(GPT_PARTITION_GUID_ESP, guid_str) != 0) {
- if (asprintf(&pfst->name, "%s", "not supported") == -1) {
- goto error_asprintf;
- }
VERBOSE(stderr, "GPT entry has unsupported GUID: %s\n",
guid_str);
- return true;
+ *fat_size = 0;
+ return 0;
}
- VERBOSE(stdout, "GPT Partition #%u is FAT/NTFS.\n", i);
+ VERBOSE(stdout, "GPT Partition has a FAT/NTFS GUID\n");
+
/* Save current file offset */
off64_t curr = lseek64(fd, 0, SEEK_CUR);
if (curr == -1) {
VERBOSE(stderr, "Error getting current seek position: %s\n",
strerror(errno));
- return false;
+ return -1;
}

/* seek to partition start */
@@ -100,7 +109,7 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
if (lseek64(fd, offset_start, SEEK_SET) == -1) {
VERBOSE(stderr, "Error seeking to partition start: %s\n",
strerror(errno));
- return false;
+ return -1;
}

/* read FAT header */
@@ -108,32 +117,17 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
if (read(fd, &header, sizeof(header)) != sizeof(header)) {
VERBOSE(stderr, "Error reading FAT header: %s\n",
strerror(errno));
- return false;
+ return -1;
}

/* 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;
+ return -1;
}
- VERBOSE(stdout, "GPT Partition #%u is %s.\n", i, pfst->name);
- return true;
-
-error_asprintf:
- VERBOSE(stderr, "Error in asprintf - possibly out of memory.\n");
- return false;
+ *fat_size = determine_FAT_bits(&header);
+ return 0;
}

static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num,
@@ -178,13 +172,28 @@ static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num,
tmpp->num = i + 1;
tmpp->fs_type = pfst;

- if (!check_GPT_FAT_entry(fd, &e, pfst, i)) {
+ int fat_size = 0;
+ if (check_GPT_FAT_entry(fd, &e, &fat_size)) {
+ VERBOSE(stderr,
+ "%u: error, skipping partitions on device\n",
+ i);
free(pfst->name);
free(pfst);
free(tmpp);
dev->part_list = NULL;
continue;
}
+ if (fat_size > 0) {
+ VERBOSE(stdout, "GPT Partition #%u is fat%d.\n", i,
+ fat_size);
+ if (asprintf(&pfst->name, "fat%d", fat_size) == -1) {
+ VERBOSE(stderr, "Error in asprintf - possibly "
+ "out of memory.\n");
+ return;
+ }
+ } else {
+ VERBOSE(stderr, "%u: not a FAT partition\n", i);
+ }

*list_end = tmpp;
list_end = &((*list_end)->next);
--
2.42.0

[Jan Kiszka]

Why not returning fat_size directly?

>0: FAT partition of detected size
=0: non-FAT partition
<0: disk access error

Jan

Siemens AG, Technology
Linux Expert Center

[Michael Adler]

Resolves a regression from commit b23816ab9626 where efibootguard would
skip config file probing for an entire device upon encountering a
partition with an indeterminable FAT bit size.

Additionally, simplified the implementation and added documentation for
clarity.

Fixes: b23816ab9626 ("fix: correctly calculate FAT bit size based on cluster count")
Reported-by: Felix Moessbauer <felix.mo...@siemens.com>
Signed-off-by: Michael Adler <michae...@siemens.com>
---

tools/ebgpart.c | 64 ++++++++++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index fc754df..814f769 100644

--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -44,7 +44,7 @@ static void add_block_dev(PedDevice *dev)
d->next = dev;
}

-static char *GUID_to_str(uint8_t *g)
+static char *GUID_to_str(const uint8_t *g)
{
(void)snprintf(buffer, 37,
"%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-"

@@ -73,26 +73,33 @@ static char *type_to_name(char t)

return "not supported";
}

-static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
- PedFileSystemType *pfst, uint32_t i)
+/**
+ * @brief Determines the FAT bit size of a disk or partition.
+ *
+ * @param fd File descriptor to inspect.
+ * @param e A pointer to the EFI partition entry.
+ *

+ * @return >0: FAT bit size of the disk or partition
+ * =0: non-FAT partition
+ * <0: I/O error
+ **/
+static int check_GPT_FAT_entry(int fd, const struct EFIpartitionentry *e)

{
char *guid_str = GUID_to_str(e->type_GUID);
if (strcmp(GPT_PARTITION_GUID_FAT_NTFS, guid_str) != 0 &&
strcmp(GPT_PARTITION_GUID_ESP, guid_str) != 0) {
- if (asprintf(&pfst->name, "%s", "not supported") == -1) {
- goto error_asprintf;
- }
VERBOSE(stderr, "GPT entry has unsupported GUID: %s\n",
guid_str);
- return true;

+ return 0;
}
- VERBOSE(stdout, "GPT Partition #%u is FAT/NTFS.\n", i);
+ VERBOSE(stdout, "GPT Partition has a FAT/NTFS GUID\n");
+
/* Save current file offset */
off64_t curr = lseek64(fd, 0, SEEK_CUR);
if (curr == -1) {
VERBOSE(stderr, "Error getting current seek position: %s\n",
strerror(errno));
- return false;
+ return -1;
}

/* seek to partition start */

@@ -100,7 +107,7 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,

if (lseek64(fd, offset_start, SEEK_SET) == -1) {
VERBOSE(stderr, "Error seeking to partition start: %s\n",
strerror(errno));
- return false;
+ return -1;
}

/* read FAT header */

@@ -108,32 +115,16 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,

+ return determine_FAT_bits(&header);

}

static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num,

@@ -178,13 +169,26 @@ static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num,

tmpp->num = i + 1;
tmpp->fs_type = pfst;

- if (!check_GPT_FAT_entry(fd, &e, pfst, i)) {

+ int result = check_GPT_FAT_entry(fd, &e);
+ if (result < 0) {
+ VERBOSE(stderr, "%u: I/O error, skipping device\n", i);

free(pfst->name);
free(pfst);
free(tmpp);
dev->part_list = NULL;
continue;
}

+ if (result > 0) { /* result is the FAT bit size */

+ VERBOSE(stdout, "GPT Partition #%u is fat%d.\n", i,

+ result);
+ if (asprintf(&pfst->name, "fat%d", result) == -1) {

+ VERBOSE(stderr, "Error in asprintf - possibly "
+ "out of memory.\n");
+ return;
+ }
+ } else {
+ VERBOSE(stderr, "%u: not a FAT partition\n", i);
+ }

*list_end = tmpp;
list_end = &((*list_end)->next);
--

2.42.0

[Michael Adler]

- Transition from string-based to enum-based file system type handling.
- Eliminates string allocations, reducing potential for allocation failures.
- Avoids error-prone string matching, improving performance and accuracy.

Signed-off-by: Michael Adler <michae...@siemens.com>
---

env/env_config_partitions.c | 7 ++-
include/ebgpart.h | 13 ++++--
tools/ebgpart.c | 90 +++++++++++++------------------------
tools/tests/fake_devices.c | 16 +------
4 files changed, 46 insertions(+), 80 deletions(-)

diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c
index 358c365..dd91a3f 100644
--- a/env/env_config_partitions.c
+++ b/env/env_config_partitions.c
@@ -121,10 +121,9 @@ bool probe_config_partitions(CONFIG_PART *cfgpart, bool search_all_devices)
}
PedPartition *part = pd->part_list;
while (part) {
- if (!part->fs_type || !part->fs_type->name ||
- (strcmp(part->fs_type->name, "fat12") != 0 &&
- strcmp(part->fs_type->name, "fat16") != 0 &&
- strcmp(part->fs_type->name, "fat32") != 0)) {
+ if (part->fs_type != FS_TYPE_FAT12 &&
+ part->fs_type != FS_TYPE_FAT16 &&
+ part->fs_type != FS_TYPE_FAT32) {
part = ped_disk_next_partition(pd, part);
continue;
}
diff --git a/include/ebgpart.h b/include/ebgpart.h
index 65b2d1a..cf3d403 100644
--- a/include/ebgpart.h
+++ b/include/ebgpart.h
@@ -106,13 +106,18 @@ struct EFIpartitionentry {
};
#pragma pack(pop)

+typedef enum {
+ FS_TYPE_UNKNOWN = 0,
+ FS_TYPE_FAT12 = 1,
+ FS_TYPE_FAT16 = 2,
+ FS_TYPE_FAT32 = 3,
+ FS_TYPE_EXTENDED = 4
+} EbgFileSystemType;
+
/* Implementing a minimalistic API replacing used libparted functions */
-typedef struct _PedFileSystemType {
- char *name;
-} PedFileSystemType;

typedef struct _PedPartition {
- PedFileSystemType *fs_type;
+ EbgFileSystemType fs_type;
uint16_t num;
struct _PedPartition *next;
} PedPartition;
diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index 814f769..fba7fb2 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -54,23 +54,23 @@ static char *GUID_to_str(const uint8_t *g)
return buffer;
}

-static char *type_to_name(char t)
+static EbgFileSystemType type_to_fstype(char t)
{
switch (t) {
case MBR_TYPE_FAT12:
- return "fat12";
+ return FS_TYPE_FAT12;
case MBR_TYPE_FAT16A:
case MBR_TYPE_FAT16:
case MBR_TYPE_FAT16_LBA:
- return "fat16";
+ return FS_TYPE_FAT16;
case MBR_TYPE_FAT32:
case MBR_TYPE_FAT32_LBA:
- return "fat32";
+ return FS_TYPE_FAT32;
case MBR_TYPE_EXTENDED_LBA:
case MBR_TYPE_EXTENDED:
- return "extended";
+ return FS_TYPE_EXTENDED;
}
- return "not supported";
+ return FS_TYPE_UNKNOWN;
}

/**
@@ -127,13 +127,33 @@ static int check_GPT_FAT_entry(int fd, const struct EFIpartitionentry *e)
return determine_FAT_bits(&header);
}

+static inline EbgFileSystemType fat_size_to_fs_type(int fat_size)
+{
+ switch (fat_size) {
+ case 0:
+ VERBOSE(stderr, "Not a FAT partition\n");
+ return FS_TYPE_UNKNOWN;
+ case 12:
+ VERBOSE(stdout, "Partition is fat12\n");
+ return FS_TYPE_FAT12;
+ case 16:
+ VERBOSE(stdout, "Partition is fat16\n");
+ return FS_TYPE_FAT16;
+ case 32:
+ VERBOSE(stdout, "Partition is fat32\n");
+ return FS_TYPE_FAT32;
+ default:
+ VERBOSE(stderr, "Error: Invalid FAT size %d\n", fat_size);
+ return FS_TYPE_UNKNOWN;
+ }
+}
+

static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num,

PedDevice *dev)
{
off64_t offset;
struct EFIpartitionentry e;
PedPartition *tmpp;
- PedFileSystemType *pfst = NULL;

offset = LB_SIZE * table_LBA;
if (lseek64(fd, offset, SEEK_SET) != offset) {
@@ -154,41 +174,22 @@ static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num,
return;
}
VERBOSE(stdout, "%u: %s\n", i, GUID_to_str(e.type_GUID));
- pfst = calloc(sizeof(PedFileSystemType), 1);
- if (!pfst) {
- VERBOSE(stderr, "Out of memory\n");
- return;
- }

tmpp = calloc(sizeof(PedPartition), 1);
if (!tmpp) {
VERBOSE(stderr, "Out of memory\n");
- free(pfst);
return;

}
tmpp->num = i + 1;

- tmpp->fs_type = pfst;

int result = check_GPT_FAT_entry(fd, &e);

if (result < 0) {

VERBOSE(stderr, "%u: I/O error, skipping device\n", i);

- free(pfst->name);
- free(pfst);

free(tmpp);
dev->part_list = NULL;
continue;
}

- if (result > 0) { /* result is the FAT bit size */
- VERBOSE(stdout, "GPT Partition #%u is fat%d.\n", i,
- result);
- if (asprintf(&pfst->name, "fat%d", result) == -1) {

- VERBOSE(stderr, "Error in asprintf - possibly "

- "out of memory.\n");
- return;
- }
- } else {
- VERBOSE(stderr, "%u: not a FAT partition\n", i);
- }
+ tmpp->fs_type = fat_size_to_fs_type(result);

*list_end = tmpp;
list_end = &((*list_end)->next);

@@ -200,7 +201,6 @@ static void scanLogicalVolumes(int fd, off64_t extended_start_LBA,
PedPartition *partition, int lognum)
{
struct Masterbootrecord next_ebr;
- PedFileSystemType *pfst = NULL;

off64_t offset = extended_start_LBA + ebr->parttable[i].start_LBA;
if (extended_start_LBA == 0) {
@@ -238,21 +238,13 @@ static void scanLogicalVolumes(int fd, off64_t extended_start_LBA,
if (!partition->next) {
goto scl_out_of_mem;
}
- pfst = calloc(sizeof(PedFileSystemType), 1);
- if (!pfst) {
- goto scl_out_of_mem;
- }
- if (asprintf(&pfst->name, "%s", type_to_name(t)) == -1) {
- goto scl_out_of_mem;
- };
partition = partition->next;
partition->num = lognum;
- partition->fs_type = pfst;
+ partition->fs_type = type_to_fstype(t);
}
return;
scl_out_of_mem:
VERBOSE(stderr, "Out of memory\n");
- free(pfst);
free(partition->next);
}

@@ -321,26 +313,18 @@ static bool check_partition_table(PedDevice *dev)
efihdr.partitions, dev);
break;
}
- PedFileSystemType *pfst = calloc(sizeof(PedFileSystemType), 1);
- if (!pfst) {
- goto cpt_out_of_mem;
- }
-
tmp = calloc(sizeof(PedPartition), 1);
if (!tmp) {
goto cpt_out_of_mem;
}

tmp->num = i + 1;
- tmp->fs_type = pfst;

*list_end = tmp;
list_end = &((*list_end)->next);

if (t == MBR_TYPE_EXTENDED || t == MBR_TYPE_EXTENDED_LBA) {
- if (asprintf(&pfst->name, "%s", "extended") == -1) {
- goto cpt_out_of_mem;
- }
+ tmp->fs_type = FS_TYPE_EXTENDED;
scanLogicalVolumes(fd, 0, &mbr, i, tmp, 5);
/* Could be we still have MBR entries after
* logical volumes */
@@ -348,15 +332,12 @@ static bool check_partition_table(PedDevice *dev)
list_end = &((*list_end)->next);
}
} else {
- if (asprintf(&pfst->name, "%s", type_to_name(t)) == -1) {
- goto cpt_out_of_mem;
- }
+ tmp->fs_type = type_to_fstype(t);
}
continue;
cpt_out_of_mem:
close(fd);
VERBOSE(stderr, "Out of mem while checking partition table\n.");
- free(pfst);
free(tmp);
return false;
}
@@ -494,15 +475,8 @@ pedprobe_error:
closedir(sysblockdir);
}

-static void ped_partition_destroy(PedPartition *p)
+static inline void ped_partition_destroy(PedPartition *p)
{
- if (!p) {
- return;
- }
- if (p->fs_type) {
- free(p->fs_type->name);
- free(p->fs_type);
- }
free(p);
}

diff --git a/tools/tests/fake_devices.c b/tools/tests/fake_devices.c
index 090d720..3fc94b3 100644
--- a/tools/tests/fake_devices.c
+++ b/tools/tests/fake_devices.c
@@ -16,7 +16,7 @@
#include <env_api.h>
#include <env_config_file.h>
#include <env_config_partitions.h>
-#include <fake_devices.h>
+#include "fake_devices.h"

PedDevice *fake_devices;
int num_fake_devices;
@@ -64,15 +64,7 @@ void add_fake_partition(int devnum)
goto allocate_fake_part_error;
}
(*pp)->num = num;
- (*pp)->fs_type =
- (PedFileSystemType *)calloc(1, sizeof(PedFileSystemType));
- if (!(*pp)->fs_type) {
- goto allocate_fake_part_error;
- }
- if (asprintf(&(*pp)->fs_type->name, "%s", "fat16") == -1) {
- (*pp)->fs_type->name = NULL;
- goto allocate_fake_part_error;
- }
+ (*pp)->fs_type = FS_TYPE_FAT16;
return;

allocate_fake_part_error:
@@ -86,10 +78,6 @@ void remove_fake_partitions(int n)
PedPartition *next;
while(pp) {
next = pp->next;
- if (pp->fs_type) {
- free(pp->fs_type->name);
- free(pp->fs_type);
- }
free(pp);
pp = next;
}
--
2.42.0

[Jan Kiszka]

Thanks, both applied. Nice cleanup here.

Jan

[JEMS EBERHARD HORBEL]

DIRECT SENDER IS HERE LETS DEAL.

JENS EBERHARD



MT103/202 DIRECT WIRE TRANSFER
PAYPAL TRANSFER
CASHAPP TRANSFER
ZELLE TRANSFER
TRANSFER WISE
WESTERN UNION TRANSFER
BITCOIN FLASHING 
BANK ACCOUNT LOADING/FLASHING
IBAN TO IBAN TRANSFER
MONEYGRAM TRANSFER
SLBC PROVIDER
CREDIT CARD TOP UP
SEPA TRANSFER
WIRE TRANSFER
GLOBALPAY INC US

Thanks.


NOTE; ONLY SERIOUS / RELIABLE RECEIVERS CAN CONTACT.

DM ME ON WHATSAPP FOR A SERIOUS DEAL.

+447405129573