> On Jun 4, 2024, at 12:08 PM, Jarkko Sakkinen <jarkko@xxxxxxxxxx> wrote: > > On Fri May 31, 2024 at 3:39 AM EEST, Eric Snowberg wrote: >> Introduce a new function to allow a keyring to link to a key contained >> within one of the system keyrings (builtin, secondary, or platform). > > "Introduce system_key_link(), a new function..." > > I hate when the exact thing added is not immediately transparent from > the commit message ;-) Helps a lot when bisecting for instance. > >> Depending on how the kernel is built, if the machine keyring is >> available, it will be checked as well, since it is linked to the secondary >> keyring. If the asymmetric key id matches a key within one of these >> system keyrings, the matching key is linked into the passed in >> keyring. >> >> Signed-off-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx> >> --- >> certs/system_keyring.c | 31 +++++++++++++++++++++++++++++++ >> include/keys/system_keyring.h | 7 ++++++- >> 2 files changed, 37 insertions(+), 1 deletion(-) >> >> diff --git a/certs/system_keyring.c b/certs/system_keyring.c >> index 9de610bf1f4b..94e47b6b3333 100644 >> --- a/certs/system_keyring.c >> +++ b/certs/system_keyring.c >> @@ -426,3 +426,34 @@ void __init set_platform_trusted_keys(struct key *keyring) >> platform_trusted_keys = keyring; >> } >> #endif >> + >> +/** >> + * system_key_link - Link to a system key > > "system_key_link() - Link to a system key" > >> + * @keyring: The keyring to link into >> + * @id: The asymmetric key id to look for in the system keyring >> + */ > > Really could use some overview keyrings traversed just as a reminder. Sure, I will make the three changes above in the next round. >> +int system_key_link(struct key *keyring, struct asymmetric_key_id *id) >> +{ >> + struct key *system_keyring; >> + struct key *key; >> + >> +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING >> + system_keyring = secondary_trusted_keys; >> +#else >> + system_keyring = builtin_trusted_keys; >> +#endif > > Why not simply make secondary_trusted_keys in the first place be alias > to builtin_trusted_keys when it is not enabled? I'll change that in the next round and remove the #ifdef completely from within this function. I'll add a clean up patch first that removes this same pattern elsewhere in the file. I think I see how the goto can be removed now. And I'll also take care of the case where the kernel is built without the platform keyring enabled. Which I now see is a problem with this current version. Thanks.