Re: [PATCH 2/2] efi: show error messages only when loading certificates is failed
From: Ard Biesheuvel <hidden>
Date: 2019-12-13 09:10:21
Also in:
linux-efi, lkml
On Fri, 13 Dec 2019 at 10:07, Lee, Chun-Yi [off-list ref] wrote:
When loading certificates list from EFI variables, the error message and efi status code always be emitted to dmesg. It looks ugly: [ 2.335031] Couldn't get size: 0x800000000000000e [ 2.335032] Couldn't get UEFI MokListRT [ 2.339985] Couldn't get size: 0x800000000000000e [ 2.339987] Couldn't get UEFI dbx list This cosmetic patch moved the messages to the error handling code path. And, it also shows the corresponding status string of status code.
So what output do we get after applying this patch when those variables don't exist?
quoted hunk ↗ jump to hunk
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> --- security/integrity/platform_certs/load_uefi.c | 40 ++++++++++++++------------- 1 file changed, 21 insertions(+), 19 deletions(-)diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c index 81b19c52832b..b6c60fb3fb6c 100644 --- a/security/integrity/platform_certs/load_uefi.c +++ b/security/integrity/platform_certs/load_uefi.c@@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/kernel.h> #include <linux/sched.h>@@ -39,7 +40,7 @@ static __init bool uefi_check_ignore_db(void) * Get a certificate list blob from the named EFI variable. */ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid, - unsigned long *size) + unsigned long *size, const char *source) { efi_status_t status; unsigned long lsize = 4;@@ -48,23 +49,30 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid, status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb); if (status != EFI_BUFFER_TOO_SMALL) { - pr_err("Couldn't get size: 0x%lx\n", status); - return NULL; + if (status == EFI_NOT_FOUND) { + pr_debug("%s list was not found\n", source); + return NULL; + } + goto err; } db = kmalloc(lsize, GFP_KERNEL); - if (!db) - return NULL; + if (!db) { + status = EFI_OUT_OF_RESOURCES; + goto err; + } status = efi.get_variable(name, guid, NULL, &lsize, db); if (status != EFI_SUCCESS) { kfree(db); - pr_err("Error reading db var: 0x%lx\n", status); - return NULL; + goto err; } *size = lsize; return db; +err: + pr_err("Couldn't get %s list: %s\n", source, efi_status_to_str(status)); + return NULL; } /*@@ -153,10 +161,8 @@ static int __init load_uefi_certs(void) * an error if we can't get them. */ if (!uefi_check_ignore_db()) { - db = get_cert_list(L"db", &secure_var, &dbsize); - if (!db) { - pr_err("MODSIGN: Couldn't get UEFI db list\n"); - } else { + db = get_cert_list(L"db", &secure_var, &dbsize, "UEFI:db"); + if (db) { rc = parse_efi_signature_list("UEFI:db", db, dbsize, get_handler_for_db); if (rc)@@ -166,10 +172,8 @@ static int __init load_uefi_certs(void) } } - mok = get_cert_list(L"MokListRT", &mok_var, &moksize); - if (!mok) { - pr_info("Couldn't get UEFI MokListRT\n"); - } else { + mok = get_cert_list(L"MokListRT", &mok_var, &moksize, "UEFI:MokListRT"); + if (mok) { rc = parse_efi_signature_list("UEFI:MokListRT", mok, moksize, get_handler_for_db); if (rc)@@ -177,10 +181,8 @@ static int __init load_uefi_certs(void) kfree(mok); } - dbx = get_cert_list(L"dbx", &secure_var, &dbxsize); - if (!dbx) { - pr_info("Couldn't get UEFI dbx list\n"); - } else { + dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, "UEFI:dbx"); + if (dbx) { rc = parse_efi_signature_list("UEFI:dbx", dbx, dbxsize, get_handler_for_dbx); --2.16.4