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

[Thomas Gleixner <tglx@kernel.org>]

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;