Messages in this thread | | | From | Andy Shevchenko <> | Date | Wed, 2 Sep 2020 10:55:21 +0300 | Subject | Re: [PATCH 2/3] integrity: Move import of MokListRT certs to a separate routine |
| |
On Wed, Aug 26, 2020 at 6:45 AM Lenny Szubowicz <lszubowi@redhat.com> wrote: > > Move the loading of certs from the UEFI MokListRT into a separate > routine to facilitate additional MokList functionality. > > There is no visible functional change as a result of this patch. > Although the UEFI dbx certs are now loaded before the MokList certs, > they are loaded onto different key rings. So the order of the keys > on their respective key rings is the same.
...
> /* > + * load_moklist_certs() - Load MokList certs > + * > + * Returns: Summary error status > + * > + * Load the certs contained in the UEFI MokListRT database into the > + * platform trusted keyring. > + */
Hmm... Is it intentionally kept out of kernel doc format?
> +static int __init load_moklist_certs(void) > +{ > + efi_guid_t mok_var = EFI_SHIM_LOCK_GUID; > + void *mok = NULL; > + unsigned long moksize = 0; > + efi_status_t status; > + int rc = 0;
Redundant assignment (see below).
> + /* Get MokListRT. It might not exist, so it isn't an error > + * if we can't get it. > + */ > + mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
> + if (!mok) {
Why not positive conditional? Sometimes ! is hard to notice.
> + if (status == EFI_NOT_FOUND) > + pr_debug("MokListRT variable wasn't found\n"); > + else > + pr_info("Couldn't get UEFI MokListRT\n"); > + } else { > + rc = parse_efi_signature_list("UEFI:MokListRT", > + mok, moksize, get_handler_for_db); > + if (rc) > + pr_err("Couldn't parse MokListRT signatures: %d\n", rc); > + kfree(mok);
kfree(...) if (rc) ... return rc;
And with positive conditional there will be no need to have redundant 'else' followed by additional level of indentation.
> + }
> + return rc;
return 0;
> +}
P.S. Yes, I see that the above was in the original code, so, consider my comments as suggestions to improve the code.
-- With Best Regards, Andy Shevchenko
| |