Fix a plausible NULL dereference.

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>

1 files changed, 12 insertions, 13 deletions

diff --git a/shim.c b/shim.c
index 9bc3d602..411833ee 100644
--- a/shim.c
+++ b/shim.c
@@ -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;
@@ -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;
@@ -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.|