From ff19ad888d2a7a9f390e492841763d5b8d1061c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 8 Mar 2019 10:49:10 +0100 Subject: efi: wrap some long lines --- src/boot/efi/boot.c | 56 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index c7ba088761..ca9ce671d1 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2028,62 +2028,79 @@ static VOID config_load_xbootldr( EFI_PARTITION_TABLE_HEADER gpt_header; uint8_t space[((sizeof(EFI_PARTITION_TABLE_HEADER) + 511) / 512) * 512]; } gpt_header_buffer; + const EFI_PARTITION_TABLE_HEADER *h = &gpt_header_buffer.gpt_header; UINT64 where; UINTN i, sz; UINT32 c; if (nr == 0) - where = 1; /* Read the first copy at LBA 1 */ + /* Read the first copy at LBA 1 */ + where = 1; else - where = block_io->Media->LastBlock; /* Read the second copy at the very last LBA of this block device */ + /* Read the second copy at the very last LBA of this block device */ + where = block_io->Media->LastBlock; /* Read the GPT header */ - r = uefi_call_wrapper(block_io->ReadBlocks, 5, block_io, block_io->Media->MediaId, where, sizeof(gpt_header_buffer), &gpt_header_buffer); + r = uefi_call_wrapper(block_io->ReadBlocks, 5, + block_io, + block_io->Media->MediaId, + where, + sizeof(gpt_header_buffer), &gpt_header_buffer); if (EFI_ERROR(r)) continue; /* Some superficial validation of the GPT header */ - if (CompareMem(&gpt_header_buffer.gpt_header.Header.Signature, "EFI PART", sizeof(gpt_header_buffer.gpt_header.Header.Signature)) != 0) + c = CompareMem(&h->Header.Signature, "EFI PART", sizeof(h->Header.Signature)); + if (c != 0) continue; - if (gpt_header_buffer.gpt_header.Header.HeaderSize < 92 || gpt_header_buffer.gpt_header.Header.HeaderSize > 512) + if (h->Header.HeaderSize < 92 || + h->Header.HeaderSize > 512) continue; - if (gpt_header_buffer.gpt_header.Header.Revision != 0x00010000U) + if (h->Header.Revision != 0x00010000U) continue; /* Calculate CRC check */ - c = ~crc32_exclude_offset((UINT32) -1, (const UINT8*) &gpt_header_buffer, gpt_header_buffer.gpt_header.Header.HeaderSize, - OFFSETOF(EFI_PARTITION_TABLE_HEADER, Header.CRC32), sizeof(gpt_header_buffer.gpt_header.Header.CRC32)); - if (c != gpt_header_buffer.gpt_header.Header.CRC32) + c = ~crc32_exclude_offset((UINT32) -1, + (const UINT8*) &gpt_header_buffer, + h->Header.HeaderSize, + OFFSETOF(EFI_PARTITION_TABLE_HEADER, Header.CRC32), + sizeof(h->Header.CRC32)); + if (c != h->Header.CRC32) continue; - if (gpt_header_buffer.gpt_header.MyLBA != where) + if (h->MyLBA != where) continue; - if (gpt_header_buffer.gpt_header.SizeOfPartitionEntry < sizeof(EFI_PARTITION_ENTRY)) + if (h->SizeOfPartitionEntry < sizeof(EFI_PARTITION_ENTRY)) continue; - if (gpt_header_buffer.gpt_header.NumberOfPartitionEntries <= 0 || gpt_header_buffer.gpt_header.NumberOfPartitionEntries > 1024) + if (h->NumberOfPartitionEntries <= 0 || + h->NumberOfPartitionEntries > 1024) continue; /* Now load the GPT entry table */ - sz = ((gpt_header_buffer.gpt_header.SizeOfPartitionEntry * gpt_header_buffer.gpt_header.NumberOfPartitionEntries + 511) / 512) * 512; + sz = ((h->SizeOfPartitionEntry * h->NumberOfPartitionEntries + 511) / 512) * 512; entries = AllocatePool(sz); - r = uefi_call_wrapper(block_io->ReadBlocks, 5, block_io, block_io->Media->MediaId, gpt_header_buffer.gpt_header.PartitionEntryLBA, sz, entries); + r = uefi_call_wrapper(block_io->ReadBlocks, 5, + block_io, + block_io->Media->MediaId, + h->PartitionEntryLBA, + sz, entries); if (EFI_ERROR(r)) continue; /* Calculate CRC of entries array, too */ c = ~crc32((UINT32) -1, entries, sz); - if (c != gpt_header_buffer.gpt_header.PartitionEntryArrayCRC32) + if (c != h->PartitionEntryArrayCRC32) continue; - for (i = 0; i < gpt_header_buffer.gpt_header.NumberOfPartitionEntries; i++) { + for (i = 0; i < h->NumberOfPartitionEntries; i++) { EFI_PARTITION_ENTRY *entry; - entry = (EFI_PARTITION_ENTRY*) ((UINT8*) entries + gpt_header_buffer.gpt_header.SizeOfPartitionEntry * i); + entry = (EFI_PARTITION_ENTRY*) ((UINT8*) entries + h->SizeOfPartitionEntry * i); if (CompareMem(&entry->PartitionTypeGUID, xbootldr_guid, 16) == 0) { UINT64 end; @@ -2355,7 +2372,10 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { UINT64 osind = (UINT64)*b; if (osind & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) - config_entry_add_call(&config, L"auto-reboot-to-firmware-setup", L"Reboot Into Firmware Interface", reboot_into_firmware); + config_entry_add_call(&config, + L"auto-reboot-to-firmware-setup", + L"Reboot Into Firmware Interface", + reboot_into_firmware); FreePool(b); } -- cgit v1.2.1 From 7a2cb0228c2f1b7d95f6be7a751d1074d03e9cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 8 Mar 2019 14:16:40 +0100 Subject: boot: avoid 32-bit calculation for a 64-bit lvalue Coverity CID#1399116: > Potentially overflowing expression > gpt_header_buffer.gpt_header.SizeOfPartitionEntry * gpt_header_buffer.gpt_header.NumberOfPartitionEntries > with type unsigned int (32 bits, unsigned) is evaluated using 32-bit > arithmetic, and then used in a context that expects an expression of type > UINTN (64 bits, unsigned). Let's import the ALIGN_TO macro to sd-boot and use it to avoid the issue. --- src/boot/efi/boot.c | 5 ++++- src/boot/efi/util.h | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index ca9ce671d1..7b3e782454 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2080,8 +2080,11 @@ static VOID config_load_xbootldr( h->NumberOfPartitionEntries > 1024) continue; + if (h->SizeOfPartitionEntry > UINTN_MAX / h->NumberOfPartitionEntries) /* overflow check */ + continue; + /* Now load the GPT entry table */ - sz = ((h->SizeOfPartitionEntry * h->NumberOfPartitionEntries + 511) / 512) * 512; + sz = ALIGN_TO((UINTN) h->SizeOfPartitionEntry * (UINTN) h->NumberOfPartitionEntries, 512); entries = AllocatePool(sz); r = uefi_call_wrapper(block_io->ReadBlocks, 5, diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 8c5e35ad25..cef127f400 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -7,6 +7,10 @@ #define ELEMENTSOF(x) (sizeof(x)/sizeof((x)[0])) #define OFFSETOF(x,y) __builtin_offsetof(x,y) +static inline UINTN ALIGN_TO(UINTN l, UINTN ali) { + return ((l + ali - 1) & ~(ali - 1)); +} + static inline const CHAR16 *yes_no(BOOLEAN b) { return b ? L"yes" : L"no"; } -- cgit v1.2.1 From 388d2993ec48ad1c8249b42f4494dfd473723518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 8 Mar 2019 14:37:26 +0100 Subject: shared/bootspec: avoid going through -1 when calculating array index Coverity was complaining in CID#1399407 that config->entries might be used while NULL. Let's add an assert to make sure it's not. Also, let's quit early if we have no entries to loop through. The code was not incorrect, but it's cleaner to avoid any negative indices. --- src/shared/bootspec.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index afcf6f7ac1..64b2574a18 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -584,6 +584,12 @@ static int boot_entries_select_default(const BootConfig *config) { int i; assert(config); + assert(config->entries || config->n_entries == 0); + + if (config->n_entries == 0) { + log_debug("Found no default boot entry :("); + return -1; /* -1 means "no default" */ + } if (config->entry_oneshot) for (i = config->n_entries - 1; i >= 0; i--) @@ -609,12 +615,8 @@ static int boot_entries_select_default(const BootConfig *config) { return i; } - if (config->n_entries > 0) - log_debug("Found default: last entry \"%s\"", config->entries[config->n_entries - 1].id); - else - log_debug("Found no default boot entry :("); - - return config->n_entries - 1; /* -1 means "no default" */ + log_debug("Found default: last entry \"%s\"", config->entries[config->n_entries - 1].id); + return config->n_entries - 1; } int boot_entries_load_config( -- cgit v1.2.1