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

[Thomas Gleixner <tglx@kernel.org>]

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,