[an error occurred while processing this directive]
lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260129121729.GRaXtP2aeWkQKegxC2@fat_crate.local>
Date: Thu, 29 Jan 2026 13:17:29 +0100
From: Borislav Petkov <bp@...en8.de>
To: "Kaplan, David" <David.Kaplan@....com>
Cc: "Chang S. Bae" <chang.seok.bae@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"peterz@...radead.org" <peterz@...radead.org>
Subject: Re: [PATCH 1/7] stop_machine: Introduce stop_machine_nmi()

On Wed, Jan 28, 2026 at 05:35:56PM +0100, Borislav Petkov wrote:
> I'd say let's make the simplest variant work first and then go optimize it.

IOW, something like this, ontop.

What I'm not sure about is:

        if (msdata->use_nmi) {
                this_cpu_write(stop_machine_nmi_ctrl.msdata, msdata);
                arch_send_self_nmi();

<--- we send the NMI IPI here...

                return raw_cpu_read(stop_machine_nmi_ctrl.err);

... and we read the err result immediately but what guarantees us that the NMI
handler on that CPU will have run and written err:

	raw_cpu_write(stop_machine_nmi_ctrl.err, err);

?

In any case, this looks simpler to me. You have a single struct
multi_stop_data.nmi_cpus which accounts for which CPU runs the NMI handler and
there's no need for any enable logic anymore.

---

diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 0ecec98d52a6..02faf75aa974 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -176,8 +176,21 @@ struct multi_stop_data {
 	atomic_t		thread_ack;
 
 	bool			use_nmi;
+
+	/*
+	 * cpumask of CPUs on which to raise an NMI; used in the NMI
+	 * stomp_machine variant.
+	 */
+	struct cpumask		nmi_cpus;
 };
 
+struct stop_machine_nmi_ctrl {
+	struct multi_stop_data *msdata;
+	int err;
+};
+
+static DEFINE_PER_CPU(struct stop_machine_nmi_ctrl, stop_machine_nmi_ctrl);
+
 static void set_state(struct multi_stop_data *msdata,
 		      enum multi_stop_state newstate)
 {
@@ -199,27 +212,6 @@ notrace void __weak stop_machine_yield(const struct cpumask *cpumask)
 	cpu_relax();
 }
 
-struct stop_machine_nmi_ctrl {
-	bool nmi_enabled;
-	struct multi_stop_data *msdata;
-	int err;
-};
-
-static DEFINE_STATIC_KEY_FALSE(stop_machine_nmi_handler_enable);
-
-bool stop_machine_nmi_handler_enabled(void)
-{
-	return static_branch_unlikely(&stop_machine_nmi_handler_enable);
-}
-
-static DEFINE_PER_CPU(struct stop_machine_nmi_ctrl, stop_machine_nmi_ctrl);
-
-static void enable_nmi_handler(struct multi_stop_data *msdata)
-{
-	this_cpu_write(stop_machine_nmi_ctrl.msdata, msdata);
-	this_cpu_write(stop_machine_nmi_ctrl.nmi_enabled, true);
-}
-
 void __weak arch_send_self_nmi(void)
 {
 	/* Arch code must implement this to support stop_machine_nmi() */
@@ -227,15 +219,12 @@ void __weak arch_send_self_nmi(void)
 
 bool noinstr stop_machine_nmi_handler(void)
 {
-	struct multi_stop_data *msdata;
+	struct multi_stop_data *msdata = raw_cpu_read(stop_machine_nmi_ctrl.msdata);
 	int err;
 
-	if (!raw_cpu_read(stop_machine_nmi_ctrl.nmi_enabled))
+	if (!cpumask_test_and_clear_cpu(smp_processor_id(), &msdata->nmi_cpus))
 		return false;
 
-	raw_cpu_write(stop_machine_nmi_ctrl.nmi_enabled, false);
-
-	msdata = raw_cpu_read(stop_machine_nmi_ctrl.msdata);
 	instrumentation_begin();
 	err = msdata->fn(msdata->data);
 	instrumentation_end();
@@ -243,6 +232,17 @@ bool noinstr stop_machine_nmi_handler(void)
 	return true;
 }
 
+static int __multi_cpu_stop(struct multi_stop_data *msdata)
+{
+	if (msdata->use_nmi) {
+		this_cpu_write(stop_machine_nmi_ctrl.msdata, msdata);
+		arch_send_self_nmi();
+		return raw_cpu_read(stop_machine_nmi_ctrl.err);
+	} else {
+		return msdata->fn(msdata->data);
+	}
+}
+
 /* This is the cpu_stop function which stops the CPU. */
 static int multi_cpu_stop(void *data)
 {
@@ -280,15 +280,8 @@ static int multi_cpu_stop(void *data)
 				hard_irq_disable();
 				break;
 			case MULTI_STOP_RUN:
-				if (is_active) {
-					if (msdata->use_nmi) {
-						enable_nmi_handler(msdata);
-						arch_send_self_nmi();
-						err = raw_cpu_read(stop_machine_nmi_ctrl.err);
-					} else {
-						err = msdata->fn(msdata->data);
-					}
-				}
+				if (is_active)
+					err = __multi_cpu_stop(msdata);
 				break;
 			default:
 				break;
@@ -648,6 +641,9 @@ static int __stop_machine_cpuslocked(cpu_stop_fn_t fn, void *data,
 		.use_nmi = use_nmi,
 	};
 
+	if (use_nmi)
+		cpumask_copy(&msdata.nmi_cpus, cpus);
+
 	lockdep_assert_cpus_held();
 
 	if (!stop_machine_initialized) {
@@ -683,13 +679,7 @@ int stop_machine_cpuslocked(cpu_stop_fn_t fn, void *data,
 int stop_machine_cpuslocked_nmi(cpu_stop_fn_t fn, void *data,
 				const struct cpumask *cpus)
 {
-	int ret;
-
-	static_branch_enable_cpuslocked(&stop_machine_nmi_handler_enable);
-	ret = __stop_machine_cpuslocked(fn, data, cpus, true);
-	static_branch_disable_cpuslocked(&stop_machine_nmi_handler_enable);
-
-	return ret;
+	return __stop_machine_cpuslocked(fn, data, cpus, true);
 }
 
 int stop_machine(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus)

---

Thx.

-- 
Regards/Gruss,
    Boris.

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