linux-kernel - Re: [PATCH -v4] x86/i8253: Call clockevent_i8253

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: <f457186f-2abf-4a23-8285-da876c83b7c6@riseup.net>
Date: Thu, 10 Apr 2025 13:54:21 +0200
From: "Fernando F. Mancera" <ffmancera@...eup.net>
To: Ingo Molnar <mingo@...nel.org>, Thomas Gleixner <tglx@...utronix.de>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, dwmw@...zon.co.uk,
 mhkelley@...look.com
Subject: Re: [PATCH -v4] x86/i8253: Call clockevent_i8253_disable() with
 interrupts disabled



On 01/04/2025 11:23, Ingo Molnar wrote:
> 
> I've cleaned up and simplified the changelog, see the patch below.
> 

Thank you. To me it looks good. Should I repost it?

Thanks,
Fernando.

> Thomas, does this look good to you too?
> 
> Thanks,
> 
> 	Ingo
> 
> =========================>
> From: Fernando Fernandez Mancera <ffmancera@...eup.net>
> Date: Fri, 28 Mar 2025 00:43:57 +0100
> Subject: [PATCH] x86/i8253: Call clockevent_i8253_disable() with interrupts disabled
> 
> There's a lockdep false positive warning related to i8253_lock:
> 
>    WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
>    ...
>    systemd-sleep/3324 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
>    ffffffffb2c23398 (i8253_lock){+.+.}-{2:2}, at: pcspkr_event+0x3f/0xe0 [pcspkr]
> 
>    ...
>    ... which became HARDIRQ-irq-unsafe at:
>    ...
>      lock_acquire+0xd0/0x2f0
>      _raw_spin_lock+0x30/0x40
>      clockevent_i8253_disable+0x1c/0x60
>      pit_timer_init+0x25/0x50
>      hpet_time_init+0x46/0x50
>      x86_late_time_init+0x1b/0x40
>      start_kernel+0x962/0xa00
>      x86_64_start_reservations+0x24/0x30
>      x86_64_start_kernel+0xed/0xf0
>      common_startup_64+0x13e/0x141
>    ...
> 
> Lockdep complains due pit_timer_init() using the lock in an IRQ-unsafe
> fashion, but it's a false positive, because there is no deadlock
> possible at that point due to init ordering: at the point where
> pit_timer_init() is called there is no other possible usage of
> i8253_lock because the system is still in the very early boot stage
> with no interrupts.
> 
> But in any case, pit_timer_init() should disable interrupts before
> calling clockevent_i8253_disable() out of general principle, and to
> keep lockdep working even in this scenario.
> 
> Use scoped_guard() for that, as suggested by Thomas Gleixner.
> 
> [ mingo: Cleaned up the changelog. ]
> 
> Suggested-by: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@...eup.net>
> Signed-off-by: Ingo Molnar <mingo@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Link: https://lore.kernel.org/r/20250327234357.3383-1-ffmancera@riseup.net
> ---
>   arch/x86/kernel/i8253.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/i8253.c b/arch/x86/kernel/i8253.c
> index 80e262bb627f..cb9852ad6098 100644
> --- a/arch/x86/kernel/i8253.c
> +++ b/arch/x86/kernel/i8253.c
> @@ -46,7 +46,8 @@ bool __init pit_timer_init(void)
>   		 * VMMs otherwise steal CPU time just to pointlessly waggle
>   		 * the (masked) IRQ.
>   		 */
> -		clockevent_i8253_disable();
> +		scoped_guard(irq)
> +			clockevent_i8253_disable();
>   		return false;
>   	}
>   	clockevent_i8253_init(true);