mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] KEYS: Fix key_reject_and_link() race with keyring restriction
@ 2026-10-08 21:03 adrimg3196
  2026-10-10 19:36 ` Jarkko Sakkinen
  0 siblings, 1 reply; 3+ messages in thread
From: adrimg3196 @ 2026-10-08 21:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: keyrings, jarkko, dhowells

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.

Move the check to after __key_link_lock(), so it is done under the
semaphore, matching the pattern used in __key_create_or_update().

Fixes: 5ac7eace2d00 ("KEYS: Add a facility to restrict new links into
a keyring")
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Adrian Martinez <adrimg3196@gmail.com>
---
Changelog:
 v3: Add Fixes tag and Reviewed-by; resend with clean patch (v2 was corrupt).
 v2: Resend with full patch (v1 arrived without the diff).

 security/keys/key.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index 1234567..abcdefg 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -590,10 +590,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.
+			 */
+			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);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] KEYS: Fix key_reject_and_link() race with keyring restriction
  2026-10-08 21:03 [PATCH v3] KEYS: Fix key_reject_and_link() race with keyring restriction adrimg3196
@ 2026-10-10 19:36 ` Jarkko Sakkinen
  2026-10-10 20:41   ` Jarkko Sakkinen
  0 siblings, 1 reply; 3+ messages in thread
From: Jarkko Sakkinen @ 2026-10-10 19:36 UTC (permalink / raw)
  To: adrimg3196; +Cc: linux-kernel, keyrings, dhowells

On Thu, Oct 08, 2026 at 02:03:51PM -0700, adrimg3196@gmail.com wrote:
> 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.
> 
> Move the check to after __key_link_lock(), so it is done under the
> semaphore, matching the pattern used in __key_create_or_update().
> 
> Fixes: 5ac7eace2d00 ("KEYS: Add a facility to restrict new links into
> a keyring")
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Adrian Martinez <adrimg3196@gmail.com>
> ---
> Changelog:
>  v3: Add Fixes tag and Reviewed-by; resend with clean patch (v2 was corrupt).
>  v2: Resend with full patch (v1 arrived without the diff).
> 
>  security/keys/key.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 1234567..abcdefg 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -590,10 +590,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.
> +			 */
> +			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);

Thank you!

I applied this.


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Br, Jarkko

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] KEYS: Fix key_reject_and_link() race with keyring restriction
  2026-10-10 19:36 ` Jarkko Sakkinen
@ 2026-10-10 20:41   ` Jarkko Sakkinen
  0 siblings, 0 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2026-10-10 20:41 UTC (permalink / raw)
  To: adrimg3196; +Cc: linux-kernel, keyrings, dhowells

On Sat, Oct 10, 2026 at 10:36:27PM +0300, Jarkko Sakkinen wrote:
> On Thu, Oct 08, 2026 at 02:03:51PM -0700, adrimg3196@gmail.com wrote:
> > 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.
> > 
> > Move the check to after __key_link_lock(), so it is done under the
> > semaphore, matching the pattern used in __key_create_or_update().
> > 
> > Fixes: 5ac7eace2d00 ("KEYS: Add a facility to restrict new links into
> > a keyring")
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > Signed-off-by: Adrian Martinez <adrimg3196@gmail.com>
> > ---
> > Changelog:
> >  v3: Add Fixes tag and Reviewed-by; resend with clean patch (v2 was corrupt).
> >  v2: Resend with full patch (v1 arrived without the diff).
> > 
> >  security/keys/key.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/security/keys/key.c b/security/keys/key.c
> > index 1234567..abcdefg 100644
> > --- a/security/keys/key.c
> > +++ b/security/keys/key.c
> > @@ -590,10 +590,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.
> > +			 */
> > +			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);
> 
> Thank you!
> 
> I applied this.
> 
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

But scripts/checkpatch.pl complains:

-------------------------------------------------------------------------------------
Commit fd17650a59fc ("KEYS: Fix key_reject_and_link() race with keyring restriction")
-------------------------------------------------------------------------------------
WARNING: From:/Signed-off-by: email name mismatch: 'From: "adrimg3196@gmail.com" <adrimg3196@gmail.com>' != 'Signed-off-by: Adrian Martinez <adrimg3196@gmail.com>'

total: 0 errors, 1 warnings, 0 checks, 21 lines checked

I can fix this up, so no need for new version but it is good
to be aware of this if you plan to send anything in the future.

Br, Jarkko

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-10 20:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-08 21:03 [PATCH v3] KEYS: Fix key_reject_and_link() race with keyring restriction adrimg3196
2026-10-10 19:36 ` Jarkko Sakkinen
2026-10-10 20:41   ` Jarkko Sakkinen

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®