[PATCH v2] KEYS: Fix key_reject_and_link() race with keyring restriction

[PATCH v2] KEYS: Fix key_reject_and_link() race with keyring restriction

[adrimg3196]

Jarkko, you're right — v1 arrived without the diff. Here's v2 with the
full patch.

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.

Signed-off-by: Adrian Martinez <adrimg3196@gmail.com>
---
 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);

Re: [PATCH v2] KEYS: Fix key_reject_and_link() race with keyring restriction

[Jarkko Sakkinen]

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

Re: [PATCH v2] KEYS: Fix key_reject_and_link() race with keyring restriction

[Jarkko Sakkinen]

On Thu, Oct 08, 2026 at 11:14:16PM +0300, Jarkko Sakkinen wrote:
> 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>

So send v3 that addresses:

(141) $ b4 shazam https://lore.kernel.org/keyrings/asf5kyDXUQ3z6RMv@kernel.org/T/#t
Grabbing thread from lore.kernel.org/all/asf5kyDXUQ3z6RMv@kernel.org/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 3 messages in the thread
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH v2] KEYS: Fix key_reject_and_link() race with keyring restriction
    + Fixes: 5ac7eace2d00 ("KEYS: Add a facility to restrict new links into a keyring")o (✓ DKIM/kernel.org)
    + Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> (✓ DKIM/kernel.org)
  ---
  ✓ Signed: DKIM/gmail.com
---
Total patches: 1
---
Applying: KEYS: Fix key_reject_and_link() race with keyring restriction
Patch failed at 0001 KEYS: Fix key_reject_and_link() race with keyring restriction
error: corrupt patch at line 31
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"

You can retain my reviewed-by and please add fixes tag while at it.

Br, Jarkko