From 2c5f9938addac2d259c9db37f3f119cc9cd4c6ac Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Tue, 12 Nov 2013 10:30:02 -0500 Subject: fallback.c: fix 32-bit compilation fh->Read expects pointer to 32-bit int, use UINTN Change-Id: If1a728efd51a9a24dfcd8123e84bf4c0713491fe Signed-off-by: Andrew Boie --- fallback.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index 82ddbf2f..c8751448 100644 --- a/fallback.c +++ b/fallback.c @@ -15,7 +15,7 @@ EFI_LOADED_IMAGE *this_image = NULL; static EFI_STATUS -get_file_size(EFI_FILE_HANDLE fh, UINT64 *retsize) +get_file_size(EFI_FILE_HANDLE fh, UINTN *retsize) { EFI_STATUS rc; void *buffer = NULL; @@ -60,7 +60,7 @@ read_file(EFI_FILE_HANDLE fh, CHAR16 *fullpath, CHAR16 **buffer, UINT64 *bs) return rc; } - UINT64 len = 0; + UINTN len = 0; CHAR16 *b = NULL; rc = get_file_size(fh2, &len); if (EFI_ERROR(rc)) { -- cgit v1.2.3 From dd77b3447cb4ee3c80659265f9ea084af8829ae4 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 21 Nov 2013 11:48:24 -0500 Subject: Rewrite directory traversal allocation path so coverity can grok it. The things we do for our tools. In this case, make the AllocatePool() happen outside of a conditional, even though that conditional will always bee satisfied. This way coverity won't think we're setting fi to NULL and passing it to StrCaseCmp. Signed-off-by: Peter Jones --- fallback.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index c8751448..ba864eec 100644 --- a/fallback.c +++ b/fallback.c @@ -445,25 +445,32 @@ find_boot_csv(EFI_FILE_HANDLE fh, CHAR16 *dirname) return EFI_SUCCESS; } FreePool(buffer); + buffer = NULL; bs = 0; do { bs = 0; rc = uefi_call_wrapper(fh->Read, 3, fh, &bs, NULL); - if (rc == EFI_BUFFER_TOO_SMALL) { - buffer = AllocateZeroPool(bs); - if (!buffer) { - Print(L"Could not allocate memory\n"); - return EFI_OUT_OF_RESOURCES; - } + if (EFI_ERROR(rc) && rc != EFI_BUFFER_TOO_SMALL) { + Print(L"Could not read \\EFI\\%s\\: %d\n", dirname, rc); + if (buffer) + FreePool(buffer); + return rc; + } - rc = uefi_call_wrapper(fh->Read, 3, fh, &bs, buffer); + buffer = AllocateZeroPool(bs); + if (!buffer) { + Print(L"Could not allocate memory\n"); + return EFI_OUT_OF_RESOURCES; } + + rc = uefi_call_wrapper(fh->Read, 3, fh, &bs, buffer); if (EFI_ERROR(rc)) { Print(L"Could not read \\EFI\\%s\\: %d\n", dirname, rc); FreePool(buffer); return rc; } + if (bs == 0) break; -- cgit v1.2.3 From 6edc6ec0a3da59e4519dec3f8af6fd48be00b980 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Fri, 31 Jan 2014 10:30:36 -0500 Subject: [fallback] For HD() device paths, use just the media node and later. UEFI 2.x section 3.1.2 provides for "short-form device path", where the first element specified is a "hard drive media device path", so that you can move a disk around on different buses without invalidating your device path. Fallback has not been using this option, though in most cases efibootmgr has. Note that we still keep the full device path, because LoadImage() isn't necessarily the layer where HD() works - one some systems BDS is responsible for resolving the full path and passes that to LoadImage() instead. So we have to do LoadImage() with the full path. --- fallback.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 78 insertions(+), 25 deletions(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index ba864eec..a12bb741 100644 --- a/fallback.c +++ b/fallback.c @@ -14,6 +14,27 @@ EFI_LOADED_IMAGE *this_image = NULL; +static EFI_STATUS +FindSubDevicePath(EFI_DEVICE_PATH *In, UINT8 Type, UINT8 SubType, + EFI_DEVICE_PATH **Out) +{ + EFI_DEVICE_PATH *dp = In; + if (!In || !Out) + return EFI_INVALID_PARAMETER; + + for (dp = In; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) { + if (DevicePathType(dp) == Type && + DevicePathSubType(dp) == SubType) { + *Out = DuplicateDevicePath(dp); + if (!*Out) + return EFI_OUT_OF_RESOURCES; + return EFI_SUCCESS; + } + } + *Out = NULL; + return EFI_NOT_FOUND; +} + static EFI_STATUS get_file_size(EFI_FILE_HANDLE fh, UINTN *retsize) { @@ -93,7 +114,9 @@ make_full_path(CHAR16 *dirname, CHAR16 *filename, CHAR16 **out, UINT64 *outlen) { UINT64 len; - len = StrLen(dirname) + StrLen(filename) + StrLen(L"\\EFI\\\\") + 2; + len = StrLen(L"\\EFI\\") + StrLen(dirname) + + StrLen(L"\\") + StrLen(filename) + + 2; CHAR16 *fullpath = AllocateZeroPool(len*sizeof(CHAR16)); if (!fullpath) { @@ -119,7 +142,8 @@ VOID *first_new_option_args = NULL; UINTN first_new_option_size = 0; EFI_STATUS -add_boot_option(EFI_DEVICE_PATH *dp, CHAR16 *filename, CHAR16 *label, CHAR16 *arguments) +add_boot_option(EFI_DEVICE_PATH *hddp, EFI_DEVICE_PATH *fulldp, + CHAR16 *filename, CHAR16 *label, CHAR16 *arguments) { static int i = 0; CHAR16 varname[] = L"Boot0000"; @@ -136,24 +160,31 @@ add_boot_option(EFI_DEVICE_PATH *dp, CHAR16 *filename, CHAR16 *label, CHAR16 *ar void *var = LibGetVariable(varname, &global); if (!var) { int size = sizeof(UINT32) + sizeof (UINT16) + - StrLen(label)*2 + 2 + DevicePathSize(dp) + - StrLen(arguments) * 2 + 2; + StrLen(label)*2 + 2 + DevicePathSize(hddp) + + StrLen(arguments) * 2; CHAR8 *data = AllocateZeroPool(size); CHAR8 *cursor = data; *(UINT32 *)cursor = LOAD_OPTION_ACTIVE; cursor += sizeof (UINT32); - *(UINT16 *)cursor = DevicePathSize(dp); + *(UINT16 *)cursor = DevicePathSize(hddp); cursor += sizeof (UINT16); StrCpy((CHAR16 *)cursor, label); cursor += StrLen(label)*2 + 2; - CopyMem(cursor, dp, DevicePathSize(dp)); - cursor += DevicePathSize(dp); + CopyMem(cursor, hddp, DevicePathSize(hddp)); + cursor += DevicePathSize(hddp); StrCpy((CHAR16 *)cursor, arguments); Print(L"Creating boot entry \"%s\" with label \"%s\" " L"for file \"%s\"\n", varname, label, filename); + + if (!first_new_option) { + first_new_option = DuplicateDevicePath(fulldp); + first_new_option_args = arguments; + first_new_option_size = StrLen(arguments) * sizeof (CHAR16); + } + rc = uefi_call_wrapper(RT->SetVariable, 5, varname, &global, EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | @@ -254,7 +285,10 @@ add_to_boot_list(EFI_FILE_HANDLE fh, CHAR16 *dirname, CHAR16 *filename, CHAR16 * if (EFI_ERROR(rc)) return rc; - EFI_DEVICE_PATH *dph = NULL, *dpf = NULL, *dp = NULL; + EFI_DEVICE_PATH *dph = NULL; + EFI_DEVICE_PATH *file = NULL; + EFI_DEVICE_PATH *full_device_path = NULL; + EFI_DEVICE_PATH *dp = NULL; dph = DevicePathFromHandle(this_image->DeviceHandle); if (!dph) { @@ -262,19 +296,31 @@ add_to_boot_list(EFI_FILE_HANDLE fh, CHAR16 *dirname, CHAR16 *filename, CHAR16 * goto err; } - dpf = FileDevicePath(fh, fullpath); - if (!dpf) { + file = FileDevicePath(fh, fullpath); + if (!file) { rc = EFI_OUT_OF_RESOURCES; goto err; } - dp = AppendDevicePath(dph, dpf); - if (!dp) { + full_device_path = AppendDevicePath(dph, file); + if (!full_device_path) { rc = EFI_OUT_OF_RESOURCES; goto err; } + rc = FindSubDevicePath(full_device_path, + MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP, &dp); + if (EFI_ERROR(rc)) { + if (rc == EFI_NOT_FOUND) { + dp = full_device_path; + } else { + rc = EFI_OUT_OF_RESOURCES; + goto err; + } + } + #ifdef DEBUG_FALLBACK + { UINTN s = DevicePathSize(dp); int i; UINT8 *dpv = (void *)dp; @@ -287,20 +333,16 @@ add_to_boot_list(EFI_FILE_HANDLE fh, CHAR16 *dirname, CHAR16 *filename, CHAR16 * CHAR16 *dps = DevicePathToStr(dp); Print(L"device path: \"%s\"\n", dps); -#endif - if (!first_new_option) { - CHAR16 *dps = DevicePathToStr(dp); - Print(L"device path: \"%s\"\n", dps); - first_new_option = DuplicateDevicePath(dp); - first_new_option_args = arguments; - first_new_option_size = StrLen(arguments) * sizeof (CHAR16); } +#endif - add_boot_option(dp, fullpath, label, arguments); + add_boot_option(dp, full_device_path, fullpath, label, arguments); err: - if (dpf) - FreePool(dpf); + if (file) + FreePool(file); + if (full_device_path) + FreePool(full_device_path); if (dp) FreePool(dp); if (fullpath) @@ -629,8 +671,19 @@ try_start_first_option(EFI_HANDLE parent_image_handle) first_new_option, NULL, 0, &image_handle); if (EFI_ERROR(rc)) { - Print(L"LoadImage failed: %d\n", rc); - uefi_call_wrapper(BS->Stall, 1, 2000000); + CHAR16 *dps = DevicePathToStr(first_new_option); + UINTN s = DevicePathSize(first_new_option); + int i; + UINT8 *dpv = (void *)first_new_option; + Print(L"LoadImage failed: %d\nDevice path: \"%s\"\n", rc, dps); + for (i = 0; i < s; i++) { + if (i > 0 && i % 16 == 0) + Print(L"\n"); + Print(L"%02x ", dpv[i]); + } + Print(L"\n"); + + uefi_call_wrapper(BS->Stall, 1, 500000000); return rc; } @@ -644,7 +697,7 @@ try_start_first_option(EFI_HANDLE parent_image_handle) rc = uefi_call_wrapper(BS->StartImage, 3, image_handle, NULL, NULL); if (EFI_ERROR(rc)) { Print(L"StartImage failed: %d\n", rc); - uefi_call_wrapper(BS->Stall, 1, 2000000); + uefi_call_wrapper(BS->Stall, 1, 500000000); } return rc; } -- cgit v1.2.3 From 9fcd221ef143df5a52671a6f23240eb246ccf448 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Fri, 31 Jan 2014 10:31:10 -0500 Subject: [fallback] Attempt to re-use existing entries when possible. Some firmwares seem to ignore our boot entries and put their fallback entries back on top. Right now that results in a lot of boot entries for our stuff, a la https://bugzilla.redhat.com/show_bug.cgi?id=995834 . Instead of that happening, if we simply find existing entries that match the entry we would create and move them to the top of the boot order, the machine will continue to operate in failure mode (which we can't avoid), but at least we won't create thousands of extra entries. Signed-off-by: Peter Jones --- fallback.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index a12bb741..44638ec0 100644 --- a/fallback.c +++ b/fallback.c @@ -225,6 +225,85 @@ add_boot_option(EFI_DEVICE_PATH *hddp, EFI_DEVICE_PATH *fulldp, return EFI_OUT_OF_RESOURCES; } +EFI_STATUS +find_boot_option(EFI_DEVICE_PATH *dp, CHAR16 *filename, CHAR16 *label, + CHAR16 *arguments, UINT16 *optnum) +{ + int size = sizeof(UINT32) + sizeof (UINT16) + + StrLen(label)*2 + 2 + DevicePathSize(dp) + + StrLen(arguments) * 2 + 2; + + CHAR8 *data = AllocateZeroPool(size); + if (!data) + return EFI_OUT_OF_RESOURCES; + CHAR8 *cursor = data; + *(UINT32 *)cursor = LOAD_OPTION_ACTIVE; + cursor += sizeof (UINT32); + *(UINT16 *)cursor = DevicePathSize(dp); + cursor += sizeof (UINT16); + StrCpy((CHAR16 *)cursor, label); + cursor += StrLen(label)*2 + 2; + CopyMem(cursor, dp, DevicePathSize(dp)); + cursor += DevicePathSize(dp); + StrCpy((CHAR16 *)cursor, arguments); + + int i = 0; + CHAR16 varname[] = L"Boot0000"; + CHAR16 hexmap[] = L"0123456789ABCDEF"; + EFI_GUID global = EFI_GLOBAL_VARIABLE; + EFI_STATUS rc; + + CHAR8 *candidate = AllocateZeroPool(size); + if (!candidate) { + FreePool(data); + return EFI_OUT_OF_RESOURCES; + } + + for(i = 0; i < nbootorder && i < 0x10000; i++) { + varname[4] = hexmap[(bootorder[i] & 0xf000) >> 12]; + varname[5] = hexmap[(bootorder[i] & 0x0f00) >> 8]; + varname[6] = hexmap[(bootorder[i] & 0x00f0) >> 4]; + varname[7] = hexmap[(bootorder[i] & 0x000f) >> 0]; + + UINTN candidate_size = size; + rc = uefi_call_wrapper(RT->GetVariable, 5, varname, &global, + NULL, &candidate_size, candidate); + if (EFI_ERROR(rc)) + continue; + + if (candidate_size != size) + continue; + + if (CompareMem(candidate, data, size)) + continue; + + /* at this point, we have duplicate data. */ + *optnum = i; + FreePool(candidate); + FreePool(data); + return EFI_SUCCESS; + } + FreePool(candidate); + FreePool(data); + return EFI_NOT_FOUND; +} + +EFI_STATUS +set_boot_order(void) +{ + CHAR16 *oldbootorder; + UINTN size; + EFI_GUID global = EFI_GLOBAL_VARIABLE; + + oldbootorder = LibGetVariableAndSize(L"BootOrder", &global, &size); + if (oldbootorder) { + nbootorder = size / sizeof (CHAR16); + bootorder = oldbootorder; + } + return EFI_SUCCESS; + +} + EFI_STATUS update_boot_order(void) { @@ -336,7 +415,23 @@ add_to_boot_list(EFI_FILE_HANDLE fh, CHAR16 *dirname, CHAR16 *filename, CHAR16 * } #endif - add_boot_option(dp, full_device_path, fullpath, label, arguments); + UINT16 option; + rc = find_boot_option(dp, fullpath, label, arguments, &option); + if (EFI_ERROR(rc)) { + add_boot_option(dp, full_device_path, fullpath, label, arguments); + } else if (option != 0) { + CHAR16 *newbootorder; + newbootorder = AllocateZeroPool(sizeof (CHAR16) * nbootorder); + if (!newbootorder) + return EFI_OUT_OF_RESOURCES; + + newbootorder[0] = bootorder[option]; + CopyMem(newbootorder + 1, bootorder, sizeof (CHAR16) * option); + CopyMem(newbootorder + option + 1, bootorder + option + 1, + sizeof (CHAR16) * (nbootorder - option - 1)); + FreePool(bootorder); + bootorder = newbootorder; + } err: if (file) @@ -717,6 +812,8 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab) Print(L"System BootOrder not found. Initializing defaults.\n"); + set_boot_order(); + rc = find_boot_options(this_image->DeviceHandle); if (EFI_ERROR(rc)) { Print(L"Error: could not find boot options: %d\n", rc); -- cgit v1.2.3 From 47a9d2c908078ff79c4a4043855ec499241c8977 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 11 Apr 2014 14:41:22 -0400 Subject: additional bounds-checking on section sizes This adds additional bounds-checking on the section sizes. Also adds -Wsign-compare to the Makefile and replaces some signed variables with unsigned counteparts for robustness. Signed-off-by: Kees Cook --- Makefile | 3 ++- MokManager.c | 6 ++--- PasswordCrypt.c | 4 +-- fallback.c | 4 +-- shim.c | 83 +++++++++++++++++++++++++++++++++++++++------------------ 5 files changed, 66 insertions(+), 34 deletions(-) (limited to 'fallback.c') diff --git a/Makefile b/Makefile index e65d28d3..46e5ef93 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,8 @@ EFI_LDS = elf_$(ARCH)_efi.lds DEFAULT_LOADER := \\\\grub.efi CFLAGS = -ggdb -O0 -fno-stack-protector -fno-strict-aliasing -fpic \ - -fshort-wchar -Wall -Werror -mno-red-zone -maccumulate-outgoing-args \ + -fshort-wchar -Wall -Wsign-compare -Werror \ + -mno-red-zone -maccumulate-outgoing-args \ -mno-mmx -mno-sse -fno-builtin \ "-DDEFAULT_LOADER=L\"$(DEFAULT_LOADER)\"" \ "-DDEFAULT_LOADER_CHAR=\"$(DEFAULT_LOADER)\"" \ diff --git a/MokManager.c b/MokManager.c index f5ed379c..3da61f43 100644 --- a/MokManager.c +++ b/MokManager.c @@ -440,7 +440,7 @@ static EFI_STATUS list_keys (void *KeyList, UINTN KeyListSize, CHAR16 *title) MokListNode *keys = NULL; INTN key_num = 0; CHAR16 **menu_strings; - int i; + unsigned int i; if (KeyListSize < (sizeof(EFI_SIGNATURE_LIST) + sizeof(EFI_SIGNATURE_DATA))) { @@ -491,7 +491,7 @@ static EFI_STATUS list_keys (void *KeyList, UINTN KeyListSize, CHAR16 *title) static UINT8 get_line (UINT32 *length, CHAR16 *line, UINT32 line_max, UINT8 show) { EFI_INPUT_KEY key; - int count = 0; + unsigned int count = 0; do { key = console_get_keystroke(); @@ -640,7 +640,7 @@ static EFI_STATUS match_password (PASSWORD_CRYPT *pw_crypt, CHAR16 password[PASSWORD_MAX]; UINT32 pw_length; UINT8 fail_count = 0; - int i; + unsigned int i; if (pw_crypt) { auth_hash = pw_crypt->hash; diff --git a/PasswordCrypt.c b/PasswordCrypt.c index 8d72a821..e0a82cfd 100644 --- a/PasswordCrypt.c +++ b/PasswordCrypt.c @@ -154,7 +154,7 @@ static EFI_STATUS sha256_crypt (const char *key, UINT32 key_len, CopyMem(cp, tmp_result, cnt); SHA256_Init(&alt_ctx); - for (cnt = 0; cnt < 16 + alt_result[0]; ++cnt) + for (cnt = 0; cnt < 16ul + alt_result[0]; ++cnt) SHA256_Update(&alt_ctx, salt, salt_size); SHA256_Final(tmp_result, &alt_ctx); @@ -242,7 +242,7 @@ static EFI_STATUS sha512_crypt (const char *key, UINT32 key_len, CopyMem(cp, tmp_result, cnt); SHA512_Init(&alt_ctx); - for (cnt = 0; cnt < 16 + alt_result[0]; ++cnt) + for (cnt = 0; cnt < 16ul + alt_result[0]; ++cnt) SHA512_Update(&alt_ctx, salt, salt_size); SHA512_Final(tmp_result, &alt_ctx); diff --git a/fallback.c b/fallback.c index 44638ec0..bc9a3c9d 100644 --- a/fallback.c +++ b/fallback.c @@ -229,7 +229,7 @@ EFI_STATUS find_boot_option(EFI_DEVICE_PATH *dp, CHAR16 *filename, CHAR16 *label, CHAR16 *arguments, UINT16 *optnum) { - int size = sizeof(UINT32) + sizeof (UINT16) + + unsigned int size = sizeof(UINT32) + sizeof (UINT16) + StrLen(label)*2 + 2 + DevicePathSize(dp) + StrLen(arguments) * 2 + 2; @@ -768,7 +768,7 @@ try_start_first_option(EFI_HANDLE parent_image_handle) if (EFI_ERROR(rc)) { CHAR16 *dps = DevicePathToStr(first_new_option); UINTN s = DevicePathSize(first_new_option); - int i; + unsigned int i; UINT8 *dpv = (void *)first_new_option; Print(L"LoadImage failed: %d\nDevice path: \"%s\"\n", rc, dps); for (i = 0; i < s; i++) { diff --git a/shim.c b/shim.c index 0e18d387..8c583a4e 100644 --- a/shim.c +++ b/shim.c @@ -102,7 +102,7 @@ typedef struct { /* * Perform basic bounds checking of the intra-image pointers */ -static void *ImageAddress (void *image, int size, unsigned int address) +static void *ImageAddress (void *image, unsigned int size, unsigned int address) { if (address > size) return NULL; @@ -494,18 +494,19 @@ static BOOLEAN secure_mode (void) * Calculate the SHA1 and SHA256 hashes of a binary */ -static EFI_STATUS generate_hash (char *data, int datasize, +static EFI_STATUS generate_hash (char *data, int datasize_in, PE_COFF_LOADER_IMAGE_CONTEXT *context, UINT8 *sha256hash, UINT8 *sha1hash) { unsigned int sha256ctxsize, sha1ctxsize; - unsigned int size = datasize; + unsigned int size = datasize_in; void *sha256ctx = NULL, *sha1ctx = NULL; char *hashbase; unsigned int hashsize; unsigned int SumOfBytesHashed, SumOfSectionBytes; unsigned int index, pos; + unsigned int datasize; EFI_IMAGE_SECTION_HEADER *Section; EFI_IMAGE_SECTION_HEADER *SectionHeader = NULL; EFI_IMAGE_SECTION_HEADER *SectionCache; @@ -517,6 +518,12 @@ static EFI_STATUS generate_hash (char *data, int datasize, sha1ctxsize = Sha1GetContextSize(); sha1ctx = AllocatePool(sha1ctxsize); + if (datasize_in < 0) { + Print(L"Invalid data size\n"); + return EFI_INVALID_PARAMETER; + } + size = datasize = (unsigned int)datasize_in; + if (!sha256ctx || !sha1ctx) { Print(L"Unable to allocate memory for hash context\n"); return EFI_OUT_OF_RESOURCES; @@ -577,22 +584,29 @@ static EFI_STATUS generate_hash (char *data, int datasize, SumOfBytesHashed = context->PEHdr->Pe32.OptionalHeader.SizeOfHeaders; #endif - Section = (EFI_IMAGE_SECTION_HEADER *) ( - (char *)context->PEHdr + sizeof (UINT32) + - sizeof (EFI_IMAGE_FILE_HEADER) + - context->PEHdr->Pe32.FileHeader.SizeOfOptionalHeader - ); - - SectionCache = Section; - + /* Validate section locations and sizes */ for (index = 0, SumOfSectionBytes = 0; index < context->PEHdr->Pe32.FileHeader.NumberOfSections; index++, SectionCache++) { - SumOfSectionBytes += SectionCache->SizeOfRawData; - } - - if (SumOfSectionBytes >= datasize) { - Print(L"Malformed binary: %x %x\n", SumOfSectionBytes, size); - status = EFI_INVALID_PARAMETER; - goto done; + EFI_IMAGE_SECTION_HEADER *SectionPtr; + + /* Validate SectionPtr is within image */ + SectionPtr = ImageAddress(data, datasize, + sizeof (UINT32) + + sizeof (EFI_IMAGE_FILE_HEADER) + + context->PEHdr->Pe32.FileHeader.SizeOfOptionalHeader + + (index * sizeof(*SectionPtr))); + if (!SectionPtr) { + Print(L"Malformed section %d\n", index); + status = EFI_INVALID_PARAMETER; + goto done; + } + /* Validate section size is within image. */ + if (SectionPtr->SizeOfRawData > + datasize - SumOfBytesHashed - SumOfSectionBytes) { + Print(L"Malformed section %d size\n", index); + status = EFI_INVALID_PARAMETER; + goto done; + } + SumOfSectionBytes += SectionPtr->SizeOfRawData; } SectionHeader = (EFI_IMAGE_SECTION_HEADER *) AllocateZeroPool (sizeof (EFI_IMAGE_SECTION_HEADER) * context->PEHdr->Pe32.FileHeader.NumberOfSections); @@ -602,6 +616,11 @@ static EFI_STATUS generate_hash (char *data, int datasize, goto done; } + /* Already validated above */ + Section = ImageAddress(data, datasize, sizeof (UINT32) + + sizeof (EFI_IMAGE_FILE_HEADER) + + context->PEHdr->Pe32.FileHeader.SizeOfOptionalHeader); + /* Sort the section headers */ for (index = 0; index < context->PEHdr->Pe32.FileHeader.NumberOfSections; index++) { pos = index; @@ -620,7 +639,6 @@ static EFI_STATUS generate_hash (char *data, int datasize, continue; } hashbase = ImageAddress(data, size, Section->PointerToRawData); - hashsize = (unsigned int) Section->SizeOfRawData; if (!hashbase) { Print(L"Malformed section header\n"); @@ -628,6 +646,15 @@ static EFI_STATUS generate_hash (char *data, int datasize, goto done; } + /* Verify hashsize within image. */ + if (Section->SizeOfRawData > + datasize - Section->PointerToRawData) { + Print(L"Malformed section raw size %d\n", index); + status = EFI_INVALID_PARAMETER; + goto done; + } + hashsize = (unsigned int) Section->SizeOfRawData; + if (!(Sha256Update(sha256ctx, hashbase, hashsize)) || !(Sha1Update(sha1ctx, hashbase, hashsize))) { Print(L"Unable to generate hash\n"); @@ -638,10 +665,10 @@ static EFI_STATUS generate_hash (char *data, int datasize, } /* Hash all remaining data */ - if (size > SumOfBytesHashed) { + if (datasize > SumOfBytesHashed) { hashbase = data + SumOfBytesHashed; hashsize = (unsigned int)( - size - + datasize - #if __LP64__ context->PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size - #else @@ -884,7 +911,8 @@ static EFI_STATUS read_header(void *data, unsigned int datasize, return EFI_UNSUPPORTED; } - if (((UINT8 *)context->SecDir - (UINT8 *)data) > (datasize - sizeof(EFI_IMAGE_DATA_DIRECTORY))) { + if ((unsigned long)((UINT8 *)context->SecDir - (UINT8 *)data) > + (datasize - sizeof(EFI_IMAGE_DATA_DIRECTORY))) { Print(L"Invalid image\n"); return EFI_UNSUPPORTED; } @@ -904,7 +932,8 @@ static EFI_STATUS handle_image (void *data, unsigned int datasize, { EFI_STATUS efi_status; char *buffer; - int i, size; + int i; + unsigned int size; EFI_IMAGE_SECTION_HEADER *Section; char *base, *end; PE_COFF_LOADER_IMAGE_CONTEXT context; @@ -1081,7 +1110,8 @@ static EFI_STATUS generate_path(EFI_LOADED_IMAGE *li, CHAR16 *ImagePath, { EFI_DEVICE_PATH *devpath; EFI_HANDLE device; - int i, j, last = -1; + unsigned int i; + int j, last = -1; unsigned int pathlen = 0; EFI_STATUS efi_status = EFI_SUCCESS; CHAR16 *bootpath; @@ -1637,9 +1667,10 @@ EFI_STATUS set_second_stage (EFI_HANDLE image_handle) EFI_STATUS status; EFI_LOADED_IMAGE *li; CHAR16 *start = NULL, *c; - int i, remaining_size = 0; + unsigned int i; + int remaining_size = 0; CHAR16 *loader_str = NULL; - int loader_len = 0; + unsigned int loader_len = 0; second_stage = DEFAULT_LOADER; load_options = NULL; -- cgit v1.2.3 From ec7eddbf05abd6b721b1f1f0fa6c1816d35ddc09 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 13 May 2014 13:23:41 -0400 Subject: [fallback] Avoid duplicate old BootOrder set_boot_order() already copies the old BootOrder to the variable, bootorder. Besides, we can adjust BootOrder when adding the newly generated boot option. So, we don't have to copy the old one again in update_boot_order(). This avoid the duplicate entries in BootOrder. Signed-off-by: Gary Ching-Pang Lin --- fallback.c | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index bc9a3c9d..4bde9c10 100644 --- a/fallback.c +++ b/fallback.c @@ -204,12 +204,12 @@ add_boot_option(EFI_DEVICE_PATH *hddp, EFI_DEVICE_PATH *fulldp, return EFI_OUT_OF_RESOURCES; int j = 0; + newbootorder[0] = i & 0xffff; if (nbootorder) { for (j = 0; j < nbootorder; j++) - newbootorder[j] = bootorder[j]; + newbootorder[j+1] = bootorder[j]; FreePool(bootorder); } - newbootorder[j] = i & 0xffff; bootorder = newbootorder; nbootorder += 1; #ifdef DEBUG_FALLBACK @@ -307,28 +307,17 @@ set_boot_order(void) EFI_STATUS update_boot_order(void) { - CHAR16 *oldbootorder; UINTN size; + UINTN len = 0; EFI_GUID global = EFI_GLOBAL_VARIABLE; CHAR16 *newbootorder = NULL; + EFI_STATUS rc; - oldbootorder = LibGetVariableAndSize(L"BootOrder", &global, &size); - if (oldbootorder) { - int n = size / sizeof (CHAR16) + nbootorder; - - newbootorder = AllocateZeroPool(n * sizeof (CHAR16)); - if (!newbootorder) - return EFI_OUT_OF_RESOURCES; - CopyMem(newbootorder, bootorder, nbootorder * sizeof (CHAR16)); - CopyMem(newbootorder + nbootorder, oldbootorder, size); - size = n * sizeof (CHAR16); - } else { - size = nbootorder * sizeof(CHAR16); - newbootorder = AllocateZeroPool(size); - if (!newbootorder) - return EFI_OUT_OF_RESOURCES; - CopyMem(newbootorder, bootorder, size); - } + size = nbootorder * sizeof(CHAR16); + newbootorder = AllocateZeroPool(size); + if (!newbootorder) + return EFI_OUT_OF_RESOURCES; + CopyMem(newbootorder, bootorder, size); #ifdef DEBUG_FALLBACK Print(L"nbootorder: %d\nBootOrder: ", size / sizeof (CHAR16)); @@ -337,13 +326,11 @@ update_boot_order(void) Print(L"%04x ", newbootorder[j]); Print(L"\n"); #endif - - if (oldbootorder) { + rc = uefi_call_wrapper(RT->GetVariable, 5, L"BootOrder", &global, + NULL, &len, NULL); + if (rc == EFI_BUFFER_TOO_SMALL) LibDeleteVariable(L"BootOrder", &global); - FreePool(oldbootorder); - } - EFI_STATUS rc; rc = uefi_call_wrapper(RT->SetVariable, 5, L"BootOrder", &global, EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | -- cgit v1.2.3 From 30cead3b403650d320926971b671b4cda3d609e5 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 13 May 2014 13:24:12 -0400 Subject: [fallback] Fix the data size for boot option comparison Signed-off-by: Gary Ching-Pang Lin --- fallback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index 4bde9c10..7f242e1c 100644 --- a/fallback.c +++ b/fallback.c @@ -231,7 +231,7 @@ find_boot_option(EFI_DEVICE_PATH *dp, CHAR16 *filename, CHAR16 *label, { unsigned int size = sizeof(UINT32) + sizeof (UINT16) + StrLen(label)*2 + 2 + DevicePathSize(dp) + - StrLen(arguments) * 2 + 2; + StrLen(arguments) * 2; CHAR8 *data = AllocateZeroPool(size); if (!data) -- cgit v1.2.3 From 8bf83b55dc252cc6b5b83bb0f33cdc8fb68c448b Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 13 May 2014 13:30:07 -0400 Subject: [fallback] Try to boot the first boot option anyway Some UEFI implementations never care the boot options, so the restored boot options could be just ignored and this results in endless reboot. To avoid this situation, this commit makes fallback.efi to load the first matched boot option even if there is no boot option to be restored. It may not be perfect, but at least the bootloader is loaded... Signed-off-by: Gary Ching-Pang Lin --- fallback.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index 7f242e1c..d10fb629 100644 --- a/fallback.c +++ b/fallback.c @@ -226,8 +226,9 @@ add_boot_option(EFI_DEVICE_PATH *hddp, EFI_DEVICE_PATH *fulldp, } EFI_STATUS -find_boot_option(EFI_DEVICE_PATH *dp, CHAR16 *filename, CHAR16 *label, - CHAR16 *arguments, UINT16 *optnum) +find_boot_option(EFI_DEVICE_PATH *dp, EFI_DEVICE_PATH *fulldp, + CHAR16 *filename, CHAR16 *label, CHAR16 *arguments, + UINT16 *optnum) { unsigned int size = sizeof(UINT32) + sizeof (UINT16) + StrLen(label)*2 + 2 + DevicePathSize(dp) + @@ -278,6 +279,12 @@ find_boot_option(EFI_DEVICE_PATH *dp, CHAR16 *filename, CHAR16 *label, continue; /* at this point, we have duplicate data. */ + if (!first_new_option) { + first_new_option = DuplicateDevicePath(fulldp); + first_new_option_args = arguments; + first_new_option_size = StrLen(arguments) * sizeof (CHAR16); + } + *optnum = i; FreePool(candidate); FreePool(data); @@ -403,7 +410,7 @@ add_to_boot_list(EFI_FILE_HANDLE fh, CHAR16 *dirname, CHAR16 *filename, CHAR16 * #endif UINT16 option; - rc = find_boot_option(dp, fullpath, label, arguments, &option); + rc = find_boot_option(dp, full_device_path, fullpath, label, arguments, &option); if (EFI_ERROR(rc)) { add_boot_option(dp, full_device_path, fullpath, label, arguments); } else if (option != 0) { -- cgit v1.2.3