[PATCH] keys: fix lost wakeup when reaping a dead key type

[PATCH] keys: fix lost wakeup when reaping a dead key type

[Karl Mehltretter]

key_garbage_collector() clears KEY_GC_REAPING_KEYTYPE and calls
wake_up_bit() after reaping a dead key type. wake_up_bit() uses a
lockless waitqueue check and requires a full barrier after the clear.

The existing smp_mb() is before clear_bit(), so the GC can see an empty
waitqueue while unregister_key_type() still sees the bit set. The final
wakeup can then be lost, leaving module unload stuck in wait_on_bit().

Use clear_and_wake_up_bit(). Its clear_bit_unlock() preserves the
ordering of the completed GC work, and its smp_mb__after_atomic()
orders the clear before the waitqueue check.

Fixes: 0c061b5707ab ("KEYS: Correctly destroy key payloads when their keytype is removed")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
LKMM (herdtools7 7.58). LKMM has no clear_bit*() primitives, so these
tests abstract the bit clear as a store while preserving the ordering
relevant to this race. The fixed test models clear_bit_unlock() with
smp_store_release() and smp_mb__after_atomic() with smp_mb().

  C keys-gc-buggy
  { flag=1; }
  P0(int *flag, int *wq)
  {
  	int r0;
  	smp_mb();
  	WRITE_ONCE(*flag, 0);
  	r0 = READ_ONCE(*wq);
  }
  P1(int *flag, int *wq)
  {
  	int r1;
  	WRITE_ONCE(*wq, 1);
  	smp_mb();
  	r1 = READ_ONCE(*flag);
  }
  exists (0:r0=0 /\ 1:r1=1)

  C keys-gc-fixed
  { flag=1; }
  P0(int *flag, int *wq)
  {
  	int r0;
  	smp_store_release(flag, 0);
  	smp_mb();
  	r0 = READ_ONCE(*wq);
  }
  P1(int *flag, int *wq)
  {
  	int r1;
  	WRITE_ONCE(*wq, 1);
  	smp_mb();
  	r1 = READ_ONCE(*flag);
  }
  exists (0:r0=0 /\ 1:r1=1)

  herd7 -conf linux-kernel.cfg keys-gc-buggy.litmus
  herd7 -conf linux-kernel.cfg keys-gc-fixed.litmus

  pre-fix: Sometimes
  fixed:   Never

 security/keys/gc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/security/keys/gc.c b/security/keys/gc.c
index 748e83818a760..eda445f815d47 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -318,9 +318,7 @@ static void key_garbage_collector(struct work_struct *work)

 	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_3)) {
 		kdebug("dead wake");
-		smp_mb();
-		clear_bit(KEY_GC_REAPING_KEYTYPE, &key_gc_flags);
-		wake_up_bit(&key_gc_flags, KEY_GC_REAPING_KEYTYPE);
+		clear_and_wake_up_bit(KEY_GC_REAPING_KEYTYPE, &key_gc_flags);
 	}

 	if (gc_state & KEY_GC_REAP_AGAIN)
--
2.53.0

Re: [PATCH] keys: fix lost wakeup when reaping a dead key type

[Jarkko Sakkinen]

On Tue, Aug 11, 2026 at 07:37:53PM +0200, Karl Mehltretter wrote:
> key_garbage_collector() clears KEY_GC_REAPING_KEYTYPE and calls
> wake_up_bit() after reaping a dead key type. wake_up_bit() uses a
> lockless waitqueue check and requires a full barrier after the clear.
> 
> The existing smp_mb() is before clear_bit(), so the GC can see an empty
> waitqueue while unregister_key_type() still sees the bit set. The final
> wakeup can then be lost, leaving module unload stuck in wait_on_bit().
> 
> Use clear_and_wake_up_bit(). Its clear_bit_unlock() preserves the
> ordering of the completed GC work, and its smp_mb__after_atomic()
> orders the clear before the waitqueue check.
> 
> Fixes: 0c061b5707ab ("KEYS: Correctly destroy key payloads when their keytype is removed")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>


I think the fix good but it would be good first to have reminder that
while clear_bit() is an atomic operation as respect to the data it
holds, it is an unordered operation [1].

That is sort of the beginning of the story.

So yeah, tweak the commit message a bit and send v2.

[1] https://docs.kernel.org/core-api/wrappers/atomic_bitops.html

BR, Jarkko