linux-kernel - Re: [PATCH 1/7] stop_machine: Introduce stop_machine

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: <dffe90ae-3dfa-4f64-b9f5-311d8fc0752b@intel.com>
Date: Thu, 29 Jan 2026 07:47:58 -0800
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Borislav Petkov <bp@...en8.de>, "Kaplan, David" <David.Kaplan@....com>
CC: "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 1/29/2026 4:17 AM, Borislav Petkov wrote:
> On Wed, Jan 28, 2026 at 05:35:56PM +0100, Borislav Petkov wrote:>
> -
> -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);
> -}
Thanks for the detailed write-up! I just wanted to clarify a few points, 
if you don't mind:

First, on patch6, it is going to be used by microcode_offline_nmi_handler()
{
	if (!raw_cpu_read(ucode_ctrl.nmi_enabled))
		return;
	raw_cpu_write(ucode_ctrl.nmi_enabled, false);
	raw_cpu_write(ucode_ctrl.result, UCODE_OFFLINE);
	raw_atomic_inc(&offline_in_nmi);
	wait_for_ctrl();
}

I assume you consider the per-CPU nmi_enabled bit is enough going there. 
Then,

DEFINE_IDTENTRY_RAW(exc_nmi)
{
  ...
	if (arch_cpu_is_offline(smp_processor_id())) {
		microcode_offline_nmi_handler();
		return;
	}
...
}

Correct?

The other bit is possible bug after the removal. The NMI path 
effectively becomes:

static noinstr void default_do_nmi(struct pt_regs *regs)
...
+	if (stop_machine_nmi_handler())
+		goto out;

which means it could be invoked from an NMI unrelated to stop-machine.

>   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);

In that case, msdata could still be NULL here.

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

Also, I suppose

	this_cpu_write(stop_machine_nmi_ctrl.msdata, NULL);

> @@ -243,6 +232,17 @@ bool noinstr stop_machine_nmi_handler(void)
>   	return true;
>   }
>   

Finally, while I do appreciate the nmi_cpus approach, I could also think 
another simple msdata == NULL check could be a guard here too.

Thanks,
Chang