From: David Howells <dhowells@redhat.com>
To: Eric Biggers <ebiggers3@gmail.com>
Cc: dhowells@redhat.com, keyrings@vger.kernel.org,
linux-kernel@vger.kernel.org, Eric Biggers <ebiggers@google.com>
Subject: Re: [PATCH] KEYS: put keyring if install_session_keyring_to_cred() fails
Date: Mon, 03 Apr 2017 16:42:05 +0100 [thread overview]
Message-ID: <2347.1491234125@warthog.procyon.org.uk> (raw)
In-Reply-To: <20170401185416.10225-1-ebiggers3@gmail.com>
Eric Biggers <ebiggers3@gmail.com> wrote:
> +error3:
> + key_put(keyring);
> error2:
> mutex_unlock(&key_session_mutex);
I would prefer the put to be outside of the locked section. How about the
attached instead?
David
---
commit 5f9e6fc4fdbbf9ada9bd03640b1c3ca50a3e5415
Author: Eric Biggers <ebiggers@google.com>
Date: Sat Apr 1 11:54:16 2017 -0700
KEYS: put keyring if install_session_keyring_to_cred() fails
In join_session_keyring(), if install_session_keyring_to_cred() were to
fail, we would leak the keyring reference, just like in the bug fixed by
commit 23567fd052a9 ("KEYS: Fix keyring ref leak in
join_session_keyring()"). Fortunately this cannot happen currently, but
we really should be more careful. Do this by adding and using a new
error label at which the keyring reference is dropped.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 44451af828c0..8e98e5d1b5fb 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -793,21 +793,20 @@ long join_session_keyring(const char *name)
KEY_ALLOC_IN_QUOTA, NULL, NULL);
if (IS_ERR(keyring)) {
ret = PTR_ERR(keyring);
- goto error2;
+ goto error_unlock_no_keyring;
}
} else if (IS_ERR(keyring)) {
ret = PTR_ERR(keyring);
- goto error2;
+ goto error_unlock_no_keyring;
} else if (keyring == new->session_keyring) {
- key_put(keyring);
ret = 0;
- goto error2;
+ goto error_unlock;
}
/* we've got a keyring - now to install it */
ret = install_session_keyring_to_cred(new, keyring);
if (ret < 0)
- goto error2;
+ goto error_unlock;
commit_creds(new);
mutex_unlock(&key_session_mutex);
@@ -817,8 +816,11 @@ long join_session_keyring(const char *name)
okay:
return ret;
-error2:
+error_unlock_no_keyring:
+ keyring = NULL;
+error_unlock:
mutex_unlock(&key_session_mutex);
+ key_put(keyring);
error:
abort_creds(new);
return ret;
next prev parent reply other threads:[~2017-04-03 15:42 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-01 18:54 [PATCH] KEYS: put keyring if install_session_keyring_to_cred() fails Eric Biggers
2017-04-03 15:42 ` David Howells [this message]
2017-04-03 17:48 ` Eric Biggers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2347.1491234125@warthog.procyon.org.uk \
--to=dhowells@redhat.com \
--cc=ebiggers3@gmail.com \
--cc=ebiggers@google.com \
--cc=keyrings@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.