linux.git/kernel/jump_label.c, branch v6.12.80

jump_label: Fix static_key_slow_dec() yet again

2024-09-10T09:57:27+00:00

While commit 83ab38ef0a0b ("jump_label: Fix concurrency issues in
static_key_slow_dec()") fixed one problem, it created yet another,
notably the following is now possible:

  slow_dec
    if (try_dec) // dec_not_one-ish, false
    // enabled == 1
                                slow_inc
                                  if (inc_not_disabled) // inc_not_zero-ish
                                  // enabled == 2
                                    return

    guard((mutex)(&jump_label_mutex);
    if (atomic_cmpxchg(1,0)==1) // false, we're 2

                                slow_dec
                                  if (try-dec) // dec_not_one, true
                                  // enabled == 1
                                    return
    else
      try_dec() // dec_not_one, false
      WARN

Use dec_and_test instead of cmpxchg(), like it was prior to
83ab38ef0a0b. Add a few WARNs for the paranoid.

Fixes: 83ab38ef0a0b ("jump_label: Fix concurrency issues in static_key_slow_dec()")
Reported-by: "Darrick J. Wong" 
Tested-by: Klara Modin 
Signed-off-by: Peter Zijlstra (Intel)

jump_label: Fix the fix, brown paper bags galore

2024-07-31T10:57:39+00:00

Per the example of:

  !atomic_cmpxchg(&key->enabled, 0, 1)

the inverse was written as:

  atomic_cmpxchg(&key->enabled, 1, 0)

except of course, that while !old is only true for old == 0, old is
true for everything except old == 0.

Fix it to read:

  atomic_cmpxchg(&key->enabled, 1, 0) == 1

such that only the 1->0 transition returns true and goes on to disable
the keys.

Fixes: 83ab38ef0a0b ("jump_label: Fix concurrency issues in static_key_slow_dec()")
Reported-by: Darrick J. Wong 
Signed-off-by: Peter Zijlstra (Intel) 
Tested-by: Darrick J. Wong 
Link: https://lkml.kernel.org/r/20240731105557.GY33588@noisy.programming.kicks-ass.net

jump_label: Simplify and clarify static_key_fast_inc_cpus_locked()

2024-06-11T09:25:24+00:00

Make the code more obvious and add proper comments to avoid future head
scratching.

Signed-off-by: Thomas Gleixner 
Signed-off-by: Peter Zijlstra (Intel) 
Link: https://lkml.kernel.org/r/20240610124406.548322963@linutronix.de

jump_label: Clarify condition in static_key_fast_inc_not_disabled()

2024-06-11T09:25:23+00:00

The second part of

      if (v <= 0 || (v + 1) < 0)

is not immediately obvious that it acts as overflow protection.

Check explicitely for v == INT_MAX instead and add a proper comment how
this is used at the call sites.

Signed-off-by: Thomas Gleixner 
Signed-off-by: Peter Zijlstra (Intel) 
Link: https://lkml.kernel.org/r/20240610124406.484973160@linutronix.de

jump_label: Fix concurrency issues in static_key_slow_dec()

2024-06-11T09:25:23+00:00

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 
Reported-by: Xingwei Lee 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Peter Zijlstra (Intel) 
Link: https://lkml.kernel.org/r/20240610124406.422897838@linutronix.de

jump_label,module: Don't alloc static_key_mod for __ro_after_init keys

2024-03-22T10:18:16+00:00

When a static_key is marked ro_after_init, its state will never change
(after init), therefore jump_label_update() will never need to iterate
the entries, and thus module load won't actually need to track this --
avoiding the static_key::next write.

Therefore, mark these keys such that jump_label_add_module() might
recognise them and avoid the modification.

Use the special state: 'static_key_linked(key) && !static_key_mod(key)'
to denote such keys.

jump_label_add_module() does not exist under CONFIG_JUMP_LABEL=n, so the
newly-introduced jump_label_init_ro() can be defined as a nop for that
configuration.

[ mingo: Renamed jump_label_ro() to jump_label_init_ro() ]

Signed-off-by: Peter Zijlstra (Intel) 
Signed-off-by: Valentin Schneider 
Signed-off-by: Ingo Molnar 
Acked-by: Josh Poimboeuf 
Link: https://lore.kernel.org/r/20240313180106.2917308-2-vschneid@redhat.com

jump_label: Prevent key->enabled int overflow

2022-12-01T23:53:05+00:00

1. With CONFIG_JUMP_LABEL=n static_key_slow_inc() doesn't have any
   protection against key->enabled refcounter overflow.
2. With CONFIG_JUMP_LABEL=y static_key_slow_inc_cpuslocked()
   still may turn the refcounter negative as (v + 1) may overflow.

key->enabled is indeed a ref-counter as it's documented in multiple
places: top comment in jump_label.h, Documentation/staging/static-keys.rst,
etc.

As -1 is reserved for static key that's in process of being enabled,
functions would break with negative key->enabled refcount:
- for CONFIG_JUMP_LABEL=n negative return of static_key_count()
  breaks static_key_false(), static_key_true()
- the ref counter may become 0 from negative side by too many
  static_key_slow_inc() calls and lead to use-after-free issues.

These flaws result in that some users have to introduce an additional
mutex and prevent the reference counter from overflowing themselves,
see bpf_enable_runtime_stats() checking the counter against INT_MAX / 2.

Prevent the reference counter overflow by checking if (v + 1) > 0.
Change functions API to return whether the increment was successful.

Signed-off-by: Dmitry Safonov 
Acked-by: Jakub Kicinski 
Acked-by: Peter Zijlstra (Intel) 
Signed-off-by: Jakub Kicinski

jump_label: Use atomic_try_cmpxchg() in static_key_slow_inc_cpuslocked()

2022-10-27T08:35:41+00:00

Use atomic_try_cmpxchg() instead of atomic_cmpxchg (*ptr, old, new) ==
old in static_key_slow_inc_cpuslocked().  x86 CMPXCHG instruction
returns success in ZF flag, so this change saves a compare after
cmpxchg (and related move instruction in front of cmpxchg).

Also, atomic_try_cmpxchg() implicitly assigns old *ptr value to "old" when
cmpxchg fails, enabling further code simplifications.

No functional change intended.

Signed-off-by: Uros Bizjak 
Signed-off-by: Peter Zijlstra (Intel) 
Link: https://lkml.kernel.org/r/20221019140850.3395-1-ubizjak@gmail.com

jump_label: make initial NOP patching the special case

2022-06-24T07:48:55+00:00

Instead of defaulting to patching NOP opcodes at init time, and leaving
it to the architectures to override this if this is not needed, switch
to a model where doing nothing is the default. This is the common case
by far, as only MIPS requires NOP patching at init time. On all other
architectures, the correct encodings are emitted by the compiler and so
no initial patching is needed.

Signed-off-by: Ard Biesheuvel 
Signed-off-by: Peter Zijlstra (Intel) 
Acked-by: Mark Rutland 
Acked-by: Peter Zijlstra (Intel) 
Link: https://lore.kernel.org/r/20220615154142.1574619-4-ardb@kernel.org

jump_label: mips: move module NOP patching into arch code

2022-06-24T07:48:55+00:00

MIPS is the only remaining architecture that needs to patch jump label
NOP encodings to initialize them at load time. So let's move the module
patching part of that from generic code into arch/mips, and drop it from
the others.

Signed-off-by: Ard Biesheuvel 
Signed-off-by: Peter Zijlstra (Intel) 
Link: https://lore.kernel.org/r/20220615154142.1574619-3-ardb@kernel.org