| 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
| ||
|
Message-ID: <1da59ef5df8e8a2bebd31535fa13264113a316ff.camel@infradead.org>
Date: Tue, 24 Sep 2024 15:20:05 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: Thomas Gleixner <tglx@...utronix.de>, Peter Zijlstra
<peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, Will Deacon
<will@...nel.org>, Waiman Long <longman@...hat.com>, Boqun Feng
<boqun.feng@...il.com>, linux-kernel <linux-kernel@...r.kernel.org>
Cc: Juergen Gross <jgross@...e.com>
Subject: Re: [PATCH v3] lockdep: add lockdep_cleanup_dead_cpu()
On Mon, 2023-10-30 at 08:45 +0000, David Woodhouse wrote:
> From: David Woodhouse <dwmw@...zon.co.uk>
>
> Add a function to check that an offline CPU left the tracing infrastructure
> in a sane state. The acpi_idle_play_dead() function was recently observed¹
> calling safe_halt() instead of raw_safe_halt(), which had the side-effect
> of setting the hardirqs_enabled flag for the offline CPU. On x86 this
> triggered lockdep warnings when the CPU came back online, but too early
> for the exception to be handled correctly, leading to a triple-fault.
>
> Add lockdep_cleanup_dead_cpu() to check for this kind of failure mode,
> print the events leading up to it, and correct it so that the CPU can
> come online again correctly.
>
> [ 61.556652] smpboot: CPU 1 is now offline
> [ 61.556769] CPU 1 left hardirqs enabled!
> [ 61.556915] irq event stamp: 128149
> [ 61.556965] hardirqs last enabled at (128149): [<ffffffff81720a36>] acpi_idle_play_dead+0x46/0x70
> [ 61.557055] hardirqs last disabled at (128148): [<ffffffff81124d50>] do_idle+0x90/0xe0
> [ 61.557117] softirqs last enabled at (128078): [<ffffffff81cec74c>] __do_softirq+0x31c/0x423
> [ 61.557199] softirqs last disabled at (128065): [<ffffffff810baae1>] __irq_exit_rcu+0x91/0x100
>
> ¹ https://lore.kernel.org/lkml/a079bba5a0e47d6534b307553fc3772d26ce911b.camel@infradead.org/
>
> Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> Reviewed-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>
Ping? Found this lying around in a branch today...
> v3: Add forward declaration of struct task_struct.
>
> v2: Fix spelling. 'Offlone' wasn't quite what I meant to type.
> Add reference to ACPI patch.
> Fix kerneldoc args for lockdep_cleanup_dead_cpu() (thanks lkp)
> Closes: https://lore.kernel.org/oe-kbuild-all/202310290041.L5ndwcQ9-lkp@intel.com/
>
> include/linux/irqflags.h | 6 ++++++
> kernel/cpu.c | 1 +
> kernel/locking/lockdep.c | 24 ++++++++++++++++++++++++
> 3 files changed, 31 insertions(+)
>
> diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
> index 2b665c32f5fe..9b44f8b042a0 100644
> --- a/include/linux/irqflags.h
> +++ b/include/linux/irqflags.h
> @@ -17,6 +17,8 @@
> #include <asm/irqflags.h>
> #include <asm/percpu.h>
>
> +struct task_struct;
> +
> /* Currently lockdep_softirqs_on/off is used only by lockdep */
> #ifdef CONFIG_PROVE_LOCKING
> extern void lockdep_softirqs_on(unsigned long ip);
> @@ -24,12 +26,16 @@
> extern void lockdep_hardirqs_on_prepare(void);
> extern void lockdep_hardirqs_on(unsigned long ip);
> extern void lockdep_hardirqs_off(unsigned long ip);
> + extern void lockdep_cleanup_dead_cpu(unsigned int cpu,
> + struct task_struct *idle);
> #else
> static inline void lockdep_softirqs_on(unsigned long ip) { }
> static inline void lockdep_softirqs_off(unsigned long ip) { }
> static inline void lockdep_hardirqs_on_prepare(void) { }
> static inline void lockdep_hardirqs_on(unsigned long ip) { }
> static inline void lockdep_hardirqs_off(unsigned long ip) { }
> + static inline void lockdep_cleanup_dead_cpu(unsigned int cpu,
> + struct task_struct *idle) {}
> #endif
>
> #ifdef CONFIG_TRACE_IRQFLAGS
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 6de7c6bb74ee..225f5bc3708f 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1371,6 +1371,7 @@ static int takedown_cpu(unsigned int cpu)
>
> cpuhp_bp_sync_dead(cpu);
>
> + lockdep_cleanup_dead_cpu(cpu, idle_thread_get(cpu));
> tick_cleanup_dead_cpu(cpu);
> rcutree_migrate_callbacks(cpu);
> return 0;
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index e85b5ad3e206..62bfda8991b8 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -4538,6 +4538,30 @@ void lockdep_softirqs_off(unsigned long ip)
> debug_atomic_inc(redundant_softirqs_off);
> }
>
> +/**
> + * lockdep_cleanup_dead_cpu - Ensure CPU lockdep state is cleanly stopped
> + *
> + * @cpu: index of offlined CPU
> + * @idle: task pointer for offlined CPU's idle thread
> + *
> + * Invoked after the CPU is dead. Ensures that the tracing infrastructure
> + * is left in a suitable state for the CPU to be subsequently brought
> + * online again.
> + */
> +void lockdep_cleanup_dead_cpu(unsigned int cpu, struct task_struct *idle)
> +{
> + if (unlikely(!debug_locks))
> + return;
> +
> + if (unlikely(per_cpu(hardirqs_enabled, cpu))) {
> + pr_warn("CPU %u left hardirqs enabled!", cpu);
> + if (idle)
> + print_irqtrace_events(idle);
> + /* Clean it up for when the CPU comes online again. */
> + per_cpu(hardirqs_enabled, cpu) = 0;
> + }
> +}
> +
> static int
> mark_usage(struct task_struct *curr, struct held_lock *hlock, int check)
> {
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)
Powered by blists - more mailing lists