Some save_logs() improvements.

In d972515e608e ("Save the debug and error logs in mok-variables") had a
few deficiencies: 1) the size of the result table isn't correctly
computed when either errlog or dbglog is 0 sized (much more likely for
the former), 2) when we save the error log we leak the allocation for
the previous mok variables, and 3) original mok variables were allocated
with AllocatePages(), but the new ones were allocated with
AllocateZeroPool().  The former guarantees page alignment, which we want
here.

This fixes all three of these.

Signed-off-by: Peter Jones <pjones@redhat.com>

4 files changed, 56 insertions, 16 deletions

diff --git a/errlog.c b/errlog.c
index fc89ae0a..b43a4bc2 100644
--- a/errlog.c
+++ b/errlog.c
@@ -173,16 +173,41 @@ format_debug_log(UINT8 *dest, size_t dest_sz)
 }
 
 void
+replace_config_table(EFI_CONFIGURATION_TABLE *CT, EFI_PHYSICAL_ADDRESS new_table, UINTN new_table_pages)
+{
+	EFI_GUID bogus_guid = { 0x29f2f0db, 0xd025, 0x4aa6, { 0x99, 0x58, 0xa0, 0x21, 0x8b, 0x1d, 0xec, 0x0e }};
+	EFI_STATUS efi_status;
+
+	if (CT) {
+		CopyMem(&CT->VendorGuid, &bogus_guid, sizeof(bogus_guid));
+		if (CT->VendorTable &&
+		    CT->VendorTable == (void *)(uintptr_t)mok_config_table) {
+			BS->FreePages(mok_config_table, mok_config_table_pages);
+			CT->VendorTable = NULL;
+		}
+	}
+
+	efi_status = BS->InstallConfigurationTable(&MOK_VARIABLE_STORE,
+						   (void *)(uintptr_t)new_table);
+	if (EFI_ERROR(efi_status)) {
+		console_print(L"Could not re-install MoK configuration table: %r\n", efi_status);
+	} else {
+		mok_config_table = new_table;
+		mok_config_table_pages = new_table_pages;
+	}
+}
+
+void
 save_logs(void)
 {
 	struct mok_variable_config_entry *cfg_table = NULL;
 	struct mok_variable_config_entry *new_table = NULL;
 	struct mok_variable_config_entry *entry = NULL;
+	EFI_PHYSICAL_ADDRESS physaddr = 0;
+	UINTN new_table_pages = 0;
 	size_t new_table_sz;
 	UINTN pos = 0;
 	EFI_STATUS efi_status;
-	EFI_CONFIGURATION_TABLE *CT;
-	EFI_GUID bogus_guid = { 0x29f2f0db, 0xd025, 0x4aa6, { 0x99, 0x58, 0xa0, 0x21, 0x8b, 0x1d, 0xec, 0x0e }};
 	size_t errlog_sz, dbglog_sz;
 
 	errlog_sz = format_error_log(NULL, 0);
@@ -194,6 +219,7 @@ save_logs(void)
 	}
 
 	for (UINTN i = 0; i < ST->NumberOfTableEntries; i++) {
+		EFI_CONFIGURATION_TABLE *CT;
 		CT = &ST->ConfigurationTable[i];
 
 		if (CompareGuid(&MOK_VARIABLE_STORE, &CT->VendorGuid) == 0) {
@@ -208,18 +234,28 @@ save_logs(void)
 		size_t entry_sz;
 		entry = (struct mok_variable_config_entry *)((uintptr_t)cfg_table + pos);
 
-		entry_sz = sizeof(*entry);
-		entry_sz += entry->data_size;
-
-		if (entry->name[0] != 0)
+		if (entry->name[0] != 0) {
+			entry_sz = sizeof(*entry);
+			entry_sz += entry->data_size;
 			pos += entry_sz;
+		}
 	}
 
-	new_table_sz = pos + 3 * sizeof(*entry) + errlog_sz + dbglog_sz;
-	new_table = AllocateZeroPool(new_table_sz);
+	new_table_sz = pos +
+		(errlog_sz ? sizeof(*entry) + errlog_sz : 0) +
+		(dbglog_sz ? sizeof(*entry) + dbglog_sz : 0) +
+		sizeof(*entry);
+	new_table = NULL;
+	new_table_pages = ALIGN_UP(new_table_sz + 4*EFI_PAGE_SIZE, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+	efi_status = BS->AllocatePages(AllocateAnyPages, EfiRuntimeServicesData, new_table_pages, &physaddr);
+	if (EFI_ERROR(efi_status)) {
+		perror(L"Couldn't allocate %llu pages\n", new_table_pages);
+		return;
+	}
+	new_table = (void *)(uintptr_t)physaddr;
 	if (!new_table)
 		return;
-
+	ZeroMem(new_table, new_table_pages * EFI_PAGE_SIZE);
 	CopyMem(new_table, cfg_table, pos);
 
 	entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos);
@@ -237,15 +273,11 @@ save_logs(void)
 		format_debug_log(&entry->data[0], dbglog_sz);
 
 		pos += sizeof(*entry) + dbglog_sz;
-		// entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos);
-	}
 
-	if (CT) {
-		CopyMem(&CT->VendorGuid, &bogus_guid, sizeof(bogus_guid));
+		entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos);
 	}
-	efi_status = BS->InstallConfigurationTable(&MOK_VARIABLE_STORE, new_table);
-	if (EFI_ERROR(efi_status))
-		console_print(L"Could not re-install MoK configuration table: %r\n", efi_status);
+
+	replace_config_table((EFI_CONFIGURATION_TABLE *)cfg_table, physaddr, new_table_pages);
 }
 
 // vim:fenc=utf-8:tw=75
diff --git a/globals.c b/globals.c
index de64c44c..11197128 100644
--- a/globals.c
+++ b/globals.c
@@ -35,4 +35,7 @@ UINT8 mok_policy = 0;
 
 UINT32 verbose = 0;
 
+EFI_PHYSICAL_ADDRESS mok_config_table = 0;
+UINTN mok_config_table_pages = 0;
+
 // vim:fenc=utf-8:tw=75:noet
diff --git a/include/mok.h b/include/mok.h
index 89edf9de..f4468ab0 100644
--- a/include/mok.h
+++ b/include/mok.h
@@ -127,6 +127,9 @@ struct mok_variable_config_entry {
 	UINT8 data[];
 };
 
+extern EFI_PHYSICAL_ADDRESS mok_config_table;
+extern UINTN mok_config_table_pages;
+
 /*
  * bit definitions for MokPolicy
  */
diff --git a/mok.c b/mok.c
index fb4c1489..8367fad5 100644
--- a/mok.c
+++ b/mok.c
@@ -1305,6 +1305,8 @@ EFI_STATUS import_mok_state(EFI_HANDLE image_handle)
 			config_table = NULL;
 		} else {
 			ZeroMem(config_table, npages << EFI_PAGE_SHIFT);
+			mok_config_table = (EFI_PHYSICAL_ADDRESS)(uintptr_t)config_table;
+			mok_config_table_pages = npages;
 		}
 	}