Re: [syzbot] [kernel?] general protection fault in timers_dead_cpu

Re: [syzbot] [kernel?] general protection fault in timers_dead_cpu

[Thomas Gleixner]

On Fri, Aug 14 2026 at 00:28, syzbot wrote:
> HEAD commit:    db2ddb871435 Linux 7.2-rc7
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=14ec2149580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=c44651ea7dd2f307

This has NUMA_EMU=y. Either turn it off or add '-smp 2,sockets=2' to the
qemu command line. Otherwise the topology code is unhappy.

> dashboard link: https://syzkaller.appspot.com/bug?extid=74de56995244fe32ffe2
> compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12ec2149580000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16d962c6580000
>
> Downloadable assets:
> disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-db2ddb87.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/698def9fcf7a/vmlinux-db2ddb87.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/fd8b6091a563/bzImage-db2ddb87.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+74de56995244fe32ffe2@syzkaller.appspotmail.com
>
> smpboot: CPU 1 is now offline
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> CPU: 2 UID: 0 PID: 6237 Comm: syz.2.92 Not tainted syzkaller #0 PREEMPT(full) 
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> RIP: 0010:__hlist_del include/linux/list.h:1029 [inline]
> RIP: 0010:detach_timer kernel/time/timer.c:891 [inline]
> RIP: 0010:migrate_timer_list kernel/time/timer.c:2493 [inline]
> RIP: 0010:timers_dead_cpu+0x326/0x860 kernel/time/timer.c:2541

This happens because the reproducer does two things in parallel:

  1) Hotplug CPU1

  2) Toggle /sys/devices/system/machinecheck/machinecheck0/ignore_ce

#2 is not serialized against CPU hotplug so it can end up interfering
with the hotplug operation:

CPU0                    CPU1

hotplug
 kick_ap()
 wait_for_ap()
                        hotplug
                        mce_cpu_pre_down()
                          mce_disable_cpu();
                          timer_delete_sync();

                        // CPU is still marked online

set_ignore_ce()
  on_each_cpu(mce_enable_ce, (void *)1, 1);

                        IPI
                          timer_start()

                        ....

hotplug

 timers_dead_cpu()
   migrate timer to CPU0
   
   // Migrates the MCE timer of CPU1, which is a bug in itself

...

hotplug
  bringup_ap()
                        ...
                        identify_secondary_cpu()
                           mcheck_cpu_init()
                             __mcheck_cpu_setup_timer()
                                timer_setup() <- FAIL

That re-initializes the active timer, which is now queued on CPU0.

What puzzled me was that debugobjects did not catch that issue. It
turned out that during some rework the debug_activate() invocation for
the timer migration case got lost. So debugobjects carries the wrong
state. That's easy to fix:

--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -2492,6 +2492,7 @@ static void migrate_timer_list(struct timer_base *new_base, struct hlist_head *h
 		timer = hlist_entry(head->first, struct timer_list, entry);
 		detach_timer(timer, false);
 		timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu;
+		debug_timer_activate(timer);
 		internal_add_timer(new_base, timer);
 	}
 }

With that it catches the culprit as expected:

 ODEBUG: init active (active state 0) object: ffff88827be234a0 object type: timer_list hint: mce_timer_fn+0x0/0x280
 WARNING: lib/debugobjects.c:632 at debug_print_object+0xec/0x230, CPU#1: swapper/1/0
 CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 7.2.0-dirty #274 PREEMPT(full)
 RIP: 0010:debug_print_object+0x18a/0x230
 Call Trace:
  __debug_object_init+0x230/0x3c0
  timer_init_key+0x5c/0x2e0
  mcheck_cpu_init+0x3e1/0x600
  identify_cpu+0x1e03/0x3660
  identify_secondary_cpu+0xaa/0x160
  ap_starting+0xa1/0x150
  start_secondary+0x66/0x110
  common_startup_64+0x13e/0x157

The knee jerk "fix" is to serialize against CPU hotplug in
set_ignore_ce() and the other sysfs write functions which can result in
exactly the same problem. It's not only the timer. CMCI suffers from the
same issue that it can be reenabled via sysfs between the
"x86/mce:online" state and going completely offline. Haven't looked
further, but that seems to be a general design problem in that code.

But guarding against hotplug alone solves it only partially because with
partial hotplug the same issue happens when:

   1) a partial hotplug goes below the "x86/mce:online" state which
      disarms the timer, but stops before the CPU is marked offline

   2) set_ignore_ce() or one of the other sysfs write functions
      reenables it

   3) a subsequent hotplug operation brings the CPU completely down.

The below quick hack, which I'm not proud of, cures it. I let the MCE
wizards think about the underlying design problem and let them come up
with a hopefully nicer solution.

Thanks,

        tglx
---

--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -68,6 +68,8 @@ static DEFINE_MUTEX(mce_sysfs_mutex);
 
 #define SPINUNIT		100	/* 100ns */
 
+static struct cpumask mce_active_cpus;
+
 DEFINE_PER_CPU_READ_MOSTLY(unsigned int, mce_num_banks);
 
 DEFINE_PER_CPU_READ_MOSTLY(struct mce_bank[MAX_NR_BANKS], mce_banks_array);
@@ -2459,6 +2461,8 @@ static void mce_cpu_restart(void *data)
 {
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
+	if (!cpumask_test_cpu(smp_processor_id(), &mce_active_cpus))
+		return;
 	__mcheck_cpu_init_generic();
 	__mcheck_cpu_init_prepare_banks();
 	__mcheck_cpu_init_timer();
@@ -2478,6 +2482,8 @@ static void mce_disable_cmci(void *data)
 {
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
+	if (!cpumask_test_cpu(smp_processor_id(), &mce_active_cpus))
+		return;
 	cmci_clear();
 }
 
@@ -2485,6 +2491,8 @@ static void mce_enable_ce(void *all)
 {
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
+	if (!cpumask_test_cpu(smp_processor_id(), &mce_active_cpus))
+		return;
 	cmci_reenable();
 	cmci_recheck();
 	if (all)
@@ -2540,6 +2548,7 @@ static ssize_t set_bank(struct device *s
 	b->ctl = new;
 
 	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
 	mce_restart();
 	mutex_unlock(&mce_sysfs_mutex);
 
@@ -2557,6 +2566,7 @@ static ssize_t set_ignore_ce(struct devi
 
 	mutex_lock(&mce_sysfs_mutex);
 	if (mca_cfg.ignore_ce ^ !!new) {
+		guard(cpus_read_lock)();
 		if (new) {
 			/* disable ce features */
 			mce_timer_delete_all();
@@ -2584,6 +2594,7 @@ static ssize_t set_cmci_disabled(struct
 
 	mutex_lock(&mce_sysfs_mutex);
 	if (mca_cfg.cmci_disabled ^ !!new) {
+		guard(cpus_read_lock)();
 		if (new) {
 			/* disable cmci */
 			on_each_cpu(mce_disable_cmci, NULL, 1);
@@ -2610,6 +2621,7 @@ static ssize_t store_int_with_restart(st
 		return ret;
 
 	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
 	mce_restart();
 	mutex_unlock(&mce_sysfs_mutex);
 
@@ -2730,6 +2742,8 @@ static void mce_disable_cpu(void)
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
 
+	cpumask_clear_cpu(smp_processor_id(), &mce_active_cpus);
+
 	if (!cpuhp_tasks_frozen)
 		cmci_clear();
 
@@ -2752,6 +2766,8 @@ static void mce_reenable_cpu(void)
 		if (b->init)
 			wrmsrq(mca_msr_reg(i, MCA_CTL), b->ctl);
 	}
+
+	cpumask_set_cpu(smp_processor_id(), &mce_active_cpus);
 }
 
 static int mce_cpu_dead(unsigned int cpu)

Re: [syzbot] [kernel?] general protection fault in timers_dead_cpu

[Thomas Gleixner]

On Mon, Aug 17 2026 at 23:14, Thomas Gleixner wrote:
> The below quick hack, which I'm not proud of, cures it. I let the MCE
> wizards think about the underlying design problem and let them come up
> with a hopefully nicer solution.

After talking to Borislav briefly, I came up with less ugly one.

Thanks,

        tglx
---
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1734,8 +1734,13 @@ int memory_failure(unsigned long pfn, in
  */
 static unsigned long check_interval = INITIAL_CHECK_INTERVAL;
 
-static DEFINE_PER_CPU(unsigned long, mce_next_interval); /* in jiffies */
-static DEFINE_PER_CPU(struct timer_list, mce_timer);
+struct mce_poll_state {
+	struct timer_list	timer;
+	unsigned long		next_interval;
+	bool			active;
+};
+
+static DEFINE_PER_CPU(struct mce_poll_state, mce_poll_state);
 
 static void __start_timer(struct timer_list *t, unsigned long interval)
 {
@@ -1764,12 +1769,12 @@ static bool should_enable_timer(unsigned
 
 static void mce_timer_fn(struct timer_list *t)
 {
-	struct timer_list *cpu_t = this_cpu_ptr(&mce_timer);
+	struct mce_poll_state *pst = this_cpu_ptr(&mce_poll_state);
 	unsigned long iv;
 
-	WARN_ON(cpu_t != t);
+	WARN_ON(&pst->timer != t);
 
-	iv = __this_cpu_read(mce_next_interval);
+	iv = pst->next_interval;
 
 	if (mce_available(this_cpu_ptr(&cpu_info)))
 		mc_poll_banks();
@@ -1786,7 +1791,7 @@ static void mce_timer_fn(struct timer_li
 	if (mce_get_storm_mode()) {
 		__start_timer(t, HZ);
 	} else if (should_enable_timer(iv)) {
-		__this_cpu_write(mce_next_interval, iv);
+		pst->next_interval = iv;
 		__start_timer(t, iv);
 	}
 }
@@ -1798,14 +1803,14 @@ static void mce_timer_fn(struct timer_li
  */
 void mce_timer_kick(bool storm)
 {
-	struct timer_list *t = this_cpu_ptr(&mce_timer);
+	struct mce_poll_state *pst = this_cpu_ptr(&mce_poll_state);
 
 	mce_set_storm_mode(storm);
 
 	if (storm)
-		__start_timer(t, HZ);
+		__start_timer(&pst->timer, HZ);
 	else
-		__this_cpu_write(mce_next_interval, check_interval * HZ);
+		pst->next_interval = check_interval * HZ;
 }
 
 /* Must not be called in IRQ context where timer_delete_sync() can deadlock */
@@ -1814,7 +1819,7 @@ static void mce_timer_delete_all(void)
 	int cpu;
 
 	for_each_online_cpu(cpu)
-		timer_delete_sync(&per_cpu(mce_timer, cpu));
+		timer_delete_sync(&per_cpu(mce_poll_state.timer, cpu));
 }
 
 static void __mcheck_cpu_mce_banks_init(void)
@@ -2070,29 +2075,29 @@ static void __mcheck_cpu_clear_vendor(st
 	}
 }
 
-static void mce_start_timer(struct timer_list *t)
+static void mce_start_timer(struct mce_poll_state *pst)
 {
 	unsigned long iv = check_interval * HZ;
 
 	if (should_enable_timer(iv)) {
-		this_cpu_write(mce_next_interval, iv);
-		__start_timer(t, iv);
+		pst->next_interval = iv;
+		__start_timer(&pst->timer, iv);
 	}
 }
 
 static void __mcheck_cpu_setup_timer(void)
 {
-	struct timer_list *t = this_cpu_ptr(&mce_timer);
+	struct mce_poll_state *pst = this_cpu_ptr(&mce_poll_state);
 
-	timer_setup(t, mce_timer_fn, TIMER_PINNED);
+	timer_setup(&pst->timer, mce_timer_fn, TIMER_PINNED);
 }
 
 static void __mcheck_cpu_init_timer(void)
 {
-	struct timer_list *t = this_cpu_ptr(&mce_timer);
+	struct mce_poll_state *pst = this_cpu_ptr(&mce_poll_state);
 
-	timer_setup(t, mce_timer_fn, TIMER_PINNED);
-	mce_start_timer(t);
+	timer_setup(&pst->timer, mce_timer_fn, TIMER_PINNED);
+	mce_start_timer(pst);
 }
 
 bool filter_mce(struct mce *m)
@@ -2459,6 +2464,8 @@ static void mce_cpu_restart(void *data)
 {
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
+	if (!this_cpu_read(mce_poll_state.active))
+		return;
 	__mcheck_cpu_init_generic();
 	__mcheck_cpu_init_prepare_banks();
 	__mcheck_cpu_init_timer();
@@ -2478,6 +2485,8 @@ static void mce_disable_cmci(void *data)
 {
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
+	if (!this_cpu_read(mce_poll_state.active))
+		return;
 	cmci_clear();
 }
 
@@ -2485,6 +2494,8 @@ static void mce_enable_ce(void *all)
 {
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
+	if (!this_cpu_read(mce_poll_state.active))
+		return;
 	cmci_reenable();
 	cmci_recheck();
 	if (all)
@@ -2540,6 +2551,7 @@ static ssize_t set_bank(struct device *s
 	b->ctl = new;
 
 	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
 	mce_restart();
 	mutex_unlock(&mce_sysfs_mutex);
 
@@ -2557,6 +2569,7 @@ static ssize_t set_ignore_ce(struct devi
 
 	mutex_lock(&mce_sysfs_mutex);
 	if (mca_cfg.ignore_ce ^ !!new) {
+		guard(cpus_read_lock)();
 		if (new) {
 			/* disable ce features */
 			mce_timer_delete_all();
@@ -2584,6 +2597,7 @@ static ssize_t set_cmci_disabled(struct
 
 	mutex_lock(&mce_sysfs_mutex);
 	if (mca_cfg.cmci_disabled ^ !!new) {
+		guard(cpus_read_lock)();
 		if (new) {
 			/* disable cmci */
 			on_each_cpu(mce_disable_cmci, NULL, 1);
@@ -2610,6 +2624,7 @@ static ssize_t store_int_with_restart(st
 		return ret;
 
 	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
 	mce_restart();
 	mutex_unlock(&mce_sysfs_mutex);
 
@@ -2764,21 +2779,23 @@ static int mce_cpu_dead(unsigned int cpu
 
 static int mce_cpu_online(unsigned int cpu)
 {
-	struct timer_list *t = this_cpu_ptr(&mce_timer);
+	struct mce_poll_state *pst = this_cpu_ptr(&mce_poll_state);
 
 	mce_device_create(cpu);
 	mce_threshold_create_device(cpu);
 	mce_reenable_cpu();
-	mce_start_timer(t);
+	mce_start_timer(pst);
+	pst->active = true;
 	return 0;
 }
 
 static int mce_cpu_pre_down(unsigned int cpu)
 {
-	struct timer_list *t = this_cpu_ptr(&mce_timer);
+	struct mce_poll_state *pst = this_cpu_ptr(&mce_poll_state);
 
+	pst->active = false;
 	mce_disable_cpu();
-	timer_delete_sync(t);
+	timer_delete_sync(&pst->timer);
 	mce_threshold_remove_device(cpu);
 	mce_device_remove(cpu);
 	return 0;

Re: [syzbot] [kernel?] general protection fault in timers_dead_cpu

[Borislav Petkov]

On Tue, Aug 18, 2026 at 12:12:45AM +0200, Thomas Gleixner wrote:
> On Mon, Aug 17 2026 at 23:14, Thomas Gleixner wrote:
> > The below quick hack, which I'm not proud of, cures it. I let the MCE
> > wizards think about the underlying design problem and let them come up
> > with a hopefully nicer solution.
> 
> After talking to Borislav briefly, I came up with less ugly one.

Yap, LGTM.

Lemme try to unify the whole MCE percpu gunk as we spoke.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH] timer: Keep debugobjects state consistent in migrate_timer_list()

[Thomas Gleixner]

When timers are migrated away from an offline CPU the debugobjects state
gets corrupted. The timer is accounted as inactive on deletion, but the
enqueue on the alive CPU lacks the activation call.

That used to work, but got broken when the trace point and the debug
objects call got separated. That change missed to fixup
migrate_timer_list().

Add the missing debug_timer_activate() invocation to fix it.

Fixes: dc1e7dc5ac62 ("timer: Move trace point to get proper index")
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Cc: stable@vger.kernel.org
---
 kernel/time/timer.c |    1 +
 1 file changed, 1 insertion(+)

--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -2492,6 +2492,7 @@ static void migrate_timer_list(struct ti
 		timer = hlist_entry(head->first, struct timer_list, entry);
 		detach_timer(timer, false);
 		timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu;
+		debug_timer_activate(timer);
 		internal_add_timer(new_base, timer);
 	}
 }

Re: [syzbot] [kernel?] general protection fault in timers_dead_cpu

[Borislav Petkov]

On Mon, Aug 17, 2026 at 11:14:35PM +0200, Thomas Gleixner wrote:
>  timers_dead_cpu()
>    migrate timer to CPU0
>    
>    // Migrates the MCE timer of CPU1, which is a bug in itself

Stupid question: can we prevent this?

As in, this timer is not migratable, do not migrate it.

But then what do you do with a timer which is not migratable and its CPU goes
offline?

Perhaps cancel it...

It won't matter in the MCE case, that's for sure.

Anyway, just some musings from reading this...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [syzbot] [kernel?] general protection fault in timers_dead_cpu

[Borislav Petkov]

On Mon, Aug 17, 2026 at 04:55:53PM -0700, Borislav Petkov wrote:
> On Mon, Aug 17, 2026 at 11:14:35PM +0200, Thomas Gleixner wrote:
> >  timers_dead_cpu()
> >    migrate timer to CPU0
> >    
> >    // Migrates the MCE timer of CPU1, which is a bug in itself
> 
> Stupid question: can we prevent this?
> 
> As in, this timer is not migratable, do not migrate it.
> 
> But then what do you do with a timer which is not migratable and its CPU goes
> offline?
> 
> Perhaps cancel it...
> 
> It won't matter in the MCE case, that's for sure.
> 
> Anyway, just some musings from reading this...

Hmm, the down path does timer_delete_sync() so I guess I'm missing an aspect
here about the timer migration.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [syzbot] [kernel?] general protection fault in timers_dead_cpu

[Thomas Gleixner]

On Mon, Aug 17 2026 at 17:18, Borislav Petkov wrote:
> On Mon, Aug 17, 2026 at 04:55:53PM -0700, Borislav Petkov wrote:
>> On Mon, Aug 17, 2026 at 11:14:35PM +0200, Thomas Gleixner wrote:
>> >  timers_dead_cpu()
>> >    migrate timer to CPU0
>> >    
>> >    // Migrates the MCE timer of CPU1, which is a bug in itself
>> 
>> Stupid question: can we prevent this?
>> 
>> As in, this timer is not migratable, do not migrate it.

We could do that, but that's just papering over the underlying issues.

>> But then what do you do with a timer which is not migratable and its CPU goes
>> offline?
>> 
>> Perhaps cancel it...

Yes, but that's not really well defined.

>> It won't matter in the MCE case, that's for sure.

Correct. You still have CMCI ...

>> Anyway, just some musings from reading this...
>
> Hmm, the down path does timer_delete_sync() so I guess I'm missing an aspect
> here about the timer migration.

Care to read my first reply where I described exactly how that happens?

The timer is rearmed by that sysfs muck _after_ the down callback
deleted it. And the same happens to CMCI. The down callback stops it and
the sysfs muck reenables it.

Alternatively we can split the hotplug callbacks and have one in the
late stage of hotplug after the point of no return, which stops the
timer and CMCI. Then let the existing one only care about the device
stuff which requires task context. Something like the untested below.

Thanks,

        tglx
---
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2762,23 +2762,33 @@ static int mce_cpu_dead(unsigned int cpu
 	return 0;
 }
 
-static int mce_cpu_online(unsigned int cpu)
+static int mce_cpu_starting(unsigned int cpu)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
-	mce_device_create(cpu);
-	mce_threshold_create_device(cpu);
 	mce_reenable_cpu();
 	mce_start_timer(t);
 	return 0;
 }
 
-static int mce_cpu_pre_down(unsigned int cpu)
+static int mce_cpu_dying(unsigned int cpu)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
 	mce_disable_cpu();
 	timer_delete_sync(t);
+	return 0;
+}
+
+static int mce_cpu_online(unsigned int cpu)
+{
+	mce_device_create(cpu);
+	mce_threshold_create_device(cpu);
+	return 0;
+}
+
+static int mce_cpu_pre_down(unsigned int cpu)
+{
 	mce_threshold_remove_device(cpu);
 	mce_device_remove(cpu);
 	return 0;
@@ -2841,6 +2851,14 @@ static __init int mcheck_init_device(voi
 				mce_cpu_dead);
 	if (err)
 		goto err_out_mem;
+	/*
+	 * Invokes mce_cpu_starting() on all CPUs which are online when
+	 * the state is installed.
+	 */
+	err = cpuhp_setup_state(CPUHP_AP_X86_MCE_STARTING, "x86/mce:starting",
+				mce_cpu_starting, mce_cpu_dying);
+	if (err < 0)
+		goto err_out_starting;
 
 	/*
 	 * Invokes mce_cpu_online() on all CPUs which are online when
@@ -2856,6 +2874,9 @@ static __init int mcheck_init_device(voi
 	return 0;
 
 err_out_online:
+	cpuhp_remove_state(CPUHP_AP_X86_MCE_STARTING);
+
+err_out_starting:
 	cpuhp_remove_state(CPUHP_X86_MCE_DEAD);
 
 err_out_mem:
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -186,6 +186,7 @@ enum cpuhp_state {
 	CPUHP_AP_HRTIMERS_DYING,
 	CPUHP_AP_TICK_DYING,
 	CPUHP_AP_X86_TBOOT_DYING,
+	CPUHP_AP_X86_MCE_STARTING,
 	CPUHP_AP_ARM_CACHE_B15_RAC_DYING,
 	CPUHP_AP_ONLINE,
 	CPUHP_TEARDOWN_CPU,

[PATCH] x86/mce: Serialize sysfs controls against CPU hotplug

[Thomas Gleixner]

syzbot reported a NULL pointer dereference in the CPU hotplug code. It
is caused by the lack of synchronizaton between CPU hotplug and MCE
sysfs controls, which allows the sysfs controls to reenable not only the
timer, but also the CMCI reporting on a CPU which is about to go out. A
similar issue exists in the CPU online path where the timer can be
enabled via sysfs control before the CPU runs the initialization of the
timer.

Prevent this by:

   1) Acquiring the mce_sysfs_mutex in the hotplug callbacks

   2) Excluding CPUs which do not have MCE fully initialized from the sysfs
      control IPIs.

That covers both the CPU up and down situations.

Fixes: 9af43b54ab45 ("x86, mce: sysfs entries for new mce options")
Reported-by: syzbot+74de56995244fe32ffe2@syzkaller.appspotmail.com
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Closes: https://lore.kernel.org/all/6a7ec39f.5b0d2c79.2ef4ef.0002.GAE@google.com
---
 arch/x86/kernel/cpu/mce/core.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2447,6 +2447,12 @@ static struct syscore mce_syscore = {
 /*
  * mce_device: Sysfs support
  */
+static cpumask_var_t mce_device_initialized;
+
+static inline void on_each_enabled_cpu(smp_call_func_t func, void *arg)
+{
+	on_each_cpu_cond_mask(NULL, func, arg, true, mce_device_initialized);
+}
 
 static void mce_cpu_restart(void *data)
 {
@@ -2462,7 +2468,7 @@ static void mce_cpu_restart(void *data)
 static void mce_restart(void)
 {
 	mce_timer_delete_all();
-	on_each_cpu(mce_cpu_restart, NULL, 1);
+	on_each_enabled_cpu(mce_cpu_restart, NULL);
 	mce_schedule_work();
 }
 
@@ -2553,12 +2559,12 @@ static ssize_t set_ignore_ce(struct devi
 		if (new) {
 			/* disable ce features */
 			mce_timer_delete_all();
-			on_each_cpu(mce_disable_cmci, NULL, 1);
+			on_each_enabled_cpu(mce_disable_cmci, NULL);
 			mca_cfg.ignore_ce = true;
 		} else {
 			/* enable ce features */
 			mca_cfg.ignore_ce = false;
-			on_each_cpu(mce_enable_ce, (void *)1, 1);
+			on_each_enabled_cpu(mce_enable_ce, (void *)1);
 		}
 	}
 	mutex_unlock(&mce_sysfs_mutex);
@@ -2579,12 +2585,12 @@ static ssize_t set_cmci_disabled(struct
 	if (mca_cfg.cmci_disabled ^ !!new) {
 		if (new) {
 			/* disable cmci */
-			on_each_cpu(mce_disable_cmci, NULL, 1);
+			on_each_enabled_cpu(mce_disable_cmci, NULL);
 			mca_cfg.cmci_disabled = true;
 		} else {
 			/* enable cmci */
 			mca_cfg.cmci_disabled = false;
-			on_each_cpu(mce_enable_ce, NULL, 1);
+			on_each_enabled_cpu(mce_enable_ce, NULL);
 		}
 	}
 	mutex_unlock(&mce_sysfs_mutex);
@@ -2641,8 +2647,6 @@ static struct device_attribute *mce_devi
 	NULL
 };
 
-static cpumask_var_t mce_device_initialized;
-
 static void mce_device_release(struct device *dev)
 {
 	kfree(dev);
@@ -2759,6 +2763,7 @@ static int mce_cpu_online(unsigned int c
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
+	guard(mutex)(&mce_sysfs_mutex);
 	mce_device_create(cpu);
 	mce_threshold_create_device(cpu);
 	mce_reenable_cpu();
@@ -2770,6 +2775,7 @@ static int mce_cpu_pre_down(unsigned int
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
+	guard(mutex)(&mce_sysfs_mutex);
 	mce_disable_cpu();
 	timer_delete_sync(t);
 	mce_threshold_remove_device(cpu);

[PATCH V2] x86/mce: Serialize sysfs controls against CPU hotplug

[Thomas Gleixner]

On Fri, Oct 23 2026 at 11:18, Thomas Gleixner wrote:
syzbot reported a NULL pointer dereference in the CPU hotplug code. It
is caused by the lack of synchronizaton between CPU hotplug and MCE
sysfs controls, which allows the sysfs controls to reenable not only the
timer, but also the CMCI reporting on a CPU which is about to go out. A
similar issue exists in the CPU online path where the timer can be
enabled via sysfs control before the CPU runs the initialization of the
timer.

Prevent this by:

   1) Serializing the sysfs writes against CPU hotplug.

   2) Removing the redundant timer_setup() call in
      __mcheck_cpu_init_timer(), which can end up in initializing an
      active timer.

   3) Splitting the hotplug callbacks into starting/dying and
      online/offline parts.
   
      The starting/dying callbacks control the timer and the CMCI
      setup/teardown. The online/offline callbacks handle the device
      creation and teardown.

This ensures that the sysfs operations always see consistent CPU online
state and the timer and CMCI are shut down at the time when the CPU goes
offline and re-enabled when the CPU starts without interfering with the
sysfs operations.

Fixes: 9af43b54ab45 ("x86, mce: sysfs entries for new mce options")
Reported-by: syzbot+74de56995244fe32ffe2@syzkaller.appspotmail.com
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Closes: https://lore.kernel.org/all/6a7ec39f.5b0d2c79.2ef4ef.0002.GAE@google.com
---
V2: Use the seperate hotplug state as sashiko rightfully pointed out
    that taking the sysfs mutex in the hotplug callback can deadlock
    against the device creation/deletion.
---
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 50b1614968be..bcf4cba2b7ab 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2091,7 +2091,6 @@ static void __mcheck_cpu_init_timer(void)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
-	timer_setup(t, mce_timer_fn, TIMER_PINNED);
 	mce_start_timer(t);
 }
 
@@ -2532,9 +2531,9 @@ static ssize_t set_bank(struct device *s, struct device_attribute *attr,
 
 	b->ctl = new;
 
-	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
+	guard(mutex)(&mce_sysfs_mutex);
 	mce_restart();
-	mutex_unlock(&mce_sysfs_mutex);
 
 	return size;
 }
@@ -2548,7 +2547,9 @@ static ssize_t set_ignore_ce(struct device *s,
 	if (kstrtou64(buf, 0, &new) < 0)
 		return -EINVAL;
 
-	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
+	guard(mutex)(&mce_sysfs_mutex);
+
 	if (mca_cfg.ignore_ce ^ !!new) {
 		if (new) {
 			/* disable ce features */
@@ -2561,7 +2562,6 @@ static ssize_t set_ignore_ce(struct device *s,
 			on_each_cpu(mce_enable_ce, (void *)1, 1);
 		}
 	}
-	mutex_unlock(&mce_sysfs_mutex);
 
 	return size;
 }
@@ -2575,7 +2575,9 @@ static ssize_t set_cmci_disabled(struct device *s,
 	if (kstrtou64(buf, 0, &new) < 0)
 		return -EINVAL;
 
-	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
+	guard(mutex)(&mce_sysfs_mutex);
+
 	if (mca_cfg.cmci_disabled ^ !!new) {
 		if (new) {
 			/* disable cmci */
@@ -2587,7 +2589,6 @@ static ssize_t set_cmci_disabled(struct device *s,
 			on_each_cpu(mce_enable_ce, NULL, 1);
 		}
 	}
-	mutex_unlock(&mce_sysfs_mutex);
 
 	return size;
 }
@@ -2602,10 +2603,9 @@ static ssize_t store_int_with_restart(struct device *s,
 	if (check_interval == old_check_interval)
 		return ret;
 
-	mutex_lock(&mce_sysfs_mutex);
+	guard(cpus_read_lock)();
+	guard(mutex)(&mce_sysfs_mutex);
 	mce_restart();
-	mutex_unlock(&mce_sysfs_mutex);
-
 	return ret;
 }
 
@@ -2755,23 +2755,33 @@ static int mce_cpu_dead(unsigned int cpu)
 	return 0;
 }
 
-static int mce_cpu_online(unsigned int cpu)
+static int mce_cpu_starting(unsigned int cpu)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
-	mce_device_create(cpu);
-	mce_threshold_create_device(cpu);
 	mce_reenable_cpu();
 	mce_start_timer(t);
 	return 0;
 }
 
-static int mce_cpu_pre_down(unsigned int cpu)
+static int mce_cpu_dying(unsigned int cpu)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
 	mce_disable_cpu();
 	timer_delete_sync(t);
+	return 0;
+}
+
+static int mce_cpu_online(unsigned int cpu)
+{
+	mce_device_create(cpu);
+	mce_threshold_create_device(cpu);
+	return 0;
+}
+
+static int mce_cpu_pre_down(unsigned int cpu)
+{
 	mce_threshold_remove_device(cpu);
 	mce_device_remove(cpu);
 	return 0;
@@ -2834,6 +2844,14 @@ static __init int mcheck_init_device(void)
 				mce_cpu_dead);
 	if (err)
 		goto err_out_mem;
+	/*
+	 * Invokes mce_cpu_starting() on all CPUs which are online when
+	 * the state is installed.
+	 */
+	err = cpuhp_setup_state(CPUHP_AP_X86_MCE_STARTING, "x86/mce:starting",
+				mce_cpu_starting, mce_cpu_dying);
+	if (err < 0)
+		goto err_out_starting;
 
 	/*
 	 * Invokes mce_cpu_online() on all CPUs which are online when
@@ -2849,6 +2867,9 @@ static __init int mcheck_init_device(void)
 	return 0;
 
 err_out_online:
+	cpuhp_remove_state(CPUHP_AP_X86_MCE_STARTING);
+
+err_out_starting:
 	cpuhp_remove_state(CPUHP_X86_MCE_DEAD);
 
 err_out_mem:
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 1b6dba159bb2..6503cb8e7f0e 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -185,6 +185,7 @@ enum cpuhp_state {
 	CPUHP_AP_HRTIMERS_DYING,
 	CPUHP_AP_TICK_DYING,
 	CPUHP_AP_X86_TBOOT_DYING,
+	CPUHP_AP_X86_MCE_STARTING,
 	CPUHP_AP_ARM_CACHE_B15_RAC_DYING,
 	CPUHP_AP_ONLINE,
 	CPUHP_TEARDOWN_CPU,

Re: [PATCH V2] x86/mce: Serialize sysfs controls against CPU hotplug

[Yazen Ghannam]

On Wed, Oct 28, 2026 at 08:45:07PM +0100, Thomas Gleixner wrote:
> On Fri, Oct 23 2026 at 11:18, Thomas Gleixner wrote:
> syzbot reported a NULL pointer dereference in the CPU hotplug code. It
> is caused by the lack of synchronizaton between CPU hotplug and MCE
> sysfs controls, which allows the sysfs controls to reenable not only the
> timer, but also the CMCI reporting on a CPU which is about to go out. A
> similar issue exists in the CPU online path where the timer can be
> enabled via sysfs control before the CPU runs the initialization of the
> timer.
> 
> Prevent this by:
> 
>    1) Serializing the sysfs writes against CPU hotplug.
> 
>    2) Removing the redundant timer_setup() call in
>       __mcheck_cpu_init_timer(), which can end up in initializing an
>       active timer.
> 
>    3) Splitting the hotplug callbacks into starting/dying and
>       online/offline parts.
>    
>       The starting/dying callbacks control the timer and the CMCI
>       setup/teardown. The online/offline callbacks handle the device
>       creation and teardown.
> 
> This ensures that the sysfs operations always see consistent CPU online
> state and the timer and CMCI are shut down at the time when the CPU goes
> offline and re-enabled when the CPU starts without interfering with the
> sysfs operations.
> 
> Fixes: 9af43b54ab45 ("x86, mce: sysfs entries for new mce options")
> Reported-by: syzbot+74de56995244fe32ffe2@syzkaller.appspotmail.com
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Closes: https://lore.kernel.org/all/6a7ec39f.5b0d2c79.2ef4ef.0002.GAE@google.com

There are three others that seem to be the same issue.

Reported-by: syzbot+9d17de59c48d71bd6824@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a6cd87b.2d659fcc.1d46f5.019c.GAE@google.com/
Reported-by: syzbot+553c9f2c5781192c2214@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6ad23c08.7952420f.283480.0348.GAE@google.com/
Reported-by: syzbot+edc6b57cbed1fb72d0f9@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a6cd832.1aa927e4.17d4bf.0008.GAE@google.com/

> ---
> V2: Use the seperate hotplug state as sashiko rightfully pointed out
>     that taking the sysfs mutex in the hotplug callback can deadlock
>     against the device creation/deletion.
> ---

I was looking at this issue before and have some QEMU test cases.

These were able to hit the new Sashiko findings.

There are two separate ones.
1) Another deadlock.
2) PREEMPT_RT warning.

The diff below fixes both. Though I'm not sure the if the msleep() is
okay.

Thanks,
Yazen

=====================

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index bcf4cba2b7ab..6d74e393eb6a 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2512,12 +2512,36 @@ static ssize_t show_bank(struct device *s, struct device_attribute *attr,
 	return sprintf(buf, "%llx\n", b->ctl);
 }
 
+/*
+ * The stores run with the kernfs active reference of their attribute held.
+ * CPU hotplug removes the attributes with cpu_hotplug_lock held and waits
+ * for that reference to drop. So never wait for cpu_hotplug_lock here.
+ * Restart the syscall instead, like lock_device_hotplug_sysfs().
+ */
+static int mce_sysfs_lock(void)
+{
+	if (cpus_read_trylock()) {
+		mutex_lock(&mce_sysfs_mutex);
+		return 0;
+	}
+	/* Avoid busy looping (5 ms of sleep should do). */
+	msleep(5);
+	return restart_syscall();
+}
+
+static void mce_sysfs_unlock(void)
+{
+	mutex_unlock(&mce_sysfs_mutex);
+	cpus_read_unlock();
+}
+
 static ssize_t set_bank(struct device *s, struct device_attribute *attr,
 			const char *buf, size_t size)
 {
 	u8 bank = attr_to_bank(attr)->bank;
 	struct mce_bank *b;
 	u64 new;
+	int err;
 
 	if (kstrtou64(buf, 0, &new) < 0)
 		return -EINVAL;
@@ -2529,12 +2553,14 @@ static ssize_t set_bank(struct device *s, struct device_attribute *attr,
 	if (!b->init)
 		return -ENODEV;
 
-	b->ctl = new;
+	err = mce_sysfs_lock();
+	if (err)
+		return err;
 
-	guard(cpus_read_lock)();
-	guard(mutex)(&mce_sysfs_mutex);
+	b->ctl = new;
 	mce_restart();
 
+	mce_sysfs_unlock();
 	return size;
 }
 
@@ -2543,12 +2569,14 @@ static ssize_t set_ignore_ce(struct device *s,
 			     const char *buf, size_t size)
 {
 	u64 new;
+	int err;
 
 	if (kstrtou64(buf, 0, &new) < 0)
 		return -EINVAL;
 
-	guard(cpus_read_lock)();
-	guard(mutex)(&mce_sysfs_mutex);
+	err = mce_sysfs_lock();
+	if (err)
+		return err;
 
 	if (mca_cfg.ignore_ce ^ !!new) {
 		if (new) {
@@ -2563,6 +2591,7 @@ static ssize_t set_ignore_ce(struct device *s,
 		}
 	}
 
+	mce_sysfs_unlock();
 	return size;
 }
 
@@ -2571,12 +2600,14 @@ static ssize_t set_cmci_disabled(struct device *s,
 				 const char *buf, size_t size)
 {
 	u64 new;
+	int err;
 
 	if (kstrtou64(buf, 0, &new) < 0)
 		return -EINVAL;
 
-	guard(cpus_read_lock)();
-	guard(mutex)(&mce_sysfs_mutex);
+	err = mce_sysfs_lock();
+	if (err)
+		return err;
 
 	if (mca_cfg.cmci_disabled ^ !!new) {
 		if (new) {
@@ -2590,6 +2621,7 @@ static ssize_t set_cmci_disabled(struct device *s,
 		}
 	}
 
+	mce_sysfs_unlock();
 	return size;
 }
 
@@ -2597,15 +2629,20 @@ static ssize_t store_int_with_restart(struct device *s,
 				      struct device_attribute *attr,
 				      const char *buf, size_t size)
 {
-	unsigned long old_check_interval = check_interval;
-	ssize_t ret = device_store_ulong(s, attr, buf, size);
+	unsigned long old_check_interval;
+	ssize_t ret;
+	int err;
 
-	if (check_interval == old_check_interval)
-		return ret;
+	err = mce_sysfs_lock();
+	if (err)
+		return err;
 
-	guard(cpus_read_lock)();
-	guard(mutex)(&mce_sysfs_mutex);
-	mce_restart();
+	old_check_interval = check_interval;
+	ret = device_store_ulong(s, attr, buf, size);
+	if (check_interval != old_check_interval)
+		mce_restart();
+
+	mce_sysfs_unlock();
 	return ret;
 }
 
@@ -2769,7 +2806,8 @@ static int mce_cpu_dying(unsigned int cpu)
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
 	mce_disable_cpu();
-	timer_delete_sync(t);
+	/* Pinned to this dying CPU, so the callback can't be running. */
+	timer_delete(t);
 	return 0;
 }
 

Re: [PATCH V2] x86/mce: Serialize sysfs controls against CPU hotplug

[Borislav Petkov]

On Thu, Oct 29, 2026 at 12:56:36PM -0400, Yazen Ghannam wrote:
> The diff below fixes both.

No, we've been trying hard to fix this shit and I already told tglx offlist,
I am that >.< close from simply disabling any hotplug operations as long as
someone touches those sysfs files.

There is no good reason to be toggling those continuously and f*ck around with
CPU hotplug at the same time. You do those configurations and then done. Not
whack them all the time.

So this code needs to be simplified - not made an even crazier and
unmaintainable mess.

So, I don't know yet in detail how we are going to query the hotplug core
whether operations are in progress but once we do, those sysfs helpers should
simply do:

	if (hotplug_operations_in_progress)
		return -ECOMEBACKLATER;

And that hotplug_operations_in_progress should include partial hotplug. I.e.,
has someone done partial hotplug? If so, error too.

And that's it.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH V2] x86/mce: Serialize sysfs controls against CPU hotplug

[Yazen Ghannam]

On Thu, Oct 29, 2026 at 10:46:46AM -0700, Borislav Petkov wrote:
> On Thu, Oct 29, 2026 at 12:56:36PM -0400, Yazen Ghannam wrote:
> > The diff below fixes both.
> 
> No, we've been trying hard to fix this shit and I already told tglx offlist,
> I am that >.< close from simply disabling any hotplug operations as long as
> someone touches those sysfs files.
> 
> There is no good reason to be toggling those continuously and f*ck around with
> CPU hotplug at the same time. You do those configurations and then done. Not
> whack them all the time.
> 
> So this code needs to be simplified - not made an even crazier and
> unmaintainable mess.
> 
> So, I don't know yet in detail how we are going to query the hotplug core
> whether operations are in progress but once we do, those sysfs helpers should
> simply do:
> 
> 	if (hotplug_operations_in_progress)
> 		return -ECOMEBACKLATER;
> 
> And that hotplug_operations_in_progress should include partial hotplug. I.e.,
> has someone done partial hotplug? If so, error too.
> 
> And that's it.
> 
> Thx.
> 

Right, so what if we deprecate this legacy machinecheck bus and put MCE
stuff under the CPU bus? This could be like cpufreq and cpuidle.

I expect this would need a lot more changes, but the end result may be
simpler.

Thanks,
Yazen

Re: [PATCH V2] x86/mce: Serialize sysfs controls against CPU hotplug

[Borislav Petkov]

On Thu, Oct 29, 2026 at 02:08:01PM -0400, Yazen Ghannam wrote:
> Right, so what if we deprecate this legacy machinecheck bus and put MCE
> stuff under the CPU bus? This could be like cpufreq and cpuidle.
> 
> I expect this would need a lot more changes, but the end result may be
> simpler.

And it would need at least a decade to whack because of user-visible ABI.

We should remember that proposal and point people to the /cpu/ hierarchy when
new things need to be added there which are mce specific.

I would prefer not to add anything ABI-like though and let that stuff die
because it does not belong there at all. But that was done a looong time
ago and we're stuck with it.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette