From 05458d227ff88e12397fc1226b48d5f59c368b31 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 27 Sep 2017 13:05:16 -0400 Subject: fallback: handle buffer allocations for fh->GetInfo() prettier. At all the places we use fh->GetInfo, covscan can't tell that fh->GetInfo() will return EFI_BUFFER_TOO_SMALL and we'll allocate on the first try. If we just explicitly check for "buffer == NULL" as well, covscan believes we're doing work we don't need to (which is true!) So instead, put an rc test to return error for everything else there, so the allocation isn't in a conditional. Yet another stupid one, but it's easier to nerf it this way than write the false-positive rule, and it also hardens against incorrect UEFI implementations (though we've not seen any yet with the problem this avoids). Signed-off-by: Peter Jones --- fallback.c | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) (limited to 'fallback.c') diff --git a/fallback.c b/fallback.c index a58c7d14..f46923db 100644 --- a/fallback.c +++ b/fallback.c @@ -102,18 +102,20 @@ get_file_size(EFI_FILE_HANDLE fh, UINTN *retsize) /* The API here is "Call it once with bs=0, it fills in bs, * then allocate a buffer and ask again to get it filled. */ rc = uefi_call_wrapper(fh->GetInfo, 4, fh, &EFI_FILE_INFO_GUID, &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; - } - rc = uefi_call_wrapper(fh->GetInfo, 4, fh, &EFI_FILE_INFO_GUID, - &bs, buffer); + if (EFI_ERROR(rc) && rc != EFI_BUFFER_TOO_SMALL) + return rc; + if (bs == 0) + return EFI_SUCCESS; + + buffer = AllocateZeroPool(bs); + if (!buffer) { + Print(L"Could not allocate memory\n"); + return EFI_OUT_OF_RESOURCES; } + rc = uefi_call_wrapper(fh->GetInfo, 4, fh, &EFI_FILE_INFO_GUID, &bs, buffer); /* This checks *either* the error from the first GetInfo, if it isn't - * the EFI_BUFFER_TOO_SMALL we're expecting, or the second GetInfo call - * in *any* case. */ + * the EFI_BUFFER_TOO_SMALL we're expecting, or the second GetInfo + * call in *any* case. */ if (EFI_ERROR(rc)) { Print(L"Could not get file info: %d\n", rc); if (buffer) @@ -141,6 +143,8 @@ read_file(EFI_FILE_HANDLE fh, CHAR16 *fullpath, CHAR16 **buffer, UINT64 *bs) CHAR16 *b = NULL; rc = get_file_size(fh2, &len); if (EFI_ERROR(rc)) { + Print(L"Could not get file size for \"%s\": %r\n", + fullpath, rc); uefi_call_wrapper(fh2->Close, 1, fh2); return rc; } @@ -682,15 +686,21 @@ find_boot_csv(EFI_FILE_HANDLE fh, CHAR16 *dirname) /* The API here is "Call it once with bs=0, it fills in bs, * then allocate a buffer and ask again to get it filled. */ rc = uefi_call_wrapper(fh->GetInfo, 4, fh, &EFI_FILE_INFO_GUID, &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; - } - rc = uefi_call_wrapper(fh->GetInfo, 4, fh, &EFI_FILE_INFO_GUID, - &bs, buffer); + if (EFI_ERROR(rc) && rc != EFI_BUFFER_TOO_SMALL) { + Print(L"Could not get directory info for \\EFI\\%s\\: %r\n", + dirname, rc); + return rc; } + if (bs == 0) + return EFI_SUCCESS; + + buffer = AllocateZeroPool(bs); + if (!buffer) { + Print(L"Could not allocate memory\n"); + return EFI_OUT_OF_RESOURCES; + } + + rc = uefi_call_wrapper(fh->GetInfo, 4, fh, &EFI_FILE_INFO_GUID, &bs, buffer); /* This checks *either* the error from the first GetInfo, if it isn't * the EFI_BUFFER_TOO_SMALL we're expecting, or the second GetInfo call * in *any* case. */ -- cgit v1.2.3