[PATCH v2] keys: fix lost wakeup when reaping a dead key type
Jarkko Sakkinen
jarkko at kernel.org
Tue Aug 25 14:07:59 UTC 2026
On Fri, Aug 21, 2026 at 04:53:27AM +0200, Karl Mehltretter wrote:
> clear_bit() is atomic with respect to the word it modifies, but it is
> an unordered operation: it implies no memory barrier on either side
> (Documentation/atomic_bitops.txt).
>
> key_garbage_collector() clears KEY_GC_REAPING_KEYTYPE with clear_bit()
> 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 nothing orders the clear
> against that check. The GC can see an empty waitqueue while
> unregister_key_type() still sees the bit set. The final wakeup is then
> lost, leaving module unload stuck in wait_on_bit().
>
> Use clear_and_wake_up_bit(). Its clear_bit_unlock() has RELEASE
> semantics, so the completed GC work stays ordered before the clear, 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 at gmail.com>
> ---
> v2: open with the ordering semantics of clear_bit(), as suggested by
> Jarkko. No code change.
> v1: https://lore.kernel.org/r/20260811173753.67616-1-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
Reviewed-by: Jarkko Sakkinen <jarkko at kernel.org>
I send this to -rc2 as I already sent keys PR and I have quite heavy
TPM PR to prepare.
BR, Jarkko
More information about the Linux-security-module-archive
mailing list