From: Jarkko Sakkinen <jarkko@kernel.org>
To: adrimg3196@gmail.com
Cc: linux-kernel@vger.kernel.org, keyrings@vger.kernel.org,
dhowells@redhat.com
Subject: Re: [PATCH v2] KEYS: Fix key_reject_and_link() race with keyring restriction
Date: Thu, 8 Oct 2026 23:14:11 +0300 [thread overview]
Message-ID: <asf5kyDXUQ3z6RMv@kernel.org> (raw)
In-Reply-To: <CACO1+RQ5KXAAbjpiQ=cs0wLZ=z+=JQbW_-pgG+BM0f23W112Tg@mail.gmail.com>
On Thu, Oct 08, 2026 at 11:18:02AM -0700, adrimg3196@gmail.com wrote:
> Jarkko, you're right — v1 arrived without the diff. Here's v2 with the
> full patch.
And it would not be your fault if it had gone to my PR as I have
ultimate responsibility for that part.
Anyhow, put side-notes under '---'. It's a good place for these
as it does not get pulled into the commit log.
For single-patch submission it is also great place to maintain
changelog.
>
> The restrict_link check in key_reject_and_link() is done before taking
> the keyring semaphore, racing with a concurrent keyring_restrict()
> that installs the restriction while holding the semaphore in write
> mode.
>
> This is the same pattern that commit dd3ea3fc ("KEYS: Fix add_key()
> race with keyring restriction") just fixed in __key_create_or_update()
> by moving the restrict_link snapshot to after the semaphore is taken.
>
> Move the check under __key_link_lock() so a restriction installed
> concurrently cannot be bypassed when linking a negative key.
>
Fixes: 5ac7eace2d00 ("KEYS: Add a facility to restrict new links into a keyring")o
> Signed-off-by: Adrian Martinez <adrimg3196@gmail.com>
> ---
Here.
> security/keys/key.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -588,10 +588,17 @@ int key_reject_and_link(struct key *key,
> ret = -EBUSY;
>
> if (keyring) {
> - if (keyring->restrict_link)
> - return -EPERM;
> -
> link_ret = __key_link_lock(keyring, &key->index_key);
> if (link_ret == 0) {
> + /*
> + * Check the restriction under the keyring semaphore.
> + * keyring_restrict() installs it while holding the
> + * semaphore, so testing it beforehand races with a
> + * concurrent restriction install (cf. the fix for
> + * __key_create_or_update()).
> + */
> + if (keyring->restrict_link) {
> + __key_link_end(keyring, &key->index_key, edit);
> + return -EPERM;
> + }
> link_ret = __key_link_begin(keyring, &key->index_key, &edit);
> if (link_ret < 0)
> __key_link_end(keyring, &key->index_key, edit);
Looks good to me, thank you.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Br, Jarkko
next prev parent reply other threads:[~2026-10-08 20:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-08 18:18 adrimg3196
2026-10-08 20:14 ` Jarkko Sakkinen [this message]
2026-10-08 20:58 ` Jarkko Sakkinen
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=asf5kyDXUQ3z6RMv@kernel.org \
--to=jarkko@kernel.org \
--cc=adrimg3196@gmail.com \
--cc=dhowells@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®