ref leak in request_asymmetric_key ?

Commit Message

Billie Alsup (balsup) Jan. 26, 2024, 2:36 a.m. UTC

In security/integrity/digsig_asymmetric.c function request_asymmetric_key, should we not do a key_ref_put() when the key is found in the ima_blacklist_keyring? I'm working with 6.6.9 kernel but have verified it is the same logic in linux-stable master branch (top of tree is ecb1b8288dc7ccbdcb3b9df005fa1c0e0c0388a7)
 
I'm new to this area of code however, so I may misunderstand it.  This is what I'm thinking is needed.

Comments

Billie Alsup (balsup) Jan. 26, 2024, 3:03 a.m. UTC | #1

>In security/integrity/digsig_asymmetric.c function request_asymmetric_key, should we 
>not do a key_ref_put() when the key is found in the ima_blacklist_keyring? I'm working 
>with 6.6.9 kernel but have verified it is the same logic in linux-stable master branch 
>(top of tree is ecb1b8288dc7ccbdcb3b9df005fa1c0e0c0388a7)

I guess it does not matter, as there doesn't appear to be a function to insert anything 
into the ima_blacklist_keyring.  Seems like we should simply use the system blacklist_keyring,
or perhaps the add_key_to_revocation_list function should take a keyring argument.

Patch

diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index 895f4b9ce8c6..b969cd56e9ef 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -35,6 +35,7 @@  static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
                kref = keyring_search(make_key_ref(key, 1),
                                      &key_type_asymmetric, name, true);
                if (!IS_ERR(kref)) {
+                       key_ref_put(kref);
                        pr_err("Key '%s' is in ima_blacklist_keyring\n", name);
                        return ERR_PTR(-EKEYREJECTED);
                }