[Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Sam Sun]

Dear developers and maintainers,

We encountered a kernel bug while using our modified
syzkaller. It was tested against the latest upstream kernel.
Kernel crash log is listed below.

```
[   82.310798][ T8020] ------------[ cut here ]------------
[   82.311236][ T8020] kernel BUG at arch/x86/kernel/jump_label.c:73!
[   82.311761][ T8020] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI
[   82.312331][ T8020] CPU: 0 PID: 8020 Comm: static_key_slow Not
tainted 6.10.0-rc2-00366-g771ed66105de 3
[   82.313044][ T8020] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.13.0-1ubuntu1.1 04/04
[   82.313778][ T8020] RIP: 0010:__jump_label_patch+0x456/0x480
[   82.314223][ T8020] Code: e8 af a1 5d 00 48 c7 c7 40 01 86 8b 48 8b
0c 24 48 89 ce 48 89 ca 4d 89 e8 4f
[   82.315745][ T8020] RSP: 0018:ffffc9000f9cf980 EFLAGS: 00010292
[   82.316204][ T8020] RAX: 000000000000008b RBX: 0000000000000085
RCX: 218564eba7c06800
[   82.316813][ T8020] RDX: 0000000000000000 RSI: 0000000080000000
RDI: 0000000000000000
[   82.317396][ T8020] RBP: ffffc9000f9cfac0 R08: ffffffff817176dc
R09: 1ffff92001f39ed0
[   82.317987][ T8020] R10: dffffc0000000000 R11: fffff52001f39ed1
R12: 0000000000000001
[   82.318548][ T8020] R13: ffffffff8b862901 R14: ffffffff90fbe8c0
R15: ffffffff8b862901
[   82.319116][ T8020] FS:  00007f04610bc540(0000)
GS:ffff88802c800000(0000) knlGS:0000000000000000
[   82.319808][ T8020] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   82.320308][ T8020] CR2: 00007f0460fd9650 CR3: 000000004a16a000
CR4: 0000000000750ef0
[   82.320883][ T8020] DR0: 0000000000000000 DR1: 0000000000000000
DR2: 0000000000000000
[   82.321460][ T8020] DR3: 0000000000000000 DR6: 00000000fffe0ff0
DR7: 0000000000000400
[   82.322004][ T8020] PKRU: 55555554
[   82.322269][ T8020] Call Trace:
[   82.322515][ T8020]  <TASK>
[   82.322729][ T8020]  ? __die_body+0x88/0xe0
[   82.323043][ T8020]  ? die+0xcf/0x110
[   82.323322][ T8020]  ? do_trap+0x155/0x3a0
[   82.323625][ T8020]  ? __jump_label_patch+0x456/0x480
[   82.324009][ T8020]  ? do_error_trap+0x1dc/0x2a0
[   82.324371][ T8020]  ? __jump_label_patch+0x456/0x480
[   82.324774][ T8020]  ? do_int3+0x50/0x50
[   82.325088][ T8020]  ? handle_invalid_op+0x34/0x40
[   82.325445][ T8020]  ? __jump_label_patch+0x456/0x480
[   82.325815][ T8020]  ? exc_invalid_op+0x34/0x50
[   82.326156][ T8020]  ? asm_exc_invalid_op+0x1a/0x20
[   82.326533][ T8020]  ? __wake_up_klogd+0xcc/0x100
[   82.326879][ T8020]  ? __jump_label_patch+0x456/0x480
[   82.327249][ T8020]  ? cr4_update_pce+0xf/0x80
[   82.327601][ T8020]  ? arch_jump_label_transform_queue+0xf0/0xf0
[   82.328047][ T8020]  ? cr4_update_pce+0xf/0x80
[   82.328380][ T8020]  ? cr4_update_pce+0x1e/0x80
[   82.328717][ T8020]  ? cr4_update_pce+0x11/0x80
[   82.329052][ T8020]  ? read_lock_is_recursive+0x20/0x20
[   82.329466][ T8020]  ? __static_key_slow_dec_cpuslocked+0xb0/0x140
[   82.329966][ T8020]  ? text_poke_queue+0x46/0x180
[   82.330329][ T8020]  arch_jump_label_transform_queue+0x63/0xf0
[   82.330742][ T8020]  __jump_label_update+0x18b/0x3b0
[   82.331101][ T8020]  __static_key_slow_dec_cpuslocked+0xe9/0x140
[   82.331519][ T8020]  static_key_slow_dec+0x50/0xa0
[   82.331873][ T8020]  set_attr_rdpmc+0x193/0x270
[   82.332179][ T8020]  ? get_attr_rdpmc+0x30/0x30
[   82.332511][ T8020]  ? sysfs_kf_write+0x18d/0x2b0
[   82.332832][ T8020]  ? sysfs_kf_read+0x370/0x370
[   82.333159][ T8020]  kernfs_fop_write_iter+0x3ab/0x500
[   82.333531][ T8020]  vfs_write+0xa8b/0xd00
[   82.333812][ T8020]  ? kernfs_fop_read_iter+0x640/0x640
[   82.334234][ T8020]  ? kernel_write+0x330/0x330
[   82.334598][ T8020]  ? do_sys_openat2+0x16b/0x1c0
[   82.334949][ T8020]  ? do_sys_open+0x230/0x230
[   82.335310][ T8020]  ? __up_read+0x2bb/0x6a0
[   82.335633][ T8020]  ksys_write+0x17b/0x2a0
[   82.335966][ T8020]  ? __ia32_sys_read+0x90/0x90
[   82.336333][ T8020]  ? do_syscall_64+0xa5/0x230
[   82.336656][ T8020]  do_syscall_64+0xe2/0x230
[   82.336983][ T8020]  ? clear_bhb_loop+0x25/0x80
[   82.337289][ T8020]  entry_SYSCALL_64_after_hwframe+0x67/0x6f
[   82.337692][ T8020] RIP: 0033:0x7f0460fd2473
[   82.337987][ T8020] Code: 8b 15 21 2a 0e 00 f7 d8 64 89 02 48 c7 c0
ff ff ff ff eb b7 0f 1f 00 64 8b 08
[   82.339230][ T8020] RSP: 002b:00007ffc46fb3058 EFLAGS: 00000246
ORIG_RAX: 0000000000000001
[   82.339804][ T8020] RAX: ffffffffffffffda RBX: 0000000000000000
RCX: 00007f0460fd2473
[   82.340331][ T8020] RDX: 0000000000000002 RSI: 00007ffc46fb3090
RDI: 0000000000000004
[   82.340852][ T8020] RBP: 00007ffc46fb3cb0 R08: 000000000000000f
R09: 0072736d2f232f75
[   82.341353][ T8020] R10: 0000000000000000 R11: 0000000000000246
R12: 00005615dc1dd1c0
[   82.341853][ T8020] R13: 0000000000000000 R14: 0000000000000000
R15: 0000000000000000
[   82.342364][ T8020]  </TASK>
[   82.342554][ T8020] Modules linked in:
[   82.342875][ T8020] ---[ end trace 0000000000000000 ]---
[   82.343230][ T8020] RIP: 0010:__jump_label_patch+0x456/0x480
[   82.343668][ T8020] Code: e8 af a1 5d 00 48 c7 c7 40 01 86 8b 48 8b
0c 24 48 89 ce 48 89 ca 4d 89 e8 4f
[   82.345750][ T8020] RSP: 0018:ffffc9000f9cf980 EFLAGS: 00010292
[   82.346161][ T8020] RAX: 000000000000008b RBX: 0000000000000085
RCX: 218564eba7c06800
[   82.346657][ T8020] RDX: 0000000000000000 RSI: 0000000080000000
RDI: 0000000000000000
[   82.347391][ T8020] RBP: ffffc9000f9cfac0 R08: ffffffff817176dc
R09: 1ffff92001f39ed0
[   82.347901][ T8020] R10: dffffc0000000000 R11: fffff52001f39ed1
R12: 0000000000000001
[   82.348652][ T8020] R13: ffffffff8b862901 R14: ffffffff90fbe8c0
R15: ffffffff8b862901
[   82.349173][ T8020] FS:  00007f04610bc540(0000)
GS:ffff88802c800000(0000) knlGS:0000000000000000
[   82.349784][ T8020] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   82.350196][ T8020] CR2: 00007f96d80450b8 CR3: 000000004a16a000
CR4: 0000000000750ef0
[   82.350717][ T8020] DR0: 0000000000000000 DR1: 0000000000000000
DR2: 0000000000000000
[   82.351243][ T8020] DR3: 0000000000000000 DR6: 00000000fffe0ff0
DR7: 0000000000000400
[   82.351769][ T8020] PKRU: 55555554
[   82.352011][ T8020] Kernel panic - not syncing: Fatal exception
[   82.352665][ T8020] Kernel Offset: disabled
[   82.352988][ T8020] Rebooting in 86400 seconds..
```

We have a C repro which can trigger this bug in latest upstream kernel
commit, listed below:

```
#define _GNU_SOURCE

#include <dirent.h>
#include <endian.h>
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/prctl.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>

static unsigned long long procid;

static void sleep_ms(uint64_t ms)
{
  usleep(ms * 1000);
}

static uint64_t current_time_ms(void)
{
  struct timespec ts;
  if (clock_gettime(CLOCK_MONOTONIC, &ts))
    exit(1);
  return (uint64_t)ts.tv_sec * 1000 + (uint64_t)ts.tv_nsec / 1000000;
}

static bool write_file(const char* file, const char* what, ...)
{
  char buf[1024];
  va_list args;
  va_start(args, what);
  vsnprintf(buf, sizeof(buf), what, args);
  va_end(args);
  buf[sizeof(buf) - 1] = 0;
  int len = strlen(buf);
  int fd = open(file, O_WRONLY | O_CLOEXEC);
  if (fd == -1)
    return false;
  if (write(fd, buf, len) != len) {
    int err = errno;
    close(fd);
    errno = err;
    return false;
  }
  close(fd);
  return true;
}

static long syz_mod_dev(volatile long a0, volatile long a1, volatile long a2,
                        volatile long a3, volatile long a4)
{
  int fd, sysfd;
  char buf[1024], sysbuf[1024], input[1024];
  char* hash;
  char dev_num;
  dev_num = 0;
  strncpy(buf, (char*)a0, sizeof(buf) - 1);
  buf[sizeof(buf) - 1] = 0;
  while ((hash = strchr(buf, '#'))) {
    *hash = '0' + (char)(a1 % 10);
    a1 /= 10;
    if (dev_num == 0)
      dev_num = *hash;
  }
  fd = open(buf, a2, 0);
  strncpy(sysbuf, (char*)a3, sizeof(sysbuf) - 1);
  sysbuf[sizeof(sysbuf) - 1] = 0;
  if ((hash = strchr(sysbuf, '#'))) {
    *hash = dev_num;
    while ((hash = strchr(sysbuf, '#'))) {
      *hash = '0' + (char)(a1 % 10);
      a1 /= 10;
    }
  }
  sysfd = open(sysbuf, O_RDWR, 0);
  strncpy(input, (char*)a4, sizeof(input) - 1);
  input[sizeof(input) - 1] = 0;
  hash = strchr(input, '\0');
  sysfd = write(sysfd, input, hash - input + 1);
  return fd;
}

static void kill_and_wait(int pid, int* status)
{
  kill(-pid, SIGKILL);
  kill(pid, SIGKILL);
  for (int i = 0; i < 100; i++) {
    if (waitpid(-1, status, WNOHANG | __WALL) == pid)
      return;
    usleep(1000);
  }
  DIR* dir = opendir("/sys/fs/fuse/connections");
  if (dir) {
    for (;;) {
      struct dirent* ent = readdir(dir);
      if (!ent)
        break;
      if (strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0)
        continue;
      char abort[300];
      snprintf(abort, sizeof(abort), "/sys/fs/fuse/connections/%s/abort",
               ent->d_name);
      int fd = open(abort, O_WRONLY);
      if (fd == -1) {
        continue;
      }
      if (write(fd, abort, 1) < 0) {
      }
      close(fd);
    }
    closedir(dir);
  } else {
  }
  while (waitpid(-1, status, __WALL) != pid) {
  }
}

static void setup_test()
{
  prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
  setpgrp();
  write_file("/proc/self/oom_score_adj", "1000");
}

static void execute_one(void);

#define WAIT_FLAGS __WALL

static void loop(void)
{
  int iter = 0;
  for (;; iter++) {
    int pid = fork();
    if (pid < 0)
      exit(1);
    if (pid == 0) {
      setup_test();
      execute_one();
      exit(0);
    }
    int status = 0;
    uint64_t start = current_time_ms();
    for (;;) {
      if (waitpid(-1, &status, WNOHANG | WAIT_FLAGS) == pid)
        break;
      sleep_ms(1);
      if (current_time_ms() - start < 5000)
        continue;
      kill_and_wait(pid, &status);
      break;
    }
  }
}

void execute_one(void)
{
  memcpy((void*)0x20000000, "/dev/cpu/#/msr\000", 15);
  memcpy((void*)0x200000c0, "/sys/devices/cpu/rdpmc\000", 23);
  memcpy((void*)0x20000100, "0\000", 2);
  syz_mod_dev(
      /*dev=*/0x20000000, /*id=*/0x8001,
      /*flags=O_TRUNC|O_PATH|O_NONBLOCK|O_NOATIME|O_EXCL|O_DIRECTORY|0x442*/
      0x250ec2, /*file=*/0x200000c0, /*buf=*/0x20000100);
  memcpy((void*)0x20000080, "/dev/cpu/#/msr\000", 15);
  memcpy((void*)0x20000140, "/sys/devices/cpu/rdpmc\000", 23);
  memcpy((void*)0x20000180, "1\000", 2);
  syz_mod_dev(/*dev=*/0x20000080, /*id=*/0,
              /*flags=__O_TMPFILE|O_APPEND*/ 0x400400, /*file=*/0x20000140,
              /*buf=*/0x20000180);
}
int main(void)
{
  syscall(__NR_mmap, /*addr=*/0x1ffff000ul, /*len=*/0x1000ul, /*prot=*/0ul,
          /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
          /*offset=*/0ul);
  syscall(__NR_mmap, /*addr=*/0x20000000ul, /*len=*/0x1000000ul,
          /*prot=PROT_WRITE|PROT_READ|PROT_EXEC*/ 7ul,
          /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
          /*offset=*/0ul);
  syscall(__NR_mmap, /*addr=*/0x21000000ul, /*len=*/0x1000ul, /*prot=*/0ul,
          /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
          /*offset=*/0ul);
  for (procid = 0; procid < 4; procid++) {
    if (fork() == 0) {
      loop();
    }
  }
  sleep(1000000);
  return 0;
}
```

If you have any questions, please contact us.

Reported by Yue Sun <samsun1006219@gmail.com>
Reported by xingwei lee <xrivendell7@gmail.com>

Best Regards,
Yue

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Steven Rostedt]

On Sun, 9 Jun 2024 14:33:01 +0800
Sam Sun <samsun1006219@gmail.com> wrote:

> Dear developers and maintainers,
> 
> We encountered a kernel bug while using our modified
> syzkaller. It was tested against the latest upstream kernel.
> Kernel crash log is listed below.
> 
> ```
> [   82.310798][ T8020] ------------[ cut here ]------------
> [   82.311236][ T8020] kernel BUG at arch/x86/kernel/jump_label.c:73!

This is not a bug with jump labels. It's a bug with whatever is using jump
labels. Looks like something tried to modify a jump label that no longer
exists.

> [   82.311761][ T8020] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI
> [   82.312331][ T8020] CPU: 0 PID: 8020 Comm: static_key_slow Not
> tainted 6.10.0-rc2-00366-g771ed66105de 3
> [   82.313044][ T8020] Hardware name: QEMU Standard PC (i440FX + PIIX,
> 1996), BIOS 1.13.0-1ubuntu1.1 04/04
> [   82.313778][ T8020] RIP: 0010:__jump_label_patch+0x456/0x480
> [   82.314223][ T8020] Code: e8 af a1 5d 00 48 c7 c7 40 01 86 8b 48 8b
> 0c 24 48 89 ce 48 89 ca 4d 89 e8 4f
> [   82.315745][ T8020] RSP: 0018:ffffc9000f9cf980 EFLAGS: 00010292
> [   82.316204][ T8020] RAX: 000000000000008b RBX: 0000000000000085
> RCX: 218564eba7c06800
> [   82.316813][ T8020] RDX: 0000000000000000 RSI: 0000000080000000
> RDI: 0000000000000000
> [   82.317396][ T8020] RBP: ffffc9000f9cfac0 R08: ffffffff817176dc
> R09: 1ffff92001f39ed0
> [   82.317987][ T8020] R10: dffffc0000000000 R11: fffff52001f39ed1
> R12: 0000000000000001
> [   82.318548][ T8020] R13: ffffffff8b862901 R14: ffffffff90fbe8c0
> R15: ffffffff8b862901
> [   82.319116][ T8020] FS:  00007f04610bc540(0000)
> GS:ffff88802c800000(0000) knlGS:0000000000000000
> [   82.319808][ T8020] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   82.320308][ T8020] CR2: 00007f0460fd9650 CR3: 000000004a16a000
> CR4: 0000000000750ef0
> [   82.320883][ T8020] DR0: 0000000000000000 DR1: 0000000000000000
> DR2: 0000000000000000
> [   82.321460][ T8020] DR3: 0000000000000000 DR6: 00000000fffe0ff0
> DR7: 0000000000000400
> [   82.322004][ T8020] PKRU: 55555554
> [   82.322269][ T8020] Call Trace:
> [   82.322515][ T8020]  <TASK>
> [   82.322729][ T8020]  ? __die_body+0x88/0xe0
> [   82.323043][ T8020]  ? die+0xcf/0x110
> [   82.323322][ T8020]  ? do_trap+0x155/0x3a0
> [   82.323625][ T8020]  ? __jump_label_patch+0x456/0x480
> [   82.324009][ T8020]  ? do_error_trap+0x1dc/0x2a0
> [   82.324371][ T8020]  ? __jump_label_patch+0x456/0x480
> [   82.324774][ T8020]  ? do_int3+0x50/0x50
> [   82.325088][ T8020]  ? handle_invalid_op+0x34/0x40
> [   82.325445][ T8020]  ? __jump_label_patch+0x456/0x480
> [   82.325815][ T8020]  ? exc_invalid_op+0x34/0x50
> [   82.326156][ T8020]  ? asm_exc_invalid_op+0x1a/0x20
> [   82.326533][ T8020]  ? __wake_up_klogd+0xcc/0x100
> [   82.326879][ T8020]  ? __jump_label_patch+0x456/0x480
> [   82.327249][ T8020]  ? cr4_update_pce+0xf/0x80
> [   82.327601][ T8020]  ? arch_jump_label_transform_queue+0xf0/0xf0
> [   82.328047][ T8020]  ? cr4_update_pce+0xf/0x80
> [   82.328380][ T8020]  ? cr4_update_pce+0x1e/0x80
> [   82.328717][ T8020]  ? cr4_update_pce+0x11/0x80
> [   82.329052][ T8020]  ? read_lock_is_recursive+0x20/0x20
> [   82.329466][ T8020]  ? __static_key_slow_dec_cpuslocked+0xb0/0x140
> [   82.329966][ T8020]  ? text_poke_queue+0x46/0x180
> [   82.330329][ T8020]  arch_jump_label_transform_queue+0x63/0xf0
> [   82.330742][ T8020]  __jump_label_update+0x18b/0x3b0
> [   82.331101][ T8020]  __static_key_slow_dec_cpuslocked+0xe9/0x140
> [   82.331519][ T8020]  static_key_slow_dec+0x50/0xa0
> [   82.331873][ T8020]  set_attr_rdpmc+0x193/0x270
> [   82.332179][ T8020]  ? get_attr_rdpmc+0x30/0x30
> [   82.332511][ T8020]  ? sysfs_kf_write+0x18d/0x2b0
> [   82.332832][ T8020]  ? sysfs_kf_read+0x370/0x370
> [   82.333159][ T8020]  kernfs_fop_write_iter+0x3ab/0x500

So, something in kernfs modified a jump label location that was freed?

-- Steve


> [   82.333531][ T8020]  vfs_write+0xa8b/0xd00
> [   82.333812][ T8020]  ? kernfs_fop_read_iter+0x640/0x640
> [   82.334234][ T8020]  ? kernel_write+0x330/0x330
> [   82.334598][ T8020]  ? do_sys_openat2+0x16b/0x1c0
> [   82.334949][ T8020]  ? do_sys_open+0x230/0x230
> [   82.335310][ T8020]  ? __up_read+0x2bb/0x6a0
> [   82.335633][ T8020]  ksys_write+0x17b/0x2a0
> [   82.335966][ T8020]  ? __ia32_sys_read+0x90/0x90
> [   82.336333][ T8020]  ? do_syscall_64+0xa5/0x230
> [   82.336656][ T8020]  do_syscall_64+0xe2/0x230
> [   82.336983][ T8020]  ? clear_bhb_loop+0x25/0x80
> [   82.337289][ T8020]  entry_SYSCALL_64_after_hwframe+0x67/0x6f
> [   82.337692][ T8020] RIP: 0033:0x7f0460fd2473
> [   82.337987][ T8020] Code: 8b 15 21 2a 0e 00 f7 d8 64 89 02 48 c7 c0
> ff ff ff ff eb b7 0f 1f 00 64 8b 08
> [   82.339230][ T8020] RSP: 002b:00007ffc46fb3058 EFLAGS: 00000246
> ORIG_RAX: 0000000000000001
> [   82.339804][ T8020] RAX: ffffffffffffffda RBX: 0000000000000000
> RCX: 00007f0460fd2473
> [   82.340331][ T8020] RDX: 0000000000000002 RSI: 00007ffc46fb3090
> RDI: 0000000000000004
> [   82.340852][ T8020] RBP: 00007ffc46fb3cb0 R08: 000000000000000f
> R09: 0072736d2f232f75
> [   82.341353][ T8020] R10: 0000000000000000 R11: 0000000000000246
> R12: 00005615dc1dd1c0
> [   82.341853][ T8020] R13: 0000000000000000 R14: 0000000000000000
> R15: 0000000000000000
> [   82.342364][ T8020]  </TASK>
> [   82.342554][ T8020] Modules linked in:
> [   82.342875][ T8020] ---[ end trace 0000000000000000 ]---
> [   82.343230][ T8020] RIP: 0010:__jump_label_patch+0x456/0x480
> [   82.343668][ T8020] Code: e8 af a1 5d 00 48 c7 c7 40 01 86 8b 48 8b
> 0c 24 48 89 ce 48 89 ca 4d 89 e8 4f
> [   82.345750][ T8020] RSP: 0018:ffffc9000f9cf980 EFLAGS: 00010292
> [   82.346161][ T8020] RAX: 000000000000008b RBX: 0000000000000085
> RCX: 218564eba7c06800
> [   82.346657][ T8020] RDX: 0000000000000000 RSI: 0000000080000000
> RDI: 0000000000000000
> [   82.347391][ T8020] RBP: ffffc9000f9cfac0 R08: ffffffff817176dc
> R09: 1ffff92001f39ed0
> [   82.347901][ T8020] R10: dffffc0000000000 R11: fffff52001f39ed1
> R12: 0000000000000001
> [   82.348652][ T8020] R13: ffffffff8b862901 R14: ffffffff90fbe8c0
> R15: ffffffff8b862901
> [   82.349173][ T8020] FS:  00007f04610bc540(0000)
> GS:ffff88802c800000(0000) knlGS:0000000000000000
> [   82.349784][ T8020] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   82.350196][ T8020] CR2: 00007f96d80450b8 CR3: 000000004a16a000
> CR4: 0000000000750ef0
> [   82.350717][ T8020] DR0: 0000000000000000 DR1: 0000000000000000
> DR2: 0000000000000000
> [   82.351243][ T8020] DR3: 0000000000000000 DR6: 00000000fffe0ff0
> DR7: 0000000000000400
> [   82.351769][ T8020] PKRU: 55555554
> [   82.352011][ T8020] Kernel panic - not syncing: Fatal exception
> [   82.352665][ T8020] Kernel Offset: disabled
> [   82.352988][ T8020] Rebooting in 86400 seconds..
> ```
> 
> We have a C repro which can trigger this bug in latest upstream kernel
> commit, listed below:
> 
> ```
> #define _GNU_SOURCE
> 
> #include <dirent.h>
> #include <endian.h>
> #include <errno.h>
> #include <fcntl.h>
> #include <signal.h>
> #include <stdarg.h>
> #include <stdbool.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/prctl.h>
> #include <sys/stat.h>
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <time.h>
> #include <unistd.h>
> 
> static unsigned long long procid;
> 
> static void sleep_ms(uint64_t ms)
> {
>   usleep(ms * 1000);
> }
> 
> static uint64_t current_time_ms(void)
> {
>   struct timespec ts;
>   if (clock_gettime(CLOCK_MONOTONIC, &ts))
>     exit(1);
>   return (uint64_t)ts.tv_sec * 1000 + (uint64_t)ts.tv_nsec / 1000000;
> }
> 
> static bool write_file(const char* file, const char* what, ...)
> {
>   char buf[1024];
>   va_list args;
>   va_start(args, what);
>   vsnprintf(buf, sizeof(buf), what, args);
>   va_end(args);
>   buf[sizeof(buf) - 1] = 0;
>   int len = strlen(buf);
>   int fd = open(file, O_WRONLY | O_CLOEXEC);
>   if (fd == -1)
>     return false;
>   if (write(fd, buf, len) != len) {
>     int err = errno;
>     close(fd);
>     errno = err;
>     return false;
>   }
>   close(fd);
>   return true;
> }
> 
> static long syz_mod_dev(volatile long a0, volatile long a1, volatile long a2,
>                         volatile long a3, volatile long a4)
> {
>   int fd, sysfd;
>   char buf[1024], sysbuf[1024], input[1024];
>   char* hash;
>   char dev_num;
>   dev_num = 0;
>   strncpy(buf, (char*)a0, sizeof(buf) - 1);
>   buf[sizeof(buf) - 1] = 0;
>   while ((hash = strchr(buf, '#'))) {
>     *hash = '0' + (char)(a1 % 10);
>     a1 /= 10;
>     if (dev_num == 0)
>       dev_num = *hash;
>   }
>   fd = open(buf, a2, 0);
>   strncpy(sysbuf, (char*)a3, sizeof(sysbuf) - 1);
>   sysbuf[sizeof(sysbuf) - 1] = 0;
>   if ((hash = strchr(sysbuf, '#'))) {
>     *hash = dev_num;
>     while ((hash = strchr(sysbuf, '#'))) {
>       *hash = '0' + (char)(a1 % 10);
>       a1 /= 10;
>     }
>   }
>   sysfd = open(sysbuf, O_RDWR, 0);
>   strncpy(input, (char*)a4, sizeof(input) - 1);
>   input[sizeof(input) - 1] = 0;
>   hash = strchr(input, '\0');
>   sysfd = write(sysfd, input, hash - input + 1);
>   return fd;
> }
> 
> static void kill_and_wait(int pid, int* status)
> {
>   kill(-pid, SIGKILL);
>   kill(pid, SIGKILL);
>   for (int i = 0; i < 100; i++) {
>     if (waitpid(-1, status, WNOHANG | __WALL) == pid)
>       return;
>     usleep(1000);
>   }
>   DIR* dir = opendir("/sys/fs/fuse/connections");
>   if (dir) {
>     for (;;) {
>       struct dirent* ent = readdir(dir);
>       if (!ent)
>         break;
>       if (strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0)
>         continue;
>       char abort[300];
>       snprintf(abort, sizeof(abort), "/sys/fs/fuse/connections/%s/abort",
>                ent->d_name);
>       int fd = open(abort, O_WRONLY);
>       if (fd == -1) {
>         continue;
>       }
>       if (write(fd, abort, 1) < 0) {
>       }
>       close(fd);
>     }
>     closedir(dir);
>   } else {
>   }
>   while (waitpid(-1, status, __WALL) != pid) {
>   }
> }
> 
> static void setup_test()
> {
>   prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
>   setpgrp();
>   write_file("/proc/self/oom_score_adj", "1000");
> }
> 
> static void execute_one(void);
> 
> #define WAIT_FLAGS __WALL
> 
> static void loop(void)
> {
>   int iter = 0;
>   for (;; iter++) {
>     int pid = fork();
>     if (pid < 0)
>       exit(1);
>     if (pid == 0) {
>       setup_test();
>       execute_one();
>       exit(0);
>     }
>     int status = 0;
>     uint64_t start = current_time_ms();
>     for (;;) {
>       if (waitpid(-1, &status, WNOHANG | WAIT_FLAGS) == pid)
>         break;
>       sleep_ms(1);
>       if (current_time_ms() - start < 5000)
>         continue;
>       kill_and_wait(pid, &status);
>       break;
>     }
>   }
> }
> 
> void execute_one(void)
> {
>   memcpy((void*)0x20000000, "/dev/cpu/#/msr\000", 15);
>   memcpy((void*)0x200000c0, "/sys/devices/cpu/rdpmc\000", 23);
>   memcpy((void*)0x20000100, "0\000", 2);
>   syz_mod_dev(
>       /*dev=*/0x20000000, /*id=*/0x8001,
>       /*flags=O_TRUNC|O_PATH|O_NONBLOCK|O_NOATIME|O_EXCL|O_DIRECTORY|0x442*/
>       0x250ec2, /*file=*/0x200000c0, /*buf=*/0x20000100);
>   memcpy((void*)0x20000080, "/dev/cpu/#/msr\000", 15);
>   memcpy((void*)0x20000140, "/sys/devices/cpu/rdpmc\000", 23);
>   memcpy((void*)0x20000180, "1\000", 2);
>   syz_mod_dev(/*dev=*/0x20000080, /*id=*/0,
>               /*flags=__O_TMPFILE|O_APPEND*/ 0x400400, /*file=*/0x20000140,
>               /*buf=*/0x20000180);
> }
> int main(void)
> {
>   syscall(__NR_mmap, /*addr=*/0x1ffff000ul, /*len=*/0x1000ul, /*prot=*/0ul,
>           /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
>           /*offset=*/0ul);
>   syscall(__NR_mmap, /*addr=*/0x20000000ul, /*len=*/0x1000000ul,
>           /*prot=PROT_WRITE|PROT_READ|PROT_EXEC*/ 7ul,
>           /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
>           /*offset=*/0ul);
>   syscall(__NR_mmap, /*addr=*/0x21000000ul, /*len=*/0x1000ul, /*prot=*/0ul,
>           /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
>           /*offset=*/0ul);
>   for (procid = 0; procid < 4; procid++) {
>     if (fork() == 0) {
>       loop();
>     }
>   }
>   sleep(1000000);
>   return 0;
> }
> ```
> 
> If you have any questions, please contact us.
> 
> Reported by Yue Sun <samsun1006219@gmail.com>
> Reported by xingwei lee <xrivendell7@gmail.com>
> 
> Best Regards,
> Yue

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Thomas Gleixner]

On Sun, Jun 09 2024 at 09:04, Steven Rostedt wrote:
> On Sun, 9 Jun 2024 14:33:01 +0800
> Sam Sun <samsun1006219@gmail.com> wrote:
>> [   82.310798][ T8020] ------------[ cut here ]------------
>> [   82.311236][ T8020] kernel BUG at arch/x86/kernel/jump_label.c:73!
>
> This is not a bug with jump labels. It's a bug with whatever is using jump
> labels. Looks like something tried to modify a jump label that no longer
> exists.

The jump label exists.

>> [   82.331873][ T8020]  set_attr_rdpmc+0x193/0x270
>> [   82.332179][ T8020]  ? get_attr_rdpmc+0x30/0x30
>> [   82.332511][ T8020]  ? sysfs_kf_write+0x18d/0x2b0
>> [   82.332832][ T8020]  ? sysfs_kf_read+0x370/0x370
>> [   82.333159][ T8020]  kernfs_fop_write_iter+0x3ab/0x500
>
> So, something in kernfs modified a jump label location that was freed?

No. What happens is:

CPU 0                           	CPU 1

kernfs_fop_write_iter()			kernfs_fop_write_iter()
  set_attr_rdpmc()		  	  set_attr_rdpmc()
    arch_jump_label_transform_queue()       arch_jump_label_transform_queue()
     mutex_lock(text_mutex)                   mutex_lock(text_mutex)
     __jump_label_patch()
     text_poke_queue()
     mutex_unlokc(text_mutex)
                                              __jump_label_patch()

CPU 1 sees the original text and not the expected because CPU 0 did not
yet invoke arch_jump_label_transform_apply().

So clearly set_attr_rdpmc() lacks serialization, no?

Thanks,

        tglx

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Steven Rostedt]

On Sun, 09 Jun 2024 16:06:05 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Sun, Jun 09 2024 at 09:04, Steven Rostedt wrote:
> > On Sun, 9 Jun 2024 14:33:01 +0800
> > Sam Sun <samsun1006219@gmail.com> wrote:  
> >> [   82.310798][ T8020] ------------[ cut here ]------------
> >> [   82.311236][ T8020] kernel BUG at arch/x86/kernel/jump_label.c:73!  
> >
> > This is not a bug with jump labels. It's a bug with whatever is using jump
> > labels. Looks like something tried to modify a jump label that no longer
> > exists.  
> 
> The jump label exists.

Ah, I missed the set_attr_rdpmc() as something not with a "?" in front :-p

> 
> >> [   82.331873][ T8020]  set_attr_rdpmc+0x193/0x270
> >> [   82.332179][ T8020]  ? get_attr_rdpmc+0x30/0x30
> >> [   82.332511][ T8020]  ? sysfs_kf_write+0x18d/0x2b0
> >> [   82.332832][ T8020]  ? sysfs_kf_read+0x370/0x370
> >> [   82.333159][ T8020]  kernfs_fop_write_iter+0x3ab/0x500  
> >
> > So, something in kernfs modified a jump label location that was freed?  
> 
> No. What happens is:
> 
> CPU 0                           	CPU 1
> 
> kernfs_fop_write_iter()			kernfs_fop_write_iter()
>   set_attr_rdpmc()		  	  set_attr_rdpmc()
>     arch_jump_label_transform_queue()       arch_jump_label_transform_queue()
>      mutex_lock(text_mutex)                   mutex_lock(text_mutex)
>      __jump_label_patch()
>      text_poke_queue()
>      mutex_unlokc(text_mutex)
>                                               __jump_label_patch()
> 
> CPU 1 sees the original text and not the expected because CPU 0 did not
> yet invoke arch_jump_label_transform_apply().
> 
> So clearly set_attr_rdpmc() lacks serialization, no?
> 

Hmm, but should jump labels fail when that happens? Or should it catch
it, and not cause a BUG?

-- Steve

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Thomas Gleixner]

On Sun, Jun 09 2024 at 10:25, Steven Rostedt wrote:
> On Sun, 09 Jun 2024 16:06:05 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
>> CPU 0                           	CPU 1
>> 
>> kernfs_fop_write_iter()			kernfs_fop_write_iter()
>>   set_attr_rdpmc()		  	  set_attr_rdpmc()
>>     arch_jump_label_transform_queue()       arch_jump_label_transform_queue()
>>      mutex_lock(text_mutex)                   mutex_lock(text_mutex)
>>      __jump_label_patch()
>>      text_poke_queue()
>>      mutex_unlokc(text_mutex)
>>                                               __jump_label_patch()
>> 
>> CPU 1 sees the original text and not the expected because CPU 0 did not
>> yet invoke arch_jump_label_transform_apply().
>> 
>> So clearly set_attr_rdpmc() lacks serialization, no?
>> 
> Hmm, but should jump labels fail when that happens? Or should it catch
> it, and not cause a BUG?

Well the bug is there to detect inconsistency and that clearly works :)

But I clearly can't read, because the jump label operations are
serialized via jump_label_mutex. Hrm...

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Thomas Gleixner]

On Sun, Jun 09 2024 at 18:02, Thomas Gleixner wrote:
> On Sun, Jun 09 2024 at 10:25, Steven Rostedt wrote:
> Well the bug is there to detect inconsistency and that clearly works :)
>
> But I clearly can't read, because the jump label operations are
> serialized via jump_label_mutex. Hrm...

Ok. Now I found if for real. It's in the jump label core:

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

static_key_slow_try_dec() is buggy. It needs similar logic as
static_key_slow_try_inc() to work correctly.

It's not only the 0, key->enabled can be -1 when the other CPU is in the
slow path of enabling it.

I'll send a patch after testing it.

Thanks,

        tglx

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Thomas Gleixner]

On Sun, Jun 09 2024 at 18:56, Thomas Gleixner wrote:

> On Sun, Jun 09 2024 at 18:02, Thomas Gleixner wrote:
>> On Sun, Jun 09 2024 at 10:25, Steven Rostedt wrote:
>> Well the bug is there to detect inconsistency and that clearly works :)
>>
>> But I clearly can't read, because the jump label operations are
>> serialized via jump_label_mutex. Hrm...
>
> Ok. Now I found if for real. It's in the jump label core:
>
> 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
>
> static_key_slow_try_dec() is buggy. It needs similar logic as
> static_key_slow_try_inc() to work correctly.
>
> It's not only the 0, key->enabled can be -1 when the other CPU is in the
> slow path of enabling it.
>
> I'll send a patch after testing it.

That's fixable, but it does not cure all of it.

set_attr_rdpmc() really needs serialization otherwise it can end up with
unbalanced operations.

CPU0                                    CPU1

x86_pmu.attr_rdpmc == 0

if (val != x86_pmu.attr_rdpmc) {
   if (val == 0)
   	...
   else if (x86_pmu.attr_rdpmc == 0)
     static_branch_dec(&rdpmc_never_available_key);

                                        if (val != x86_pmu.attr_rdpmc) {
                                           if (val == 0)
                                              	...
                                            else if (x86_pmu.attr_rdpmc == 0)
                               FAIL --->        static_branch_dec(&rdpmc_never_available_key);

No?

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Peter Zijlstra]

On Sun, Jun 09, 2024 at 06:56:14PM +0200, Thomas Gleixner wrote:

> Ok. Now I found if for real. It's in the jump label core:
> 
> 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
> 
> static_key_slow_try_dec() is buggy. It needs similar logic as
> static_key_slow_try_inc() to work correctly.
> 
> It's not only the 0, key->enabled can be -1 when the other CPU is in the
> slow path of enabling it.

Well, the -1 thing is in the 0->1 path, that is, the very first enabler.

That *should* not race with a disabler. If it does, there is external
confusion. (As I think the follow up email shows..)

Re: [Linux kernel bug] WARNING in static_key_slow_inc_cpuslocked

[Thomas Gleixner]

On Mon, Jun 10 2024 at 08:46, Peter Zijlstra wrote:
> On Sun, Jun 09, 2024 at 06:56:14PM +0200, Thomas Gleixner wrote:
>
>> Ok. Now I found if for real. It's in the jump label core:
>> 
>> 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
>> 
>> static_key_slow_try_dec() is buggy. It needs similar logic as
>> static_key_slow_try_inc() to work correctly.
>> 
>> It's not only the 0, key->enabled can be -1 when the other CPU is in the
>> slow path of enabling it.
>
> Well, the -1 thing is in the 0->1 path, that is, the very first enabler.
>
> That *should* not race with a disabler. If it does, there is external
> confusion. (As I think the follow up email shows..)

Right, but all of this is too fragile. Let me send out those patches.

[patch 0/4] perf/x86, jump_label: Cure serialization issues

[Thomas Gleixner]

Yue and Xingwei reported a syzkaller crash related to the rdpmc attribute
file and jump labels. It turns out to be lack of serialization in the
attribute write path and non-robustness on the jump label side.

The following series addresses these issues.

Thanks,

	tglx
---
 arch/x86/events/core.c |    3 ++
 kernel/jump_label.c    |   67 ++++++++++++++++++++++++++++---------------------
 2 files changed, 42 insertions(+), 28 deletions(-)

[patch 1/4] perf/x86: Serialize set_attr_rdpmc()

[Thomas Gleixner]

Yue and Xingwei reported a jump label failure. It's caused by the lack of
serialization in set_attr_rdpmc():

CPU0                           CPU1

Assume: x86_pmu.attr_rdpmc == 0

if (val != x86_pmu.attr_rdpmc) {
  if (val == 0)
    ...
  else if (x86_pmu.attr_rdpmc == 0)
    static_branch_dec(&rdpmc_never_available_key);

				if (val != x86_pmu.attr_rdpmc) {
				   if (val == 0)
				      ...
				   else if (x86_pmu.attr_rdpmc == 0)
     FAIL, due to imbalance --->      static_branch_dec(&rdpmc_never_available_key);

The reported BUG() is a consequence of the above and of another bug in the
jump label core code. The core code needs a separate fix, but that cannot
prevent the imbalance problem caused by set_attr_rdpmc().

Prevent this by serializing set_attr_rdpmc() locally.

Fixes: a66734297f78 ("perf/x86: Add /sys/devices/cpu/rdpmc=2 to allow rdpmc for all tasks")
Reported-by: Yue Sun <samsun1006219@gmail.com>
Reported-by: Xingwei Lee <xrivendell7@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Closes: https://lore.kernel.org/r/CAEkJfYNzfW1vG=ZTMdz_Weoo=RXY1NDunbxnDaLyj8R4kEoE_w@mail.gmail.com
---
 arch/x86/events/core.c |    3 +++
 1 file changed, 3 insertions(+)

--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2547,6 +2547,7 @@ static ssize_t set_attr_rdpmc(struct dev
 			      struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
+	static DEFINE_MUTEX(rdpmc_mutex);
 	unsigned long val;
 	ssize_t ret;
 
@@ -2560,6 +2561,8 @@ static ssize_t set_attr_rdpmc(struct dev
 	if (x86_pmu.attr_rdpmc_broken)
 		return -ENOTSUPP;
 
+	guard(mutex)(&rdpmc_mutex);
+
 	if (val != x86_pmu.attr_rdpmc) {
 		/*
 		 * Changing into or out of never available or always available,

[patch 2/4] jump_label: Fix concurrency issues in static_key_slow_dec()

[Thomas Gleixner]

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().

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>
---
 kernel/jump_label.c |   38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -131,7 +131,7 @@ bool static_key_fast_inc_not_disabled(st
 	STATIC_KEY_CHECK_USE(key);
 	/*
 	 * Negative key->enabled has a special meaning: it sends
-	 * static_key_slow_inc() down the slow path, and it is non-zero
+	 * static_key_slow_inc/dec() down the slow path, and it is non-zero
 	 * so it counts as "enabled" in jump_label_update().  Note that
 	 * atomic_inc_unless_negative() checks >= 0, so roll our own.
 	 */
@@ -150,7 +150,7 @@ bool static_key_slow_inc_cpuslocked(stru
 	lockdep_assert_cpus_held();
 
 	/*
-	 * Careful if we get concurrent static_key_slow_inc() calls;
+	 * Careful if we get concurrent static_key_slow_inc/dec() calls;
 	 * later calls must wait for the first one to _finish_ the
 	 * jump_label_update() process.  At the same time, however,
 	 * the jump_label_update() call below wants to see
@@ -247,20 +247,25 @@ EXPORT_SYMBOL_GPL(static_key_disable);
 
 static bool static_key_slow_try_dec(struct static_key *key)
 {
-	int val;
-
-	val = atomic_fetch_add_unless(&key->enabled, -1, 1);
-	if (val == 1)
-		return false;
+	int v;
 
 	/*
-	 * The negative count check is valid even when a negative
-	 * key->enabled is in use by static_key_slow_inc(); a
-	 * __static_key_slow_dec() before the first static_key_slow_inc()
-	 * returns is unbalanced, because all other static_key_slow_inc()
-	 * instances block while the update is in progress.
+	 * Go into the slow path if key::enabled is less than or equal than
+	 * one. One is valid to shut down the key, anything less than one
+	 * is an imbalance, which is handled at the call site.
+	 *
+	 * That includes the special case of '-1' which is set in
+	 * static_key_slow_inc_cpuslocked(), but that's harmless as it is
+	 * fully serialized in the slow path below. By the time this task
+	 * acquires the jump label lock the value is back to one and the
+	 * retry under the lock must succeed.
 	 */
-	WARN(val < 0, "jump label: negative count!\n");
+	v = atomic_read(&key->enabled);
+	do {
+		if (v <= 1)
+			return false;
+	} while (!likely(atomic_try_cmpxchg(&key->enabled, &v, v - 1)));
+
 	return true;
 }
 
@@ -271,10 +276,11 @@ static void __static_key_slow_dec_cpuslo
 	if (static_key_slow_try_dec(key))
 		return;
 
-	jump_label_lock();
-	if (atomic_dec_and_test(&key->enabled))
+	guard(mutex)(&jump_label_mutex);
+	if (atomic_cmpxchg(&key->enabled, 1, 0))
 		jump_label_update(key);
-	jump_label_unlock();
+	else
+		WARN_ON_ONCE(!static_key_slow_try_dec(key));
 }
 
 static void __static_key_slow_dec(struct static_key *key)

[patch 3/4] jump_label: Clarify condition in static_key_fast_inc_not_disabled()

[Thomas Gleixner]

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 <tglx@linutronix.de>
---
 kernel/jump_label.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -132,12 +132,15 @@ bool static_key_fast_inc_not_disabled(st
 	/*
 	 * Negative key->enabled has a special meaning: it sends
 	 * static_key_slow_inc/dec() down the slow path, and it is non-zero
-	 * so it counts as "enabled" in jump_label_update().  Note that
-	 * atomic_inc_unless_negative() checks >= 0, so roll our own.
+	 * so it counts as "enabled" in jump_label_update().
+	 *
+	 * The INT_MAX overflow condition is either used by the networking
+	 * code to reset or detected in the slow path of
+	 * static_key_slow_inc_cpuslocked().
 	 */
 	v = atomic_read(&key->enabled);
 	do {
-		if (v <= 0 || (v + 1) < 0)
+		if (v <= 0 || v == INT_MAX)
 			return false;
 	} while (!likely(atomic_try_cmpxchg(&key->enabled, &v, v + 1)));
 

[patch 4/4] jump_label: Simplify and clarify static_key_fast_inc_cpus_locked()

[Thomas Gleixner]

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

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/jump_label.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -162,22 +162,24 @@ bool static_key_slow_inc_cpuslocked(stru
 	if (static_key_fast_inc_not_disabled(key))
 		return true;
 
-	jump_label_lock();
-	if (atomic_read(&key->enabled) == 0) {
-		atomic_set(&key->enabled, -1);
+	guard(mutex)(&jump_label_mutex);
+	/* Try to mark it as 'enabling in progress. */
+	if (!atomic_cmpxchg(&key->enabled, 0, -1)) {
 		jump_label_update(key);
 		/*
-		 * Ensure that if the above cmpxchg loop observes our positive
-		 * value, it must also observe all the text changes.
+		 * Ensure that when static_key_fast_inc_not_disabled() or
+		 * static_key_slow_try_dec() observe the positive value,
+		 * they must also observe all the text changes.
 		 */
 		atomic_set_release(&key->enabled, 1);
 	} else {
-		if (WARN_ON_ONCE(!static_key_fast_inc_not_disabled(key))) {
-			jump_label_unlock();
+		/*
+		 * While holding the mutex this should never observe
+		 * anything else than a value >= 1 and succeed
+		 */
+		if (WARN_ON_ONCE(!static_key_fast_inc_not_disabled(key)))
 			return false;
-		}
 	}
-	jump_label_unlock();
 	return true;
 }
 

Re: [patch 2/4] jump_label: Fix concurrency issues in static_key_slow_dec()

[Peter Zijlstra]

On Mon, Jun 10, 2024 at 02:46:36PM +0200, Thomas Gleixner wrote:

> @@ -247,20 +247,25 @@ EXPORT_SYMBOL_GPL(static_key_disable);
>  
>  static bool static_key_slow_try_dec(struct static_key *key)
>  {
> +	int v;
>  
>  	/*
> +	 * Go into the slow path if key::enabled is less than or equal than
> +	 * one. One is valid to shut down the key, anything less than one
> +	 * is an imbalance, which is handled at the call site.
> +	 *
> +	 * That includes the special case of '-1' which is set in
> +	 * static_key_slow_inc_cpuslocked(), but that's harmless as it is
> +	 * fully serialized in the slow path below. By the time this task
> +	 * acquires the jump label lock the value is back to one and the
> +	 * retry under the lock must succeed.

Harmless yes, but it really should not happen to begin with. If this
happens it means someone wants to disable a key that is in the middle of
getting enabled for the first time.

I'm tempted to want a WARN here instead. Hmm?

>  	 */
> +	v = atomic_read(&key->enabled);
> +	do {
> +		if (v <= 1)
> +			return false;
> +	} while (!likely(atomic_try_cmpxchg(&key->enabled, &v, v - 1)));
> +
>  	return true;
>  }

Re: [patch 2/4] jump_label: Fix concurrency issues in static_key_slow_dec()

[Thomas Gleixner]

On Mon, Jun 10 2024 at 19:57, Peter Zijlstra wrote:
> On Mon, Jun 10, 2024 at 02:46:36PM +0200, Thomas Gleixner wrote:
>
>> @@ -247,20 +247,25 @@ EXPORT_SYMBOL_GPL(static_key_disable);
>>  
>>  static bool static_key_slow_try_dec(struct static_key *key)
>>  {
>> +	int v;
>>  
>>  	/*
>> +	 * Go into the slow path if key::enabled is less than or equal than
>> +	 * one. One is valid to shut down the key, anything less than one
>> +	 * is an imbalance, which is handled at the call site.
>> +	 *
>> +	 * That includes the special case of '-1' which is set in
>> +	 * static_key_slow_inc_cpuslocked(), but that's harmless as it is
>> +	 * fully serialized in the slow path below. By the time this task
>> +	 * acquires the jump label lock the value is back to one and the
>> +	 * retry under the lock must succeed.
>
> Harmless yes, but it really should not happen to begin with. If this
> happens it means someone wants to disable a key that is in the middle of
> getting enabled for the first time.
>
> I'm tempted to want a WARN here instead. Hmm?

No strong opinion

Re: [patch 4/4] jump_label: Simplify and clarify static_key_fast_inc_cpus_locked()

[Uros Bizjak]

> Make the code more obvious and add proper comments to avoid future head
> scratching.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>   kernel/jump_label.c |   20 +++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
> 
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -162,22 +162,24 @@ bool static_key_slow_inc_cpuslocked(stru
>   	if (static_key_fast_inc_not_disabled(key))
>   		return true;
>   
> -	jump_label_lock();
> -	if (atomic_read(&key->enabled) == 0) {
> -		atomic_set(&key->enabled, -1);
> +	guard(mutex)(&jump_label_mutex);
> +	/* Try to mark it as 'enabling in progress. */

Missing closing quotation mark above.

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

This can be:

int tmp = 0;
if (atomic_try_cmpxchg(&key->enabled, &tmp, -1)) {
...

and will result in more optimal code and will IMO also be more readable 
because it is clear that the code is executed when cmpxchg succeeds.

(BTW: The atomic_read()/atomic_set() pair can also be converted to 
cmpxchg in static_key_enable_cpuslocked().)

Uros.

[tip: locking/core] jump_label: Simplify and clarify static_key_fast_inc_cpus_locked()

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     9bc2ff871f00437ad2f10c1eceff51aaa72b478f
Gitweb:        https://git.kernel.org/tip/9bc2ff871f00437ad2f10c1eceff51aaa72b478f
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Mon, 10 Jun 2024 14:46:39 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 11 Jun 2024 11:25:24 +02:00

jump_label: Simplify and clarify static_key_fast_inc_cpus_locked()

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

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20240610124406.548322963@linutronix.de
---
 kernel/jump_label.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 4d06ec2..4ad5ed8 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -162,22 +162,24 @@ bool static_key_slow_inc_cpuslocked(struct static_key *key)
 	if (static_key_fast_inc_not_disabled(key))
 		return true;
 
-	jump_label_lock();
-	if (atomic_read(&key->enabled) == 0) {
-		atomic_set(&key->enabled, -1);
+	guard(mutex)(&jump_label_mutex);
+	/* Try to mark it as 'enabling in progress. */
+	if (!atomic_cmpxchg(&key->enabled, 0, -1)) {
 		jump_label_update(key);
 		/*
-		 * Ensure that if the above cmpxchg loop observes our positive
-		 * value, it must also observe all the text changes.
+		 * Ensure that when static_key_fast_inc_not_disabled() or
+		 * static_key_slow_try_dec() observe the positive value,
+		 * they must also observe all the text changes.
 		 */
 		atomic_set_release(&key->enabled, 1);
 	} else {
-		if (WARN_ON_ONCE(!static_key_fast_inc_not_disabled(key))) {
-			jump_label_unlock();
+		/*
+		 * While holding the mutex this should never observe
+		 * anything else than a value >= 1 and succeed
+		 */
+		if (WARN_ON_ONCE(!static_key_fast_inc_not_disabled(key)))
 			return false;
-		}
 	}
-	jump_label_unlock();
 	return true;
 }
 

[tip: locking/core] jump_label: Clarify condition in static_key_fast_inc_not_disabled()

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     695ef796467ed228b60f1915995e390aea3d85c6
Gitweb:        https://git.kernel.org/tip/695ef796467ed228b60f1915995e390aea3d85c6
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Mon, 10 Jun 2024 14:46:37 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 11 Jun 2024 11:25:23 +02:00

jump_label: Clarify condition in static_key_fast_inc_not_disabled()

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 <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20240610124406.484973160@linutronix.de
---
 kernel/jump_label.c |  9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 1f05a19..4d06ec2 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -132,12 +132,15 @@ bool static_key_fast_inc_not_disabled(struct static_key *key)
 	/*
 	 * Negative key->enabled has a special meaning: it sends
 	 * static_key_slow_inc/dec() down the slow path, and it is non-zero
-	 * so it counts as "enabled" in jump_label_update().  Note that
-	 * atomic_inc_unless_negative() checks >= 0, so roll our own.
+	 * so it counts as "enabled" in jump_label_update().
+	 *
+	 * The INT_MAX overflow condition is either used by the networking
+	 * code to reset or detected in the slow path of
+	 * static_key_slow_inc_cpuslocked().
 	 */
 	v = atomic_read(&key->enabled);
 	do {
-		if (v <= 0 || (v + 1) < 0)
+		if (v <= 0 || v == INT_MAX)
 			return false;
 	} while (!likely(atomic_try_cmpxchg(&key->enabled, &v, v + 1)));
 

[tip: locking/core] perf/x86: Serialize set_attr_rdpmc()

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     bb9bb45f746b0f9457de9c3fc4da143a6351bdc9
Gitweb:        https://git.kernel.org/tip/bb9bb45f746b0f9457de9c3fc4da143a6351bdc9
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Mon, 10 Jun 2024 14:46:35 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 11 Jun 2024 11:25:22 +02:00

perf/x86: Serialize set_attr_rdpmc()

Yue and Xingwei reported a jump label failure. It's caused by the lack of
serialization in set_attr_rdpmc():

CPU0                           CPU1

Assume: x86_pmu.attr_rdpmc == 0

if (val != x86_pmu.attr_rdpmc) {
  if (val == 0)
    ...
  else if (x86_pmu.attr_rdpmc == 0)
    static_branch_dec(&rdpmc_never_available_key);

				if (val != x86_pmu.attr_rdpmc) {
				   if (val == 0)
				      ...
				   else if (x86_pmu.attr_rdpmc == 0)
     FAIL, due to imbalance --->      static_branch_dec(&rdpmc_never_available_key);

The reported BUG() is a consequence of the above and of another bug in the
jump label core code. The core code needs a separate fix, but that cannot
prevent the imbalance problem caused by set_attr_rdpmc().

Prevent this by serializing set_attr_rdpmc() locally.

Fixes: a66734297f78 ("perf/x86: Add /sys/devices/cpu/rdpmc=2 to allow rdpmc for all tasks")
Closes: https://lore.kernel.org/r/CAEkJfYNzfW1vG=ZTMdz_Weoo=RXY1NDunbxnDaLyj8R4kEoE_w@mail.gmail.com
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.359476013@linutronix.de
---
 arch/x86/events/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 5b0dd07..acd367c 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2547,6 +2547,7 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
 			      struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
+	static DEFINE_MUTEX(rdpmc_mutex);
 	unsigned long val;
 	ssize_t ret;
 
@@ -2560,6 +2561,8 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
 	if (x86_pmu.attr_rdpmc_broken)
 		return -ENOTSUPP;
 
+	guard(mutex)(&rdpmc_mutex);
+
 	if (val != x86_pmu.attr_rdpmc) {
 		/*
 		 * Changing into or out of never available or always available,

[tip: locking/core] jump_label: Fix concurrency issues in static_key_slow_dec()

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     83ab38ef0a0b2407d43af9575bb32333fdd74fb2
Gitweb:        https://git.kernel.org/tip/83ab38ef0a0b2407d43af9575bb32333fdd74fb2
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Mon, 10 Jun 2024 14:46:36 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 11 Jun 2024 11:25:23 +02:00

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 | 45 ++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 3218fa5..1f05a19 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -131,7 +131,7 @@ bool static_key_fast_inc_not_disabled(struct static_key *key)
 	STATIC_KEY_CHECK_USE(key);
 	/*
 	 * Negative key->enabled has a special meaning: it sends
-	 * static_key_slow_inc() down the slow path, and it is non-zero
+	 * static_key_slow_inc/dec() down the slow path, and it is non-zero
 	 * so it counts as "enabled" in jump_label_update().  Note that
 	 * atomic_inc_unless_negative() checks >= 0, so roll our own.
 	 */
@@ -150,7 +150,7 @@ bool static_key_slow_inc_cpuslocked(struct static_key *key)
 	lockdep_assert_cpus_held();
 
 	/*
-	 * Careful if we get concurrent static_key_slow_inc() calls;
+	 * Careful if we get concurrent static_key_slow_inc/dec() calls;
 	 * later calls must wait for the first one to _finish_ the
 	 * jump_label_update() process.  At the same time, however,
 	 * the jump_label_update() call below wants to see
@@ -247,20 +247,32 @@ EXPORT_SYMBOL_GPL(static_key_disable);
 
 static bool static_key_slow_try_dec(struct static_key *key)
 {
-	int val;
-
-	val = atomic_fetch_add_unless(&key->enabled, -1, 1);
-	if (val == 1)
-		return false;
+	int v;
 
 	/*
-	 * The negative count check is valid even when a negative
-	 * key->enabled is in use by static_key_slow_inc(); a
-	 * __static_key_slow_dec() before the first static_key_slow_inc()
-	 * returns is unbalanced, because all other static_key_slow_inc()
-	 * instances block while the update is in progress.
+	 * Go into the slow path if key::enabled is less than or equal than
+	 * one. One is valid to shut down the key, anything less than one
+	 * is an imbalance, which is handled at the call site.
+	 *
+	 * That includes the special case of '-1' which is set in
+	 * static_key_slow_inc_cpuslocked(), but that's harmless as it is
+	 * fully serialized in the slow path below. By the time this task
+	 * acquires the jump label lock the value is back to one and the
+	 * retry under the lock must succeed.
 	 */
-	WARN(val < 0, "jump label: negative count!\n");
+	v = atomic_read(&key->enabled);
+	do {
+		/*
+		 * Warn about the '-1' case though; since that means a
+		 * decrement is concurrent with a first (0->1) increment. IOW
+		 * people are trying to disable something that wasn't yet fully
+		 * enabled. This suggests an ordering problem on the user side.
+		 */
+		WARN_ON_ONCE(v < 0);
+		if (v <= 1)
+			return false;
+	} while (!likely(atomic_try_cmpxchg(&key->enabled, &v, v - 1)));
+
 	return true;
 }
 
@@ -271,10 +283,11 @@ static void __static_key_slow_dec_cpuslocked(struct static_key *key)
 	if (static_key_slow_try_dec(key))
 		return;
 
-	jump_label_lock();
-	if (atomic_dec_and_test(&key->enabled))
+	guard(mutex)(&jump_label_mutex);
+	if (atomic_cmpxchg(&key->enabled, 1, 0))
 		jump_label_update(key);
-	jump_label_unlock();
+	else
+		WARN_ON_ONCE(!static_key_slow_try_dec(key));
 }
 
 static void __static_key_slow_dec(struct static_key *key)