From 9c423e09cd9a0196888253b16fe7022deadc20fc Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 25 Feb 2025 14:33:50 -0500 Subject: 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 --- errlog.c | 64 ++++++++++++++++++++++++++++++++++++++++++++--------------- globals.c | 3 +++ include/mok.h | 3 +++ mok.c | 2 ++ 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 @@ -172,17 +172,42 @@ format_debug_log(UINT8 *dest, size_t dest_sz) return debug_log_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; } } -- cgit v1.2.3