3.16.52-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Eric Biggers ebiggers@google.com
commit 44d8143340a99b167c74365e844516b73523c087 upstream.
In request_key_auth_new(), if key_alloc() or key_instantiate_and_link() were to fail, we would leak a reference to the 'struct cred'. Currently this can only happen if key_alloc() fails to allocate memory. But it still should be fixed, as it is a more severe bug waiting to happen.
Fix it by cleaning things up to use a helper function which frees a 'struct request_key_auth' correctly.
Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials") Signed-off-by: Eric Biggers ebiggers@google.com Signed-off-by: David Howells dhowells@redhat.com [bwh: Backported to 3.16: adjust context] Signed-off-by: Ben Hutchings ben@decadent.org.uk --- security/keys/request_key_auth.c | 68 ++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 37 deletions(-)
--- a/security/keys/request_key_auth.c +++ b/security/keys/request_key_auth.c @@ -107,6 +107,18 @@ static void request_key_auth_revoke(stru } }
+static void free_request_key_auth(struct request_key_auth *rka) +{ + if (!rka) + return; + key_put(rka->target_key); + key_put(rka->dest_keyring); + if (rka->cred) + put_cred(rka->cred); + kfree(rka->callout_info); + kfree(rka); +} + /* * Destroy an instantiation authorisation token key. */ @@ -116,15 +128,7 @@ static void request_key_auth_destroy(str
kenter("{%d}", key->serial);
- if (rka->cred) { - put_cred(rka->cred); - rka->cred = NULL; - } - - key_put(rka->target_key); - key_put(rka->dest_keyring); - kfree(rka->callout_info); - kfree(rka); + free_request_key_auth(rka); }
/* @@ -138,22 +142,17 @@ struct key *request_key_auth_new(struct const struct cred *cred = current->cred; struct key *authkey = NULL; char desc[20]; - int ret; + int ret = -ENOMEM;
kenter("%d,", target->serial);
/* allocate a auth record */ - rka = kmalloc(sizeof(*rka), GFP_KERNEL); - if (!rka) { - kleave(" = -ENOMEM"); - return ERR_PTR(-ENOMEM); - } + rka = kzalloc(sizeof(*rka), GFP_KERNEL); + if (!rka) + goto error; rka->callout_info = kmalloc(callout_len, GFP_KERNEL); - if (!rka->callout_info) { - kleave(" = -ENOMEM"); - kfree(rka); - return ERR_PTR(-ENOMEM); - } + if (!rka->callout_info) + goto error_free_rka;
/* see if the calling process is already servicing the key request of * another process */ @@ -163,8 +162,12 @@ struct key *request_key_auth_new(struct
/* if the auth key has been revoked, then the key we're * servicing is already instantiated */ - if (test_bit(KEY_FLAG_REVOKED, &cred->request_key_auth->flags)) - goto auth_key_revoked; + if (test_bit(KEY_FLAG_REVOKED, + &cred->request_key_auth->flags)) { + up_read(&cred->request_key_auth->sem); + ret = -EKEYREVOKED; + goto error_free_rka; + }
irka = cred->request_key_auth->payload.data; rka->cred = get_cred(irka->cred); @@ -192,32 +195,23 @@ struct key *request_key_auth_new(struct KEY_USR_VIEW, KEY_ALLOC_NOT_IN_QUOTA); if (IS_ERR(authkey)) { ret = PTR_ERR(authkey); - goto error_alloc; + goto error_free_rka; }
/* construct the auth key */ ret = key_instantiate_and_link(authkey, rka, 0, NULL, NULL); if (ret < 0) - goto error_inst; + goto error_put_authkey;
kleave(" = {%d,%d}", authkey->serial, atomic_read(&authkey->usage)); return authkey;
-auth_key_revoked: - up_read(&cred->request_key_auth->sem); - kfree(rka->callout_info); - kfree(rka); - kleave("= -EKEYREVOKED"); - return ERR_PTR(-EKEYREVOKED); - -error_inst: +error_put_authkey: key_revoke(authkey); key_put(authkey); -error_alloc: - key_put(rka->target_key); - key_put(rka->dest_keyring); - kfree(rka->callout_info); - kfree(rka); +error_free_rka: + free_request_key_auth(rka); +error: kleave("= %d", ret); return ERR_PTR(ret); }