linux-kernel - Re: [PATCH] rcu/nocb: Fix rcuog wake-up from offline softirq

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] [day] [month] [year] [list]

Message-ID: <126f384f-bef4-468b-9082-0e3d47bdfd06@paulmck-laptop>
Date: Tue, 22 Oct 2024 10:23:31 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Boqun Feng <boqun.feng@...il.com>,
	Joel Fernandes <joel@...lfernandes.org>,
	Neeraj Upadhyay <neeraj.upadhyay@....com>,
	Uladzislau Rezki <urezki@...il.com>,
	Zqiang <qiang.zhang1211@...il.com>, rcu <rcu@...r.kernel.org>,
	kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH] rcu/nocb: Fix rcuog wake-up from offline softirq

On Thu, Oct 10, 2024 at 06:36:09PM +0200, Frederic Weisbecker wrote:
> After a CPU has set itself offline and before it eventually calls
> rcutree_report_cpu_dead(), there are still opportunities for callbacks
> to be enqueued, for example from a softirq. When that happens on NOCB,
> the rcuog wake-up is deferred through an IPI to an online CPU in order
> not to call into the scheduler and risk arming the RT-bandwidth after
> hrtimers have been migrated out and disabled.
> 
> But performing a synchronized IPI from a softirq is buggy as reported in
> the following scenario:
> 
>         WARNING: CPU: 1 PID: 26 at kernel/smp.c:633 smp_call_function_single
>         Modules linked in: rcutorture torture
>         CPU: 1 UID: 0 PID: 26 Comm: migration/1 Not tainted 6.11.0-rc1-00012-g9139f93209d1 #1
>         Stopper: multi_cpu_stop+0x0/0x320 <- __stop_cpus+0xd0/0x120
>         RIP: 0010:smp_call_function_single
>         <IRQ>
>         swake_up_one_online
>         __call_rcu_nocb_wake
>         __call_rcu_common
>         ? rcu_torture_one_read
>         call_timer_fn
>         __run_timers
>         run_timer_softirq
>         handle_softirqs
>         irq_exit_rcu
>         ? tick_handle_periodic
>         sysvec_apic_timer_interrupt
>         </IRQ>
> 
> Fix this with forcing deferred rcuog wake up through the NOCB timer when
> the CPU is offline. The actual wake up will happen from
> rcutree_report_cpu_dead().
> 
> Reported-by: kernel test robot <oliver.sang@...el.com>
> Closes: https://lore.kernel.org/oe-lkp/202409231644.4c55582d-lkp@intel.com
> Fixes: 9139f93209d1 ("rcu/nocb: Fix RT throttling hrtimer armed from offline CPU")
> Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>

Reviewed-by: Paul E. McKenney <paulmck@...nel.org>

> ---
>  kernel/rcu/tree_nocb.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 97b99cd06923..16865475120b 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -554,13 +554,19 @@ static void __call_rcu_nocb_wake(struct rcu_data *rdp, bool was_alldone,
>  			rcu_nocb_unlock(rdp);
>  			wake_nocb_gp_defer(rdp, RCU_NOCB_WAKE_LAZY,
>  					   TPS("WakeLazy"));
> -		} else if (!irqs_disabled_flags(flags)) {
> +		} else if (!irqs_disabled_flags(flags) && cpu_online(rdp->cpu)) {
>  			/* ... if queue was empty ... */
>  			rcu_nocb_unlock(rdp);
>  			wake_nocb_gp(rdp, false);
>  			trace_rcu_nocb_wake(rcu_state.name, rdp->cpu,
>  					    TPS("WakeEmpty"));
>  		} else {
> +			/*
> +			 * Don't do the wake-up upfront on fragile paths.
> +			 * Also offline CPUs can't call swake_up_one_online() from
> +			 * (soft-)IRQs. Rely on the final deferred wake-up from
> +			 * rcutree_report_cpu_dead()
> +			 */
>  			rcu_nocb_unlock(rdp);
>  			wake_nocb_gp_defer(rdp, RCU_NOCB_WAKE,
>  					   TPS("WakeEmptyIsDeferred"));
> -- 
> 2.46.0
>