Messages in this thread | | | Subject | Re: [PATCH] efi: Only print errors about failing to get certs if EFI vars are found | From | Javier Martinez Canillas <> | Date | Tue, 19 Nov 2019 12:38:36 +0100 |
| |
Hello Hans,
Thanks a lot for your feedback.
On 11/19/19 11:59 AM, Hans de Goede wrote: > Hi, > > On 19-11-2019 10:18, Javier Martinez Canillas wrote: >> If CONFIG_LOAD_UEFI_KEYS is enabled, the kernel attempts to load the certs >> from the db, dbx and MokListRT EFI variables into the appropriate keyrings. >> >> But it just assumes that the variables will be present and prints an error >> if the certs can't be loaded, even when is possible that the variables may >> not exist. For example the MokListRT variable will only be present if shim >> is used. >> >> So only print an error message about failing to get the certs list from an >> EFI variable if this is found. Otherwise these printed errors just pollute >> the kernel ring buffer with confusing messages like the following: >> >> [ 5.427251] Couldn't get size: 0x800000000000000e >> [ 5.427261] MODSIGN: Couldn't get UEFI db list >> [ 5.428012] Couldn't get size: 0x800000000000000e >> [ 5.428023] Couldn't get UEFI MokListRT >> >> Reported-by: Hans de Goede <hdegoede@redhat.com> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > > Thanks for this, I just noticed a potential issue which I missed > when you send this to me for testing: >
[snip]
>> >> if (!efi.get_variable) >> @@ -153,8 +156,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) { >> + db = get_cert_list(L"db", &secure_var, &dbsize, &status); >> + if (!db && status != EFI_NOT_FOUND) { >> pr_err("MODSIGN: Couldn't get UEFI db list\n"); >> } else { >> rc = parse_efi_signature_list("UEFI:db",
You are correct, this is another instance of the same issue that you mentioned.
>> @@ -166,8 +169,8 @@ static int __init load_uefi_certs(void) >> } >> } >> >> - mok = get_cert_list(L"MokListRT", &mok_var, &moksize); >> - if (!mok) { >> + mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status); >> + if (!mok && status != EFI_NOT_FOUND) { >> pr_info("Couldn't get UEFI MokListRT\n"); >> } else { >> rc = parse_efi_signature_list("UEFI:MokListRT", > > This means that if status == EFI_NOT_FOUND we end up still > trying to parse the signature list, I guess that moksize == 0 > or some such is saving us here, but I believe that > this should really be: > > if (!mok) { > if (status != EFI_NOT_FOUND) > pr_info("Couldn't get UEFI MokListRT\n"); > } else { > rc = parse_efi_signature_list("UEFI:MokListRT", >
Agreed. I'll fix the issues and post a v2. Since we are adding these statements, I could also print debug messages for the case that status == EFI_NOT_FOUND.
Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat
| |