From 4816cd7533f7a9921bd945c12a1fcec48d95c2ed Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 19 Oct 2017 13:22:23 -0400 Subject: lib: find_in_variable_esl(): Fix a tiny nitpick clang-analyze has. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clang-analyze believes the following: 311 EFI_STATUS 312 variable_enroll_hash(CHAR16 *var, EFI_GUID owner, 313 UINT8 hash[SHA256_DIGEST_SIZE]) 314 { 315 EFI_STATUS efi_status; 316 317 efi_status = find_in_variable_esl(var, owner, hash, SHA256_DIGEST_SIZE); > Calling 'find_in_variable_esl' → 260 EFI_STATUS 261 find_in_variable_esl(CHAR16* var, EFI_GUID owner, UINT8 *key, UINTN keylen) 262 { 263 UINTN DataSize; 264 UINT8 *Data; > ← 'Data' declared without an initial value → 265 EFI_STATUS efi_status; 266 267 efi_status = get_variable(var, &Data, &DataSize, owner); > ← Calling 'get_variable' → 237 EFI_STATUS 238 get_variable(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner) 239 { 240 return get_variable_attr(var, data, len, owner, NULL); > ← Calling 'get_variable_attr' → 213 EFI_STATUS 214 get_variable_attr(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner, 215 UINT32 *attributes) 216 { 217 EFI_STATUS efi_status; 218 219 *len = 0; 220 221 efi_status = GetVariable(var, &owner, NULL, len, NULL); > ← Calling 'GetVariable' → > ← Returning from 'GetVariable' → 222 if (efi_status != EFI_BUFFER_TOO_SMALL) > ← Assuming the condition is true → > ← Taking true branch → 223 return efi_status; 224 225 *data = AllocateZeroPool(*len); 226 if (!*data) 227 return EFI_OUT_OF_RESOURCES; 228 229 efi_status = GetVariable(var, &owner, attributes, len, *data); 230 if (EFI_ERROR(efi_status)) { 231 FreePool(*data); 232 *data = NULL; 233 } 234 return efi_status; 235 } And it can't figure out that the first GetVariable() call will, in fact, always return EFI_BUFFER_TOO_SMALL, and that AllocateZeroPool() will then *correctly* clobber the two variables we never assigned the value from. It also then believes that efi_status might have been returned /without/ being an error, and thinks that means we'll use the uninitialized pointer. This won't happen, but hey, let's make the code better express to the checker what is intended. Signed-off-by: Peter Jones --- lib/variables.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'lib/variables.c') diff --git a/lib/variables.c b/lib/variables.c index 044ddae5..7c28eaa5 100644 --- a/lib/variables.c +++ b/lib/variables.c @@ -211,13 +211,16 @@ get_variable_attr(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner, efi_status = uefi_call_wrapper(RT->GetVariable, 5, var, &owner, NULL, len, NULL); - if (efi_status != EFI_BUFFER_TOO_SMALL) + if (efi_status != EFI_BUFFER_TOO_SMALL) { + if (!EFI_ERROR(efi_status)) /* this should never happen */ + return EFI_PROTOCOL_ERROR; return efi_status; + } *data = AllocateZeroPool(*len); if (!*data) return EFI_OUT_OF_RESOURCES; - + efi_status = uefi_call_wrapper(RT->GetVariable, 5, var, &owner, attributes, len, *data); @@ -254,8 +257,8 @@ find_in_esl(UINT8 *Data, UINTN DataSize, UINT8 *key, UINTN keylen) EFI_STATUS find_in_variable_esl(CHAR16* var, EFI_GUID owner, UINT8 *key, UINTN keylen) { - UINTN DataSize; - UINT8 *Data; + UINTN DataSize = 0; + UINT8 *Data = NULL; EFI_STATUS status; status = get_variable(var, &Data, &DataSize, owner); -- cgit v1.2.3