[PATCH] KEYS: Reduce smp_mb() calls in key_put()

[PATCH] KEYS: Reduce smp_mb() calls in key_put()

[Jarkko Sakkinen]

Rely only on the memory ordering of spin_unlock() when setting
KEY_FLAG_FINAL_PUT under key->user->lock in key_put().

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/key.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index 7198cd2ac3a3..aecbd624612d 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -656,10 +656,12 @@ void key_put(struct key *key)
 				spin_lock_irqsave(&key->user->lock, flags);
 				key->user->qnkeys--;
 				key->user->qnbytes -= key->quotalen;
+				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
 				spin_unlock_irqrestore(&key->user->lock, flags);
+			} else {
+				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
+				smp_mb(); /* key->user before FINAL_PUT set. */
 			}
-			smp_mb(); /* key->user before FINAL_PUT set. */
-			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
 			schedule_work(&key_gc_work);
 		}
 	}
-- 
2.47.2

Re: [PATCH] KEYS: Reduce smp_mb() calls in key_put()

[Jarkko Sakkinen]

On Wed, Apr 30, 2025 at 06:25:53PM +0300, Jarkko Sakkinen wrote:
> Rely only on the memory ordering of spin_unlock() when setting
> KEY_FLAG_FINAL_PUT under key->user->lock in key_put().
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  security/keys/key.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 7198cd2ac3a3..aecbd624612d 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -656,10 +656,12 @@ void key_put(struct key *key)
>  				spin_lock_irqsave(&key->user->lock, flags);
>  				key->user->qnkeys--;
>  				key->user->qnbytes -= key->quotalen;
> +				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
>  				spin_unlock_irqrestore(&key->user->lock, flags);
> +			} else {
> +				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> +				smp_mb(); /* key->user before FINAL_PUT set. */
>  			}
> -			smp_mb(); /* key->user before FINAL_PUT set. */
> -			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);

Oops, my bad (order swap), sorry. Should have been:
	
 				spin_unlock_irqrestore(&key->user->lock, flags);
			} else {
				smp_mb(); /* key->user before FINAL_PUT set. */
 			}
			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);

Should spin_lock()/unlock() be good enough or what good does smp_mb() do
in that branch? Just checking if I'm missing something before sending
fixed version.

BR, Jarkko

Re: [PATCH] KEYS: Reduce smp_mb() calls in key_put()

[Herbert Xu]

On Sat, May 03, 2025 at 05:39:16PM +0300, Jarkko Sakkinen wrote:
> On Wed, Apr 30, 2025 at 06:25:53PM +0300, Jarkko Sakkinen wrote:
> > Rely only on the memory ordering of spin_unlock() when setting
> > KEY_FLAG_FINAL_PUT under key->user->lock in key_put().
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >  security/keys/key.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/security/keys/key.c b/security/keys/key.c
> > index 7198cd2ac3a3..aecbd624612d 100644
> > --- a/security/keys/key.c
> > +++ b/security/keys/key.c
> > @@ -656,10 +656,12 @@ void key_put(struct key *key)
> >  				spin_lock_irqsave(&key->user->lock, flags);
> >  				key->user->qnkeys--;
> >  				key->user->qnbytes -= key->quotalen;
> > +				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> >  				spin_unlock_irqrestore(&key->user->lock, flags);
> > +			} else {
> > +				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> > +				smp_mb(); /* key->user before FINAL_PUT set. */
> >  			}
> > -			smp_mb(); /* key->user before FINAL_PUT set. */
> > -			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> 
> Oops, my bad (order swap), sorry. Should have been:
> 	
>  				spin_unlock_irqrestore(&key->user->lock, flags);
> 			} else {
> 				smp_mb(); /* key->user before FINAL_PUT set. */

You can use smp_mb__before_atomic here as it is equivalent to
smp_mb in this situation.

>  			}
> 			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> 
> Should spin_lock()/unlock() be good enough or what good does smp_mb() do
> in that branch? Just checking if I'm missing something before sending
> fixed version.

I don't think spin_unlock alone is enough to replace an smp_mb.
A spin_lock + spin_unlock would be enough though.

However, looking at the bigger picture this smp_mb looks bogus.
What exactly is it protecting against?

The race condition that this is supposed to fix should have been
dealt with by the set_bit/test_bit of FINAL_PUT alone.  I don't
see any point in having this smb_mb at all.

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

Re: [PATCH] KEYS: Reduce smp_mb() calls in key_put()

[David Howells]

Jarkko Sakkinen <jarkko@kernel.org> wrote:

> Oops, my bad (order swap), sorry. Should have been:
> 	
>  				spin_unlock_irqrestore(&key->user->lock, flags);
> 			} else {
> 				smp_mb(); /* key->user before FINAL_PUT set. */
>  			}
> 			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> 
> Should spin_lock()/unlock() be good enough or what good does smp_mb() do
> in that branch? Just checking if I'm missing something before sending
> fixed version.

spin_unlock() is semi-permeable, so stuff after it can leak into the inside of
it up as far as the spin_lock().  With your change, the garbage collector can
no longer guarantee that key_put() will have done with accessing key->user
when it sees KEY_FLAG_FINAL_PUT is set.

So, NAK on this patch, I think.  If you want a second opinion, I'd suggest
waving it in front of Paul McKenney.

Possibly we only need smp_mb() in the IN_QUOTA branch in key_put().

David

Re: [PATCH] KEYS: Reduce smp_mb() calls in key_put()

[Herbert Xu]

On Sat, May 03, 2025 at 11:19:21PM +0100, David Howells wrote:
>
> Possibly we only need smp_mb() in the IN_QUOTA branch in key_put().

Just change the smp_mb to smp_mb__before_atomic, at least on x86
it just disappears because set_bit is already a serialising operation.

Or even better, reverse the FINAL_PUT bit and call it ALIVE, so
that you can use test_bit_acquire and clear_bit_unlock.

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

[PATCH] KEYS: Invert FINAL_PUT bit

[Herbert Xu]

On Sun, May 04, 2025 at 08:35:57AM +0800, Herbert Xu wrote:
> 
> Or even better, reverse the FINAL_PUT bit and call it ALIVE, so
> that you can use test_bit_acquire and clear_bit_unlock.

Something like:

---8<---
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>

diff --git a/include/linux/key.h b/include/linux/key.h
index ba05de8579ec..aaab26d84d25 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_DONT_GC_YET	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..d00002054ada 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_DONT_GC_YET, &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..7be12d132c4e 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 |= KEY_FLAG_DONT_GC_YET;
 	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_DONT_GC_YET, &key->flags);
 			schedule_work(&key_gc_work);
 		}
 	}
-- 
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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[David Howells]

Herbert Xu <herbert@gondor.apana.org.au> wrote:

> +	key->flags |= KEY_FLAG_DONT_GC_YET;

You need __set_bit() or 1<<N.

Also, don't really like the name, but that's just bikeshedding.  I think I'd
lean more to your initial suggestion of KEY_FLAG_ALIVE.

David

[v2 PATCH] KEYS: Invert FINAL_PUT bit

[Herbert Xu]

On Sun, May 04, 2025 at 08:44:13AM +0100, David Howells wrote:
> 
> You need __set_bit() or 1<<N.

Sorry, I'll fix that.

> Also, don't really like the name, but that's just bikeshedding.  I think I'd
> lean more to your initial suggestion of KEY_FLAG_ALIVE.

I was going to do that but there is already a flag called
KEY_FLAG_DEAD and it would be very confusing since they mean
completely diferent things.

How about USER_ALIVE?

---8<---
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>

diff --git a/include/linux/key.h b/include/linux/key.h
index ba05de8579ec..aaab26d84d25 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..d00002054ada 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..fb78c3a0be76 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);
 		}
 	}

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

Re: [PATCH] KEYS: Reduce smp_mb() calls in key_put()

[Jarkko Sakkinen]

On Sat, May 03, 2025 at 11:19:21PM +0100, David Howells wrote:
> Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
> > Oops, my bad (order swap), sorry. Should have been:
> > 	
> >  				spin_unlock_irqrestore(&key->user->lock, flags);
> > 			} else {
> > 				smp_mb(); /* key->user before FINAL_PUT set. */
> >  			}
> > 			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> > 
> > Should spin_lock()/unlock() be good enough or what good does smp_mb() do
> > in that branch? Just checking if I'm missing something before sending
> > fixed version.
> 
> spin_unlock() is semi-permeable, so stuff after it can leak into the inside of
> it up as far as the spin_lock().  With your change, the garbage collector can
> no longer guarantee that key_put() will have done with accessing key->user
> when it sees KEY_FLAG_FINAL_PUT is set.
> 
> So, NAK on this patch, I think.  If you want a second opinion, I'd suggest
> waving it in front of Paul McKenney.

Fair enough.

If I revisit this in a way or another,  I'll cc to him for comments but
for this I'll buy what you said.

> 
> Possibly we only need smp_mb() in the IN_QUOTA branch in key_put().
> 
> David

Thank you for the comments.

BR, Jarkko

Re: [PATCH] KEYS: Reduce smp_mb() calls in key_put()

[Jarkko Sakkinen]

On Sat, May 03, 2025 at 11:02:57PM +0800, Herbert Xu wrote:
> On Sat, May 03, 2025 at 05:39:16PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Apr 30, 2025 at 06:25:53PM +0300, Jarkko Sakkinen wrote:
> > > Rely only on the memory ordering of spin_unlock() when setting
> > > KEY_FLAG_FINAL_PUT under key->user->lock in key_put().
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > >  security/keys/key.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > index 7198cd2ac3a3..aecbd624612d 100644
> > > --- a/security/keys/key.c
> > > +++ b/security/keys/key.c
> > > @@ -656,10 +656,12 @@ void key_put(struct key *key)
> > >  				spin_lock_irqsave(&key->user->lock, flags);
> > >  				key->user->qnkeys--;
> > >  				key->user->qnbytes -= key->quotalen;
> > > +				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> > >  				spin_unlock_irqrestore(&key->user->lock, flags);
> > > +			} else {
> > > +				set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> > > +				smp_mb(); /* key->user before FINAL_PUT set. */
> > >  			}
> > > -			smp_mb(); /* key->user before FINAL_PUT set. */
> > > -			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> > 
> > Oops, my bad (order swap), sorry. Should have been:
> > 	
> >  				spin_unlock_irqrestore(&key->user->lock, flags);
> > 			} else {
> > 				smp_mb(); /* key->user before FINAL_PUT set. */
> 
> You can use smp_mb__before_atomic here as it is equivalent to
> smp_mb in this situation.
> 
> >  			}
> > 			set_bit(KEY_FLAG_FINAL_PUT, &key->flags);
> > 
> > Should spin_lock()/unlock() be good enough or what good does smp_mb() do
> > in that branch? Just checking if I'm missing something before sending
> > fixed version.
> 
> I don't think spin_unlock alone is enough to replace an smp_mb.
> A spin_lock + spin_unlock would be enough though.
> 
> However, looking at the bigger picture this smp_mb looks bogus.
> What exactly is it protecting against?
> 
> The race condition that this is supposed to fix should have been
> dealt with by the set_bit/test_bit of FINAL_PUT alone.  I don't
> see any point in having this smb_mb at all.

smp_mb() there makes sure that key->user change don't spill between
key_put() and gc.

GC pairs smp_mb() in key_put() after FINAL_PUT to make sure that also
in its side key->user changes have been walled before moving the key
as part of unrefenced keys.

See also [1]. It cleared this up for me. Here user->lock easily misleads
to overlook the actual synchronization scheme.

> 
> 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

[1] https://lore.kernel.org/keyrings/1121543.1746310761@warthog.procyon.org.uk/

BR, Jarkko

Re: [v2 PATCH] KEYS: Invert FINAL_PUT bit

[kernel test robot]

Hello,


our bot applied this patch directly upon v6.15-rc5. could you let us know if
this is a correct appliment?

* a78cdfa4388ab9 (linux-review/Herbert-Xu/KEYS-Invert-FINAL_PUT-bit/20250505-122533) KEYS: Invert FINAL_PUT bit
* 92a09c47464d04 (tag: v6.15-rc5, 


below reports is based on this appliement.


kernel test robot noticed "refcount_t:underflow;use-after-free" on:

commit: a78cdfa4388ab9b210c804b92453f14bbe199cbf ("[v2 PATCH] KEYS: Invert FINAL_PUT bit")
url: https://github.com/intel-lab-lkp/linux/commits/Herbert-Xu/KEYS-Invert-FINAL_PUT-bit/20250505-122533
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 92a09c47464d040866cf2b4cd052bc60555185fb
patch link: https://lore.kernel.org/all/aBccz2nJs5Asg6cN@gondor.apana.org.au/
patch subject: [v2 PATCH] KEYS: Invert FINAL_PUT bit

in testcase: trinity
version: trinity-i386-abe9de86-1_20230429
with following parameters:

	runtime: 300s
	group: group-04
	nr_groups: 5



config: i386-randconfig-014-20250509
compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


there are other (random) issues as below.

+-------------------------------------------------------------------------+-----------+------------+
|                                                                         | v6.15-rc5 | a78cdfa438 |
+-------------------------------------------------------------------------+-----------+------------+
| boot_successes                                                          | 80        | 0          |
| boot_failures                                                           | 0         | 48         |
| refcount_t:underflow;use-after-free                                     | 0         | 48         |
| WARNING:at_lib/refcount.c:#refcount_warn_saturate                       | 0         | 47         |
| EIP:refcount_warn_saturate                                              | 0         | 48         |
| addition_on#;use-after-free                                             | 0         | 46         |
| saturated;leaking_memory                                                | 0         | 44         |
| BUG:kernel_NULL_pointer_dereference,address                             | 0         | 31         |
| Oops                                                                    | 0         | 41         |
| EIP:keyctl_read_key                                                     | 0         | 27         |
| Kernel_panic-not_syncing:Fatal_exception                                | 0         | 36         |
| BUG:unable_to_handle_page_fault_for_address                             | 0         | 10         |
| EIP:key_put                                                             | 0         | 1          |
| Kernel_panic-not_syncing:Fatal_exception_in_interrupt                   | 0         | 5          |
| EIP:kmem_cache_alloc_noprof                                             | 0         | 2          |
| BUG:Bad_rss-counter_state_mm:#type:MM_SWAPENTS_val                      | 0         | 1          |
| EIP:keyctl_describe_key                                                 | 0         | 1          |
| EIP:keyring_gc_check_iterator                                           | 0         | 1          |
| EIP:dst_destroy                                                         | 0         | 3          |
| EIP:_raw_spin_unlock_irqrestore                                         | 0         | 1          |
| EIP:put_pid                                                             | 0         | 4          |
| EIP:rb_erase                                                            | 0         | 1          |
| EIP:kernel_init_pages                                                   | 0         | 1          |
| EIP:lookup_user_key                                                     | 0         | 1          |
| EIP:strlen                                                              | 0         | 1          |
| INFO:task_blocked_for_more_than#seconds                                 | 0         | 1          |
| BUG:kernel_hang_in_test_stage                                           | 0         | 1          |
+-------------------------------------------------------------------------+-----------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202505091721.245cbe78-lkp@intel.com


[    8.510562][   T60] ------------[ cut here ]------------
[    8.511283][   T60] refcount_t: underflow; use-after-free.
[ 8.511950][ T60] WARNING: CPU: 0 PID: 60 at lib/refcount.c:28 refcount_warn_saturate (kbuild/obj/consumer/i386-randconfig-014-20250509/lib/refcount.c:28 (discriminator 3)) 
[    8.512948][   T60] Modules linked in:
[    8.513488][   T60] CPU: 0 UID: 0 PID: 60 Comm: kworker/0:2 Not tainted 6.15.0-rc5-00001-ga78cdfa4388a #1 PREEMPT  231a29fdcec5e4259d3c91818150ae4baf2b3615
[    8.514973][   T60] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    8.516145][   T60] Workqueue: events key_garbage_collector
[ 8.516849][ T60] EIP: refcount_warn_saturate (kbuild/obj/consumer/i386-randconfig-014-20250509/lib/refcount.c:28 (discriminator 3)) 
[ 8.517490][ T60] Code: fa c2 82 01 68 28 15 60 82 e8 e3 88 72 ff 0f 0b 58 c9 c3 8d b6 00 00 00 00 c6 05 2e fa c2 82 01 68 d0 14 60 82 e8 c7 88 72 ff <0f> 0b 59 c9 c3 66 90 89 c2 8b 00 3d 00 00 00 c0 74 12 83 f8 01 74
All code
========
   0:	fa                   	cli
   1:	c2 82 01             	ret    $0x182
   4:	68 28 15 60 82       	push   $0xffffffff82601528
   9:	e8 e3 88 72 ff       	call   0xffffffffff7288f1
   e:	0f 0b                	ud2
  10:	58                   	pop    %rax
  11:	c9                   	leave
  12:	c3                   	ret
  13:	8d b6 00 00 00 00    	lea    0x0(%rsi),%esi
  19:	c6 05 2e fa c2 82 01 	movb   $0x1,-0x7d3d05d2(%rip)        # 0xffffffff82c2fa4e
  20:	68 d0 14 60 82       	push   $0xffffffff826014d0
  25:	e8 c7 88 72 ff       	call   0xffffffffff7288f1
  2a:*	0f 0b                	ud2		<-- trapping instruction
  2c:	59                   	pop    %rcx
  2d:	c9                   	leave
  2e:	c3                   	ret
  2f:	66 90                	xchg   %ax,%ax
  31:	89 c2                	mov    %eax,%edx
  33:	8b 00                	mov    (%rax),%eax
  35:	3d 00 00 00 c0       	cmp    $0xc0000000,%eax
  3a:	74 12                	je     0x4e
  3c:	83 f8 01             	cmp    $0x1,%eax
  3f:	74                   	.byte 0x74

Code starting with the faulting instruction
===========================================
   0:	0f 0b                	ud2
   2:	59                   	pop    %rcx
   3:	c9                   	leave
   4:	c3                   	ret
   5:	66 90                	xchg   %ax,%ax
   7:	89 c2                	mov    %eax,%edx
   9:	8b 00                	mov    (%rax),%eax
   b:	3d 00 00 00 c0       	cmp    $0xc0000000,%eax
  10:	74 12                	je     0x24
  12:	83 f8 01             	cmp    $0x1,%eax
  15:	74                   	.byte 0x74
[    8.519470][   T60] EAX: 00000026 EBX: 85c8c9c0 ECX: 0000025c EDX: 00000000
[    8.520241][   T60] ESI: 85d4ede0 EDI: 821a0f00 EBP: 8405fe6c ESP: 8405fe68
[    8.521168][   T60] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010286
[    8.522055][   T60] CR0: 80050033 CR2: 77ecb6a1 CR3: 040b8000 CR4: 000406f0
[    8.522824][   T60] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[    8.523614][   T60] DR6: fffe0ff0 DR7: 00000400
[    8.524161][   T60] Call Trace:
[ 8.524619][ T60] key_put (kbuild/obj/consumer/i386-randconfig-014-20250509/include/linux/refcount.h:400 kbuild/obj/consumer/i386-randconfig-014-20250509/include/linux/refcount.h:432 kbuild/obj/consumer/i386-randconfig-014-20250509/include/linux/refcount.h:450 kbuild/obj/consumer/i386-randconfig-014-20250509/security/keys/key.c:652) 
[ 8.525119][ T60] keyring_free_object (kbuild/obj/consumer/i386-randconfig-014-20250509/security/keys/keyring.c:390) 
[ 8.525736][ T60] assoc_array_destroy_subtree+0x7b/0x17c 
[ 8.526446][ T60] assoc_array_destroy (kbuild/obj/consumer/i386-randconfig-014-20250509/lib/assoc_array.c:445) 
[ 8.527048][ T60] keyring_destroy (kbuild/obj/consumer/i386-randconfig-014-20250509/security/keys/keyring.c:432) 
[ 8.527617][ T60] key_gc_unused_keys+0xfb/0x134 
[ 8.528301][ T60] key_garbage_collector (kbuild/obj/consumer/i386-randconfig-014-20250509/security/keys/gc.c:305) 
[ 8.528967][ T60] process_one_work (kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/workqueue.c:3243) 
[ 8.529586][ T60] worker_thread (kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/workqueue.c:3313 kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/workqueue.c:3400) 
[ 8.530157][ T60] kthread (kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/kthread.c:464) 
[ 8.530681][ T60] ? rescuer_thread (kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/workqueue.c:3346) 
[ 8.531244][ T60] ? kthread_fetch_affinity+0x34/0x34 
[ 8.531930][ T60] ret_from_fork (kbuild/obj/consumer/i386-randconfig-014-20250509/arch/x86/kernel/process.c:159) 
[ 8.532498][ T60] ? kthread_fetch_affinity+0x34/0x34 
[ 8.533164][ T60] ret_from_fork_asm (kbuild/obj/consumer/i386-randconfig-014-20250509/arch/x86/entry/entry_32.S:737) 
[ 8.533766][ T60] entry_INT80_32 (kbuild/obj/consumer/i386-randconfig-014-20250509/arch/x86/entry/entry_32.S:945) 
[    8.534333][   T60] irq event stamp: 3905
[ 8.534868][ T60] hardirqs last enabled at (3917): __up_console_sem (kbuild/obj/consumer/i386-randconfig-014-20250509/arch/x86/include/asm/irqflags.h:42 (discriminator 1) kbuild/obj/consumer/i386-randconfig-014-20250509/arch/x86/include/asm/irqflags.h:119 (discriminator 1) kbuild/obj/consumer/i386-randconfig-014-20250509/arch/x86/include/asm/irqflags.h:159 (discriminator 1) kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/printk/printk.c:344 (discriminator 1)) 
[ 8.535880][ T60] hardirqs last disabled at (3928): __up_console_sem (kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/printk/printk.c:342 (discriminator 1)) 
[ 8.535891][ T60] softirqs last enabled at (3856): handle_softirqs (kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/softirq.c:426 kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/softirq.c:607) 
[ 8.535896][ T60] softirqs last disabled at (3851): __do_softirq (kbuild/obj/consumer/i386-randconfig-014-20250509/kernel/softirq.c:614) 
[    8.535904][   T60] ---[ end trace 0000000000000000 ]---


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250509/202505091721.245cbe78-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[v3 PATCH] KEYS: Invert FINAL_PUT bit

[Herbert Xu]

On Fri, May 09, 2025 at 05:34:09PM +0800, kernel test robot wrote:
> 
> our bot applied this patch directly upon v6.15-rc5. could you let us know if
> this is a correct appliment?

Yes it is correct.

The patch was buggy, the test_bit_acquire should've been inverted too:

---8<---
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>

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);
 		}
 	}
-- 
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

Re: [v3 PATCH] KEYS: Invert FINAL_PUT bit

[David Howells]

Herbert Xu <herbert@gondor.apana.org.au> wrote:

> 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>

Acked-by: David Howells <dhowells@redhat.com>

Re: [v3 PATCH] KEYS: Invert FINAL_PUT bit

[Jarkko Sakkinen]

On Mon, May 12, 2025 at 10:19:14AM +0100, David Howells wrote:
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > 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>
> 
> Acked-by: David Howells <dhowells@redhat.com>

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

David, are you going to pick this?

BR, Jarkko

Re: [v3 PATCH] KEYS: Invert FINAL_PUT bit

[David Howells]

Jarkko Sakkinen <jarkko@kernel.org> wrote:

> > Acked-by: David Howells <dhowells@redhat.com>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> David, are you going to pick this?

I can do unless Herbert wants to pass it through his tree?

David

Re: [v3 PATCH] KEYS: Invert FINAL_PUT bit

[Herbert Xu]

On Mon, May 12, 2025 at 01:01:50PM +0100, David Howells wrote:
>
> I can do unless Herbert wants to pass it through his tree?

I have no desire to do that :)

Thanks,
-- 
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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Jarkko Sakkinen]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Linus Torvalds]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Al Viro]

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/?

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Linus Torvalds]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[David Howells]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Linus Torvalds]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Jarkko Sakkinen]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Paul Moore]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Herbert Xu]

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

Re: [PATCH] KEYS: Invert FINAL_PUT bit

[Paul Moore]

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

[PATCH] KEYS: Invert FINAL_PUT bit

[David Howells]

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);
 		}
 	}