Fix boot failures due to variable size constraintsupstream/shim-15.3-rc4shim-15.3-rc4

There are multiple issues in the MOK variable mirroring code due
to volatile variable size constraints, which all result in boot
failures:

- If a signature is encountered which doesn't fit in to a single
variable, the code enters an infinite loop because the cursor
isn't advanced in mirror_mok_db() after the call to
mirror_one_esl().

- If an ESL is encountered which doesn't fit in to a single
variable, it looks like the intention is for the ESL to be split
across multiple variables. However, mirror_one_esl() will write
the maximum variable size on each call, regardless of how much
data is remaining for the current ESL. If the size of a ESL isn't
a multiple of the maximum variable size, the final call to
mirror_one_esl() will append data from the start of the next
ESL and the cursor in mirror_mok_db() will be advanced to an
arbitrary location in the next ESL. This either results in garbage
being mirrored (if you're lucky), or in my case - another infinite
loop as it appears to encounter a signature that doesn't fit in to
a single variable.

- If no signatures can be mirrored when mirror_mok_db() is called
with only_first=TRUE, it tries to create a variable with a single
SHA256 signature in it. But mirror_mok_db() returns an error
(EFI_INVALID_PARAMETER) regardless of whether this succeeds.

2 files changed, 32 insertions, 30 deletions

diff --git a/lib/variables.c b/lib/variables.c
index 53a5d14a..f606e248 100644
--- a/lib/variables.c
+++ b/lib/variables.c
@@ -22,7 +22,7 @@ fill_esl(const EFI_SIGNATURE_DATA *first_sig, const size_t howmany,
 	size_t needed = 0;
 	size_t data_len = howmany * sig_size;
 
-	dprint(L"fill_esl: data=%p, data_len=%lu", first_sig, data_len);
+	dprint(L"fill_esl: first_sig=0x%llx, data_len=%lu\n", first_sig, data_len);
 
 	if ((out && !first_sig) || !howmany || !type || !sig_size || !outlen)
 		return EFI_INVALID_PARAMETER;
diff --git a/mok.c b/mok.c
index 345c322c..5ad9072b 100644
--- a/mok.c
+++ b/mok.c
@@ -300,18 +300,12 @@ get_max_var_sz(UINT32 attrs, SIZE_T *max_var_szp)
 static EFI_STATUS
 mirror_one_esl(CHAR16 *name, EFI_GUID *guid, UINT32 attrs,
 	       EFI_SIGNATURE_LIST *esl, EFI_SIGNATURE_DATA *esd,
-	       UINTN *newsz, SIZE_T maxsz)
+	       SIZE_T howmany)
 {
 	EFI_STATUS efi_status;
-	SIZE_T howmany, varsz = 0;
+	SIZE_T varsz = 0;
 	UINT8 *var;
 
-	howmany = MIN((maxsz - sizeof(*esl)) / esl->SignatureSize,
-		      (esl->SignatureListSize - sizeof(*esl)) / esl->SignatureSize);
-	if (howmany < 1) {
-		return EFI_BUFFER_TOO_SMALL;
-	}
-
 	/*
 	 * We always assume esl->SignatureHeaderSize is 0 (and so far,
 	 * that's true as per UEFI 2.8)
@@ -346,8 +340,6 @@ mirror_one_esl(CHAR16 *name, EFI_GUID *guid, UINT32 attrs,
 		return efi_status;
 	}
 
-	*newsz = howmany * esl->SignatureSize;
-
 	return efi_status;
 }
 
@@ -459,12 +451,25 @@ mirror_mok_db(CHAR16 *name, CHAR8 *name8, EFI_GUID *guid, UINT32 attrs,
 			return efi_status;
 		}
 
+		/* The name counts towards the size of the variable */
+		max_var_sz -= (StrLen(namen) + 1) * 2;
+		dprint(L"max_var_sz - name: %lx\n", max_var_sz);
+
 		SIZE_T howmany;
-		UINTN adj = 0;
 		howmany = MIN((max_var_sz - sizeof(*esl)) / esl->SignatureSize,
-			      (esl->SignatureListSize - sizeof(*esl)) / esl->SignatureSize);
-		if (!only_first && i == 0 && howmany >= 1) {
-			adj = howmany * esl->SignatureSize;
+			      (esl_end_pos - pos) / esl->SignatureSize);
+		if (howmany == 0) {
+			/* No signatures from this ESL can be mirrored in to a
+			 * single variable, so skip it.
+			 */
+			dprint(L"skipping esl, pos:0x%llx->0x%llx\n", pos, esl_end_pos);
+			pos = esl_end_pos;
+			continue;
+		}
+
+		UINTN adj = howmany * esl->SignatureSize;
+
+		if (!only_first && i == 0) {
 			dprint(L"pos:0x%llx->0x%llx\n", pos, pos + adj);
 			pos += adj;
 			i++;
@@ -473,25 +478,25 @@ mirror_mok_db(CHAR16 *name, CHAR8 *name8, EFI_GUID *guid, UINT32 attrs,
 		}
 
 		efi_status = mirror_one_esl(namen, guid, attrs,
-					    esl, esd, &adj, max_var_sz);
+					    esl, esd, howmany);
 		dprint(L"esd:0x%llx adj:0x%llx\n", esd, adj);
-		if (EFI_ERROR(efi_status) && efi_status != EFI_BUFFER_TOO_SMALL) {
+		if (EFI_ERROR(efi_status)) {
 			LogError(L"Could not mirror mok variable \"%s\": %r\n",
 				 namen, efi_status);
 			break;
 		}
 
-		if (!EFI_ERROR(efi_status)) {
-			did_one = TRUE;
-			if (only_first)
-				break;
-			dprint(L"pos:0x%llx->0x%llx\n", pos, pos + adj);
-			pos += adj;
-			i++;
-		}
+		dprint(L"pos:0x%llx->0x%llx\n", pos, pos + adj);
+		pos += adj;
+		did_one = TRUE;
+		if (only_first)
+			break;
+		i++;
 	}
 
-	if (only_first && !did_one) {
+	if (EFI_ERROR(efi_status)) {
+		perror(L"Failed to set %s: %r\n", name, efi_status);
+	} else if (only_first && !did_one) {
 		/*
 		 * In this case we're going to try to create a
 		 * dummy variable so that there's one there.  It
@@ -513,15 +518,12 @@ mirror_mok_db(CHAR16 *name, CHAR8 *name8, EFI_GUID *guid, UINT32 attrs,
 		 * doesn't.
 		 */
 		if (!EFI_ERROR(efi_status) && var && varsz) {
-			SetVariable(name, guid,
+			efi_status = SetVariable(name, guid,
 				    EFI_VARIABLE_BOOTSERVICE_ACCESS
 				    | EFI_VARIABLE_RUNTIME_ACCESS,
 				    varsz, var);
 			FreePool(var);
 		}
-		efi_status = EFI_INVALID_PARAMETER;
-	} else if (EFI_ERROR(efi_status)) {
-		perror(L"Failed to set %s: %r\n", name, efi_status);
 	}
 	return efi_status;
 }