]> www.infradead.org Git - users/hch/block.git/commit
jump_label: Fix concurrency issues in static_key_slow_dec()
authorThomas Gleixner <tglx@linutronix.de>
Mon, 10 Jun 2024 12:46:36 +0000 (14:46 +0200)
committerPeter Zijlstra <peterz@infradead.org>
Tue, 11 Jun 2024 09:25:23 +0000 (11:25 +0200)
commit83ab38ef0a0b2407d43af9575bb32333fdd74fb2
tree6f704708e2a7335a5f45ac72d542caba237ae338
parentbb9bb45f746b0f9457de9c3fc4da143a6351bdc9
jump_label: Fix concurrency issues in static_key_slow_dec()

The commit which tried to fix the concurrency issues of concurrent
static_key_slow_inc() failed to fix the equivalent issues
vs. static_key_slow_dec():

CPU0                     CPU1

static_key_slow_dec()
  static_key_slow_try_dec()

key->enabled == 1
val = atomic_fetch_add_unless(&key->enabled, -1, 1);
if (val == 1)
     return false;

  jump_label_lock();
  if (atomic_dec_and_test(&key->enabled)) {
     --> key->enabled == 0
   __jump_label_update()

 static_key_slow_dec()
   static_key_slow_try_dec()

     key->enabled == 0
     val = atomic_fetch_add_unless(&key->enabled, -1, 1);

      --> key->enabled == -1 <- FAIL

There is another bug in that code, when there is a concurrent
static_key_slow_inc() which enables the key as that sets key->enabled to -1
so on the other CPU

val = atomic_fetch_add_unless(&key->enabled, -1, 1);

will succeed and decrement to -2, which is invalid.

Cure all of this by replacing the atomic_fetch_add_unless() with a
atomic_try_cmpxchg() loop similar to static_key_fast_inc_not_disabled().

[peterz: add WARN_ON_ONCE for the -1 race]
Fixes: 4c5ea0a9cd02 ("locking/static_key: Fix concurrent static_key_slow_inc()")
Reported-by: Yue Sun <samsun1006219@gmail.com>
Reported-by: Xingwei Lee <xrivendell7@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20240610124406.422897838@linutronix.de
kernel/jump_label.c