From 89126c1ab582a32d22bea59cd29d81bee0765bb1 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 27 Sep 2017 16:13:57 -0400 Subject: [PATCH] fallback: find_boot_options(): make the allocation path prettier. 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 --- fallback.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/fallback.c b/fallback.c index 341092c..6e4ba19 100644 --- a/fallback.c +++ b/fallback.c @@ -850,24 +850,24 @@ find_boot_options(EFI_HANDLE device) do { bs = 0; rc = uefi_call_wrapper(fh2->Read, 3, fh2, &bs, NULL); - if (rc == EFI_BUFFER_TOO_SMALL || - (rc == EFI_SUCCESS && bs != 0)) { - buffer = AllocateZeroPool(bs); - if (!buffer) { - Print(L"Could not allocate memory\n"); - /* sure, this might work, why not? */ - uefi_call_wrapper(fh2->Close, 1, fh2); - uefi_call_wrapper(fh->Close, 1, fh); - return EFI_OUT_OF_RESOURCES; - } - - rc = uefi_call_wrapper(fh2->Read, 3, fh2, &bs, buffer); + if (EFI_ERROR(rc) && rc != EFI_BUFFER_TOO_SMALL) { + Print(L"Could not read \\EFI\\: %d\n", rc); + return rc; } if (bs == 0) break; + buffer = AllocateZeroPool(bs); + if (!buffer) { + Print(L"Could not allocate memory\n"); + /* sure, this might work, why not? */ + uefi_call_wrapper(fh2->Close, 1, fh2); + uefi_call_wrapper(fh->Close, 1, fh); + return EFI_OUT_OF_RESOURCES; + } + + rc = uefi_call_wrapper(fh2->Read, 3, fh2, &bs, buffer); if (EFI_ERROR(rc)) { - Print(L"Could not read \\EFI\\: %d\n", rc); if (buffer) { FreePool(buffer); buffer = NULL;