mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

  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®