linux-kernel - Re: [PATCH v2] arm64: smp: smp_send_stop() and crash_smp_send

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: <20240805165336.GA10196@willie-the-truck>
Date: Mon, 5 Aug 2024 17:53:37 +0100
From: Will Deacon <will@...nel.org>
To: Douglas Anderson <dianders@...omium.org>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Mark Rutland <mark.rutland@....com>,
	Sumit Garg <sumit.garg@...aro.org>, Yu Zhao <yuzhao@...gle.com>,
	Misono Tomohiro <misono.tomohiro@...itsu.com>,
	Stephen Boyd <swboyd@...omium.org>, Chen-Yu Tsai <wens@...e.org>,
	Marc Zyngier <maz@...nel.org>,
	Daniel Thompson <daniel.thompson@...aro.org>,
	D Scott Phillips <scott@...amperecomputing.com>,
	Frederic Weisbecker <frederic@...nel.org>,
	"Guilherme G. Piccoli" <gpiccoli@...lia.com>,
	James Morse <james.morse@....com>, Kees Cook <kees@...nel.org>,
	Tony Luck <tony.luck@...el.com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64: smp: smp_send_stop() and crash_smp_send_stop()
 should try non-NMI first

Hi Doug,

On Tue, Jun 25, 2024 at 04:07:22PM -0700, Douglas Anderson wrote:
> @@ -1084,79 +1088,87 @@ static inline unsigned int num_other_online_cpus(void)
>  
>  void smp_send_stop(void)
>  {
> +	static unsigned long stop_in_progress;
> +	cpumask_t mask;
>  	unsigned long timeout;
>  
> -	if (num_other_online_cpus()) {
> -		cpumask_t mask;
> +	/*
> +	 * If this cpu is the only one alive at this point in time, online or
> +	 * not, there are no stop messages to be sent around, so just back out.
> +	 */
> +	if (num_other_online_cpus() == 0)
> +		goto skip_ipi;
>  
> -		cpumask_copy(&mask, cpu_online_mask);
> -		cpumask_clear_cpu(smp_processor_id(), &mask);
> +	/* Only proceed if this is the first CPU to reach this code */
> +	if (test_and_set_bit(0, &stop_in_progress))
> +		return;
>  
> -		if (system_state <= SYSTEM_RUNNING)
> -			pr_crit("SMP: stopping secondary CPUs\n");
> -		smp_cross_call(&mask, IPI_CPU_STOP);
> -	}
> +	cpumask_copy(&mask, cpu_online_mask);
> +	cpumask_clear_cpu(smp_processor_id(), &mask);
>  
> -	/* Wait up to one second for other CPUs to stop */
> +	if (system_state <= SYSTEM_RUNNING)
> +		pr_crit("SMP: stopping secondary CPUs\n");
> +
> +	/*
> +	 * Start with a normal IPI and wait up to one second for other CPUs to
> +	 * stop. We do this first because it gives other processors a chance
> +	 * to exit critical sections / drop locks and makes the rest of the
> +	 * stop process (especially console flush) more robust.
> +	 */
> +	smp_cross_call(&mask, IPI_CPU_STOP);

I realise you've moved this out of crash_smp_send_stop() and it looks
like we inherited the code from x86, but do you know how this serialise
against CPU hotplug operations? I've spent the last 20m looking at the
code and I can't see what prevents other CPUs from coming and going
while we're trying to IPI a non-atomic copy of 'cpu_online_mask'.

Will