Hi Linus,

Could you apply this, please?  There shouldn't be any functional change,
rather it's a switch to using combined bit-barrier ops and lesser barriers.
A better way to do this might be to provide set_bit_release(), but the end
result would be much the same.

Thanks,
David
---
From: Herbert Xu <herbert@gondor.apana.org.au>

KEYS: Invert FINAL_PUT bit

Invert the FINAL_PUT bit so that test_bit_acquire and clear_bit_unlock
can be used instead of smp_mb.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
cc: keyrings@vger.kernel.org
cc: linux-security-module@vger.kernel.org
cc: linux-crypto@vger.kernel.org
cc: linux-integrity@vger.kernel.org
---
 include/linux/key.h |    2 +-
 security/keys/gc.c  |    4 ++--
 security/keys/key.c |    5 +++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index ba05de8579ec..81b8f05c6898 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -236,7 +236,7 @@ struct key {
 #define KEY_FLAG_ROOT_CAN_INVAL	7	/* set if key can be invalidated by root without permission */
 #define KEY_FLAG_KEEP		8	/* set if key should not be removed */
 #define KEY_FLAG_UID_KEYRING	9	/* set if key is a user or user session keyring */
-#define KEY_FLAG_FINAL_PUT	10	/* set if final put has happened on key */
+#define KEY_FLAG_USER_ALIVE	10	/* set if final put has not happened on key yet */
 
 	/* the key type and key description string
 	 * - the desc is used to match a key against search criteria
diff --git a/security/keys/gc.c b/security/keys/gc.c
index f27223ea4578..748e83818a76 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -218,8 +218,8 @@ static void key_garbage_collector(struct work_struct *work)
 		key = rb_entry(cursor, struct key, serial_node);
 		cursor = rb_next(cursor);
 
-		if (test_bit(KEY_FLAG_FINAL_PUT, &key->flags)) {
-			smp_mb(); /* Clobber key->user after FINAL_PUT seen. */
+		if (!test_bit_acquire(KEY_FLAG_USER_ALIVE, &key->flags)) {
+			/* Clobber key->user after final put seen. */
 			goto found_unreferenced_key;
 		}
 
diff --git a/security/keys/key.c b/security/keys/key.c
index 7198cd2ac3a3..3bbdde778631 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -298,6 +298,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	key->restrict_link = restrict_link;
 	key->last_used_at = ktime_get_real_seconds();
 
+	key->flags |= 1 << KEY_FLAG_USER_ALIVE;
 	if (!(flags & KEY_ALLOC_NOT_IN_QUOTA))
 		key->flags |= 1 << KEY_FLAG_IN_QUOTA;
 	if (flags & KEY_ALLOC_BUILT_IN)
@@ -658,8 +659,8 @@ void key_put(struct key *key)
 				key->user->qnbytes -= key->quotalen;
 				spin_unlock_irqrestore(&key->user->lock, flags);
 			}
-			smp_mb(); /* key->user before FINAL_PUT set. */
-			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
+			/* Mark key as safe for GC after key->user done. */
+			clear_bit_unlock(KEY_FLAG_USER_ALIVE, &key->flags);
 			schedule_work(&key_gc_work);
 		}
 	}

On Wed, May 28, 2025 at 8:19 AM David Howells <dhowells@redhat.com> wrote:
>
> Hi Linus,
>
> Could you apply this, please?  There shouldn't be any functional change,
> rather it's a switch to using combined bit-barrier ops and lesser barriers.
> A better way to do this might be to provide set_bit_release(), but the end
> result would be much the same.
>
> Thanks,
> David
> ---
> From: Herbert Xu <herbert@gondor.apana.org.au>
>
> KEYS: Invert FINAL_PUT bit
>
> Invert the FINAL_PUT bit so that test_bit_acquire and clear_bit_unlock
> can be used instead of smp_mb.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> cc: keyrings@vger.kernel.org
> cc: linux-security-module@vger.kernel.org
> cc: linux-crypto@vger.kernel.org
> cc: linux-integrity@vger.kernel.org
> ---
>  include/linux/key.h |    2 +-
>  security/keys/gc.c  |    4 ++--
>  security/keys/key.c |    5 +++--
>  3 files changed, 6 insertions(+), 5 deletions(-)

It doesn't look like this has made its way to Linus.  David or Jarkko,
do one of you want to pick this up into a tree and send this to Linus
properly?

> diff --git a/include/linux/key.h b/include/linux/key.h
> index ba05de8579ec..81b8f05c6898 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -236,7 +236,7 @@ struct key {
>  #define KEY_FLAG_ROOT_CAN_INVAL        7       /* set if key can be invalidated by root without permission */
>  #define KEY_FLAG_KEEP          8       /* set if key should not be removed */
>  #define KEY_FLAG_UID_KEYRING   9       /* set if key is a user or user session keyring */
> -#define KEY_FLAG_FINAL_PUT     10      /* set if final put has happened on key */
> +#define KEY_FLAG_USER_ALIVE    10      /* set if final put has not happened on key yet */
>
>         /* the key type and key description string
>          * - the desc is used to match a key against search criteria
> diff --git a/security/keys/gc.c b/security/keys/gc.c
> index f27223ea4578..748e83818a76 100644
> --- a/security/keys/gc.c
> +++ b/security/keys/gc.c
> @@ -218,8 +218,8 @@ static void key_garbage_collector(struct work_struct *work)
>                 key = rb_entry(cursor, struct key, serial_node);
>                 cursor = rb_next(cursor);
>
> -               if (test_bit(KEY_FLAG_FINAL_PUT, &key->flags)) {
> -                       smp_mb(); /* Clobber key->user after FINAL_PUT seen. */
> +               if (!test_bit_acquire(KEY_FLAG_USER_ALIVE, &key->flags)) {
> +                       /* Clobber key->user after final put seen. */
>                         goto found_unreferenced_key;
>                 }
>
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 7198cd2ac3a3..3bbdde778631 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -298,6 +298,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>         key->restrict_link = restrict_link;
>         key->last_used_at = ktime_get_real_seconds();
>
> +       key->flags |= 1 << KEY_FLAG_USER_ALIVE;
>         if (!(flags & KEY_ALLOC_NOT_IN_QUOTA))
>                 key->flags |= 1 << KEY_FLAG_IN_QUOTA;
>         if (flags & KEY_ALLOC_BUILT_IN)
> @@ -658,8 +659,8 @@ void key_put(struct key *key)
>                                 key->user->qnbytes -= key->quotalen;
>                                 spin_unlock_irqrestore(&key->user->lock, flags);
>                         }
> -                       smp_mb(); /* key->user before FINAL_PUT set. */
> -                       set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> +                       /* Mark key as safe for GC after key->user done. */
> +                       clear_bit_unlock(KEY_FLAG_USER_ALIVE, &key->flags);
>                         schedule_work(&key_gc_work);
>                 }
>         }
>
>

-- 
paul-moore.com

On Tue, 10 Jun 2025 at 17:23, Paul Moore <paul@paul-moore.com> wrote:
>
> It doesn't look like this has made its way to Linus.

Bah. It "made it" in the sense that sure, it's in my inbox.

But particularly during the the early merge window I end up heavily
limiting my emails to pull requests. And then it ended up composted at
the bottom of my endless pile of emails.

I guess I can still take it if people just say "do it".

            Linus

On Wed, Jun 11, 2025 at 10:50:46AM -0700, Linus Torvalds wrote:
> On Tue, 10 Jun 2025 at 17:23, Paul Moore <paul@paul-moore.com> wrote:
> >
> > It doesn't look like this has made its way to Linus.
> 
> Bah. It "made it" in the sense that sure, it's in my inbox.
> 
> But particularly during the the early merge window I end up heavily
> limiting my emails to pull requests. And then it ended up composted at
> the bottom of my endless pile of emails.
> 
> I guess I can still take it if people just say "do it".
> 
>             Linus

+1 for picking it.

BR, Jarkko

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> I guess I can still take it if people just say "do it".

Do you want a signed tag and git pull for it?

David

On Wed, 11 Jun 2025 at 11:45, David Howells <dhowells@redhat.com> wrote:
>
> Do you want a signed tag and git pull for it?

Particularly during the merge window that makes sense just to make it
trigger my usual "git pull" pattern, but now that I'm more aware of it
I can just take the patch directly.

Anyway - done just to get this behind us. But for next time, just do
it as a signed tag pull request, _particularly_ during the merge
window when most other emails get much lower priority.

             Linus

On Wed, Jun 11, 2025 at 11:59:19AM -0700, Linus Torvalds wrote:
> On Wed, 11 Jun 2025 at 11:45, David Howells <dhowells@redhat.com> wrote:
> >
> > Do you want a signed tag and git pull for it?
> 
> Particularly during the merge window that makes sense just to make it
> trigger my usual "git pull" pattern, but now that I'm more aware of it
> I can just take the patch directly.
> 
> Anyway - done just to get this behind us. But for next time, just do
> it as a signed tag pull request, _particularly_ during the merge
> window when most other emails get much lower priority.

Speaking of the stuff fallen through the cracks - could you take another
look at https://lore.kernel.org/all/20250602041118.GA2675383@ZenIV/?

On Wed, 11 Jun 2025 at 13:38, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Speaking of the stuff fallen through the cracks - could you take another
> look at https://lore.kernel.org/all/20250602041118.GA2675383@ZenIV/?

Also done.

Well, the script part is, it's still doing the test-build and I'll
have to make a commit message etc.

              Linus

On Tue, Jun 10, 2025 at 08:22:59PM -0400, Paul Moore wrote:
> On Wed, May 28, 2025 at 8:19 AM David Howells <dhowells@redhat.com> wrote:
> >
> > Hi Linus,
> >
> > Could you apply this, please?  There shouldn't be any functional change,
> > rather it's a switch to using combined bit-barrier ops and lesser barriers.
> > A better way to do this might be to provide set_bit_release(), but the end
> > result would be much the same.
> >
> > Thanks,
> > David
> > ---
> > From: Herbert Xu <herbert@gondor.apana.org.au>
> >
> > KEYS: Invert FINAL_PUT bit
> >
> > Invert the FINAL_PUT bit so that test_bit_acquire and clear_bit_unlock
> > can be used instead of smp_mb.
> >
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > cc: keyrings@vger.kernel.org
> > cc: linux-security-module@vger.kernel.org
> > cc: linux-crypto@vger.kernel.org
> > cc: linux-integrity@vger.kernel.org
> > ---
> >  include/linux/key.h |    2 +-
> >  security/keys/gc.c  |    4 ++--
> >  security/keys/key.c |    5 +++--
> >  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> It doesn't look like this has made its way to Linus.  David or Jarkko,
> do one of you want to pick this up into a tree and send this to Linus
> properly?

I'm open for anything but need comment from David at first. It is up to
him as he carries the torch ATM for this one :-)

BR, Jarkko

On Tue, Jun 10, 2025 at 08:22:59PM -0400, Paul Moore wrote:
>
> It doesn't look like this has made its way to Linus.  David or Jarkko,
> do one of you want to pick this up into a tree and send this to Linus
> properly?

I can pick it up for the next merge window.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

On Wed, Jun 11, 2025 at 5:12 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Jun 10, 2025 at 08:22:59PM -0400, Paul Moore wrote:
> >
> > It doesn't look like this has made its way to Linus.  David or Jarkko,
> > do one of you want to pick this up into a tree and send this to Linus
> > properly?
>
> I can pick it up for the next merge window.

Great, thanks Herbert.

-- 
paul-moore.com