linux-kernel - Re: [PATCH v3] lockdep: add lockdep_cleanup_dead

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: <53d3d2e143eac428312ea74c8e6209aef1737a63.camel@infradead.org>
Date: Thu, 26 Sep 2024 15:34:57 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: Boqun Feng <boqun.feng@...il.com>
Cc: 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>,  linux-kernel
 <linux-kernel@...r.kernel.org>, Juergen Gross <jgross@...e.com>
Subject: Re: [PATCH v3] lockdep: add lockdep_cleanup_dead_cpu()

On Thu, 2024-09-26 at 05:34 -0700, Boqun Feng wrote:
> On Thu, Sep 26, 2024 at 01:16:32PM +0100, David Woodhouse wrote:
> > On Thu, 2024-09-26 at 05:09 -0700, Boqun Feng wrote:
> > > 
> > > 
> > > I think this is already fixed by:
> > > 
> > >         9bb69ba4c177 ("ACPI: processor_idle: use raw_safe_halt() in acpi_idle_play_dead()")
> > > 
> > > , no?
> > 
> > That patch fixed the bug.
> > 
> > *This* patch fixes the fact that lockdep didn't *tell* us about the bug.
> 
> But I thought along with the above commit, Peter also made it possible
> that objtool can detect leaving noinstr section in the offline path? Do
> you have a case where you can alter hardirqs_enabled flag in offline
> path but don't hit the objtool warning?

I do not recall such. Peter?

IIRC the bug fixed by commit 9bb69ba4c177 only showed up under real
Xen, as QEMU doesn't expose processor C-states. So I reintroduced the
equivalent bug by doing this instead:

--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1390,7 +1390,7 @@ void __noreturn hlt_play_dead(void)
                wbinvd();
 
        while (1)
-               native_halt();
+               safe_halt();
 }
 

Without this patch, I get a triple-fault on bringing the CPU back
online as before. With it, as intended, I get a warning, but success:

[root@...alhost ~]# echo 0 > /sys/devices/system/cpu/cpu1/online 
[   42.090839] smpboot: CPU 1 is now offline
[   42.091989] CPU 1 left hardirqs enabled!
[   42.091997] irq event stamp: 144559
[   42.094155] hardirqs last  enabled at (144559): [<ffffffff89098b2e>] hlt_play_dead+0x1e/0x30
[   42.096196] hardirqs last disabled at (144558): [<ffffffff891800ee>] do_idle+0xae/0x260
[   42.098062] softirqs last  enabled at (144530): [<ffffffff89107260>] __irq_exit_rcu+0xb0/0xd0
[   42.100056] softirqs last disabled at (144519): [<ffffffff89107260>] __irq_exit_rcu+0xb0/0xd0
[root@...alhost ~]# echo 1 > /sys/devices/system/cpu/cpu1/online 
[   47.480889] installing Xen timer for CPU 1
[   47.485308] smpboot: Booting Node 0 Processor 1 APIC 0x1
[   47.491569] cpu 1 spinlock event irq 35


So I think the patch is still applicable.

> Anyway, the commit log needs a rework.

Sure. Other than to refer to commit 9bb69ba4c177 instead of the mailing
list message, is there anything else that needs changing? I suppose I
should drop the word 'recently' from '...was recently observed'? :)


Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)