summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2018-04-05Audit get_variable() calls for correct FreePool() use.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-05Fix get_variable() usage in setup_verbosity()Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-05Make setup_console(-1) do GetMode() and call it from setup_verbosity()Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-05Make handle_image() use console_print() not console_notify() on successPeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-05Fix lib/ rebuild-on-change dependencies in the MakefilePeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-05Get rid of dprinta(), it's uselessPeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-04tpm_log_event_raw(): be more careful about EFI_NOT_FOUNDPeter Jones
Don't return EFI_NOT_FOUND from tpm_log_event*() unless we're in REQUIRE_TPM mode. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-04Make the 'something has gone seriously wrong' message less ambiguousPeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-04read_header(): fix the case where signatures have been removed.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-04-04Add another TODO for shim-16Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-23Add some TODO items for shim-16Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-23.travis.yml: update travis to get newer gnu-efi.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-23Revert "Allow shim to handle multiple trusted certificates"Peter Jones
This was merged before it was really ready - verify_trusted_cert needs to check each certificate against vendor_dbx, "dbx", and "MokListX", or else it can enable a blacklisted certificate accidentally. This reverts commit 8721bbe6fb1bfdfbc8bd16e05673929e4cbbdedc.
2018-03-20Revert "MokManager: stop using StrnCat"Peter Jones
This reverts commit 6aa5a62515d62139a2d3b34626fac8910e864a3d. Everything Hans said was correct. But StrnCat() is in gnu-efi 3.0.8, and using just StrCpy() here confuses coverity. I'd rather have a CI page that's not completely full of chaff, but a little bit of redundancy in the code. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-19Fix i386 pointer type error.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-15Avoid a minor scan-build complaint.Peter Jones
scan-build doesn't like it when we assign return values but don't use them. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-15Work around clang bugs for scan-build.Peter Jones
I don't think the x86 binaries clang builds will actually work unless they just infer -maccumulate-outgoing-args from __attribute__((__ms_abi__), but it's nice to have the analyzer working. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-14travis: Fix a typoPeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-14Fix the working directory we start in.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-14Add some configs for CI using github+travis+dockerPeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-14Allow shim to handle multiple trusted certificatesMichael Brown
Allow shim to perform verification against a list of trusted certificates by simply concatenating the DER files. Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
2018-03-14Allow memory allocated by handle_image() to be freedMichael Brown
There is currently no way for a caller of handle_image() to free the memory allocated to hold the relocated executable. Fix by adding the allocated memory address and number of pages as returned parameters from handle_image(). Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
2018-03-14Remove global entry_point variableMichael Brown
Treat entry_point as a returned parameter from handle_image(), rather than using a global variable. Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
2018-03-14Do not modify original imageMichael Brown
relocate_coff() currently modifies the PE header within the raw data. This appears to be unnecessary, and causes a verification failure if a second attempt is made to verify the same data buffer. Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
2018-03-13MokManager: stop using StrnCatHans de Goede
StrnCat is not available in gnu-efi-3.0.5 (I did not check if it does actually exists in 3.0.6). Moreover using strcat on a buffer where we've just done: "buf[0] = '\0'" is a bit silly, we might as well drop the 0 termination and just use strcpy. It seems there also is no StrnCpy in gnu-efi-3.0.5, but we are passing in a pointer to the end of file_name minus 4, so strcpy will consume only 4 bytes anyways and there is no need for the "n". Signed-off-by: Hans de Goede <hdegoede@redhat.com>
2018-03-13console: Fix indentationHans de Goede
The manual merge of the "console: Do not set EFI console to textmode until something is printed" patch has lead to a bunch of tabs being replaced with 7 spaces. This commit fixes this. Signed-off-by: Hans de Goede <hdegoede@redhat.com>
2018-03-12console: Do not set EFI console to textmode until something is printedHans de Goede
Remove the setup_console(1) calls from shim and instead make lib/console.c make that call when necessary. This avoids shim forcing the EFI console to switch to text-mode if nothing is printed. This commit also modifies MokManager to work the same way for consistency, even though MokManager will always print something. Signed-off-by: Hans de Goede <hdegoede@redhat.com>
2018-03-12console: Move setup_console() definition to higher in the fileHans de Goede
This is a preparation patch for making setup_console() private. Signed-off-by: Hans de Goede <hdegoede@redhat.com>
2018-03-12console: Add console_print and console_print_at helpersHans de Goede
This is a preparation commit for removing the setup_console(1) calls from MokManager and shim so that we don't force the EFI console to switch to text-mode. This commit replaces all direct calls to Print / PrintAt with calls to the new helpers (no functional changes) so that we can delay calling setup_console(1) till the first Print call in a follow-up patch. Signed-off-by: Hans de Goede <hdegoede@redhat.com>
2018-03-12Fix syntastic config for include/Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: Don't overwrite EFI_LOADED_IMAGE's LoadOptions when not neededTamas K Lengyel
When the firmware is using EFI_LOAD_OPTION to specify options for the secondary loader, the shim will properly detect that and return in set_second_stage. Later howerer in handle_image EFI_LOADED_IMAGE is being overwritten with load_option irrespective of the fact that load_option was never set. This effectively prevents the EFI_LOAD_OPTION from reaching the secondary loader. Only overwrite EFI_LOADED_IMAGE's LoadOptions when load_option is not NULL solves the problem. Signed-off-by: Tamas K Lengyel <lengyelt@ainfosec.com>
2018-03-12Make EFI_INCLUDE path configurable during makeTamas K Lengyel
Signed-off-by: Tamas K Lengyel <lengyelt@ainfosec.com>
2018-03-12Make sure fallback.efi gets errlog.o as wellPeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12Fix a minor merge error.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: Make our variable validation and mirroring table driven.Peter Jones
This makes it so shim's idea of Mok variables all resides in one table of data, and we don't need a bunch of nearly identical ad-hoc functions to handle each of them. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: make everything use a common perror() call.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: Improve the bounds checking of ImageAddress()Peter Jones
Make ImageAddress() directly check for overflow in its math. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: main(): Don't save the value from mok_ignore_db()Peter Jones
We don't really care if setting MokIgnoreDB fails; don't save the return. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: generate_hash(): make clang-analyzer not get confused.Peter Jones
clang-analyzer thinks that because we're not checking for NULL from ImageAddress() it can be NULL, but doesn't realize we've already checked that value once before. Check it again, it can't hurt. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: Mitigate a minor clang-analyzer complaint.Peter Jones
Clang believes "SumOfBytesHashed" is never used, because the thing that uses that computed value is #if 0'd currently. Just swizzle the #if's so that line is also not compiled. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: tpm.c: Make sure old_caps is initialized in tpm2 code.Peter Jones
clang-analyzer caught this. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12Don't use uefi_call_wrapper(), ever.Peter Jones
I'm pretty done with typing uefi_call_wrapper() and counting arguments every time. Instead, just make the compiler error if we don't have ms_abi. Also, make it so nothing can use uefi_call_wrapper() directly. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: Use EFI_ERROR() instead of comparing to EFI_SUCCESS everywhere.Peter Jones
Also consistently name our status variable "efi_status" unless there's a good reason not to, such as already having another one of those. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: relocate_coff(): get rid of "FixupData" stuffPeter Jones
"FixupData" in the edk2 tree is a log of the relocations that happened, which is allocated by the "client" calling relocate, and written into while it does relocations. Since we never allocate that log anywhere, FixupData is always NULL, and so covscan says: 318 case EFI_IMAGE_REL_BASED_HIGH: 319 Fixup16 = (UINT16 *) Fixup; 320 *Fixup16 = (UINT16) (*Fixup16 + ((UINT16) ((UINT32) Adjust >> 16))); null: At condition FixupData != NULL, the value of FixupData must be NULL. dead_error_condition: The condition FixupData != NULL cannot be true. 321 if (FixupData != NULL) { CID 182859 (#1 of 4): Logically dead code (DEADCODE)dead_error_begin: Execution cannot reach this statement: *((UINT16 *)FixupData) = *F.... 322 *(UINT16 *) FixupData = *Fixup16; 323 FixupData = FixupData + sizeof (UINT16); 324 } 325 break; And it's right; all four occurrances are deadcode that never do anything but confuse the reader. Kill it with fire. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: check_db_cert_in_ram(): clear openssl errors /before/ returning.Peter Jones
Covscan says: 455 if (IsFound) { 456 tpm_measure_variable(dbname, guid, CertSize, Cert->SignatureData); 457 return DATA_FOUND; CID 182850 (#1 of 1): Structurally dead code (UNREACHABLE)unreachable: This code cannot be reached: drain_openssl_errors();. 458 drain_openssl_errors(); 459 } else { 460 LogError(L"AuthenticodeVerify(): %d\n", IsFound); 461 } And, well... woops. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12shim: ensure generate_hash() never operates on a negative (signed) number.Peter Jones
Covscan noticed: 746static EFI_STATUS generate_hash (char *data, unsigned int datasize_in, 747 PE_COFF_LOADER_IMAGE_CONTEXT *context, 748 UINT8 *sha256hash, UINT8 *sha1hash) 749 750{ ... 764 CID 182849 (#1 of 1): Unsigned compared against 0 (NO_EFFECT)unsigned_compare: This less-than-zero comparison of an unsigned value is never true. datasize_in < 0U. 765 if (datasize_in < 0) { 766 perror(L"Invalid data size\n"); 767 return EFI_INVALID_PARAMETER; 768 } And I guess that's a fair point, but some of the callers take the size as a signed integer. So we should be handling that on all the input cases instead of getting that far. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12fallback: find_boot_options(): don't leak a file handle.Peter Jones
If we open it, we have to close it. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12fallback: Use EFI_ERROR() instead of comparing to EFI_SUCCESS everywhere.Peter Jones
Also consistently name our status variable "efi_status" unless there's a good reason not to, such as already having another one of those. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12fallback: find_boot_options(): make the allocation path prettier.Peter Jones
Covscan believes all this stuff: 852 bs = 0; 853 rc = uefi_call_wrapper(fh2->Read, 3, fh2, &bs, NULL); 7. Condition rc == (9223372036854775813UL /* 0x8000000000000000UL | 5 */), taking false branch. 8. Condition rc == 0, taking false branch. 15. Condition rc == (9223372036854775813UL /* 0x8000000000000000UL | 5 */), taking false branch. 16. Condition rc == 0, taking true branch. 17. Condition bs != 0, taking true branch. 30. Condition rc == (9223372036854775813UL /* 0x8000000000000000UL | 5 */), taking false branch. 31. Condition rc == 0, taking false branch. 854 if (rc == EFI_BUFFER_TOO_SMALL || 855 (rc == EFI_SUCCESS && bs != 0)) { 856 buffer = AllocateZeroPool(bs); 18. Condition !buffer, taking false branch. 857 if (!buffer) { 858 Print(L"Could not allocate memory\n"); 859 /* sure, this might work, why not? */ 860 uefi_call_wrapper(fh2->Close, 1, fh2); 861 uefi_call_wrapper(fh->Close, 1, fh); 862 return EFI_OUT_OF_RESOURCES; 863 } 864 865 rc = uefi_call_wrapper(fh2->Read, 3, fh2, &bs, buffer); 866 } 9. Condition bs == 0, taking false branch. 19. Condition bs == 0, taking false branch. 32. Condition bs == 0, taking false branch. 867 if (bs == 0) 868 break; 869 10. Condition (INTN)rc < 0, taking false branch. 20. Condition (INTN)rc < 0, taking false branch. 33. Condition (INTN)rc < 0, taking false branch. 870 if (EFI_ERROR(rc)) { 871 Print(L"Could not read \\EFI\\: %d\n", rc); 872 if (buffer) { 873 FreePool(buffer); 874 buffer = NULL; 875 } 876 uefi_call_wrapper(fh2->Close, 1, fh2); 877 uefi_call_wrapper(fh->Close, 1, fh); 878 return rc; 879 } 34. alias_transfer: Assigning: fi = buffer. 880 EFI_FILE_INFO *fi = buffer; 881 11. Condition !(fi->Attribute & 16), taking false branch. 21. Condition !(fi->Attribute & 16), taking false branch. CID 182858 (#1-3 of 3): Explicit null dereferenced (FORWARD_NULL)35. var_deref_op: Dereferencing null pointer fi. 882 if (!(fi->Attribute & EFI_FILE_DIRECTORY)) { 883 FreePool(buffer); 884 buffer = NULL; 885 continue; 886 } Because it doesn't know that when bs==0, fh2->Read() will return EFI_BUFFER_TOO_SMALL and set bs to the size we need to allocate, so the allocation path is always taken. Instead, handle our exit/error paths directly there, and make the allocation path nonconditional. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12fallback: find_boot_csv(): eliminate dead code.Peter Jones
Covscan sez: 720 FreePool(buffer); assignment: Assigning: buffer = NULL. 721 buffer = NULL; 722 723 CHAR16 *bootcsv=NULL, *bootarchcsv=NULL; 724 725 bs = 0; 726 do { 727 bs = 0; 728 rc = uefi_call_wrapper(fh->Read, 3, fh, &bs, NULL); 729 if (EFI_ERROR(rc) && rc != EFI_BUFFER_TOO_SMALL) { 730 Print(L"Could not read \\EFI\\%s\\: %d\n", dirname, rc); null: At condition buffer, the value of buffer must be NULL. dead_error_condition: The condition buffer cannot be true. 731 if (buffer) CID 182851 (#1 of 1): Logically dead code (DEADCODE)dead_error_line: Execution cannot reach this statement: FreePool(buffer);. 732 FreePool(buffer); 733 return rc; 734 } And it's right; buffer can never be non-NULL there. So just take that out. Signed-off-by: Peter Jones <pjones@redhat.com>