Skip to content

Commit

Permalink
Fix a plausible NULL dereference.
Browse files Browse the repository at this point in the history
scan-build kindly pointed out:

| shim.c:1568:10: warning: Array access (from variable 'start') results in a null pointer dereference [core.NullDereference]
|                 while (start[loader_len++] != L'\0');
|                        ^~~~~~~~~~~~~~~~~~~
| 1 warning generated.

It thinks that because of a bad assumption it's making because of the
test immediately before it, which isn't currently necessary /at all/.
In fact, neither is this loop; it appears to be vestigial and the goal
was done in the loop above it.

This patch just solves for how much space is left arithmetically
instead.

Signed-off-by: Peter Jones <pjones@redhat.com>
  • Loading branch information
vathpela authored and martinezjavier committed Mar 12, 2021
1 parent 2a2ae40 commit 9e8dde5
Showing 1 changed file with 12 additions and 13 deletions.
25 changes: 12 additions & 13 deletions shim.c
Original file line number Diff line number Diff line change
Expand Up @@ -1388,7 +1388,7 @@ EFI_STATUS set_second_stage (EFI_HANDLE image_handle)
EFI_STATUS efi_status;
EFI_LOADED_IMAGE *li = NULL;
CHAR16 *start = NULL;
int remaining_size = 0;
UINTN remaining_size = 0;
CHAR16 *loader_str = NULL;
UINTN loader_len = 0;
unsigned int i;
Expand Down Expand Up @@ -1510,9 +1510,14 @@ EFI_STATUS set_second_stage (EFI_HANDLE image_handle)
li->LoadOptionsSize);

/*
* In the case where strings == 1 check to see if L' ' is being used as
* a delimiter, if so replace it with NULLs since the code already
* handles that case.
* In some cases we get strings == 1 because BDS is using L' ' as the
* delimeter:
* 0000:74 00 65 00 73 00 74 00 2E 00 65 00 66 00 69 00 t.e.s.t...e.f.i.
* 0016:20 00 6F 00 6E 00 65 00 20 00 74 00 77 00 6F 00 ..o.n.e...t.w.o.
* 0032:20 00 74 00 68 00 72 00 65 00 65 00 00 00 ..t.h.r.e.e...
*
* If so replace it with NULs since the code already handles that
* case.
*/
if (strings == 1) {
UINT16 *cur = li->LoadOptions;
Expand Down Expand Up @@ -1561,18 +1566,12 @@ EFI_STATUS set_second_stage (EFI_HANDLE image_handle)
break;
}
}
/* if we didn't find at least one NULL, something is wrong */
if (start == li->LoadOptions)
return EFI_SUCCESS;

while (start[loader_len++] != L'\0');
loader_len *= 2;

remaining_size -= loader_len;
remaining_size -= i * 2 + 2;
} else if (strings == 1 && is_our_path(li, start)) {
/*
* And then I found a version of BDS that gives us our own path
* in LoadOptions:
* And then I found a version of BDS that gives us our own path
* in LoadOptions:
77162C58 5c 00 45 00 46 00 49 00 |\.E.F.I.|
77162C60 5c 00 42 00 4f 00 4f 00 54 00 5c 00 42 00 4f 00 |\.B.O.O.T.\.B.O.|
Expand Down

0 comments on commit 9e8dde5

Please sign in to comment.