linux-kernel - [PATCH] x86/kvm: Don't enable IRQ when IRQ enabled in kvm

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

Message-Id: <20210814035129.154242-1-jiangshanlai@gmail.com>
Date:   Sat, 14 Aug 2021 11:51:29 +0800
From:   Lai Jiangshan <jiangshanlai@...il.com>
To:     linux-kernel@...r.kernel.org
Cc:     Lai Jiangshan <laijs@...ux.alibaba.com>,
        Mark Rutland <mark.rutland@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        kvm@...r.kernel.org
Subject: [PATCH] x86/kvm: Don't enable IRQ when IRQ enabled in kvm_wait

From: Lai Jiangshan <laijs@...ux.alibaba.com>

Commit f4e61f0c9add3 ("x86/kvm: Fix broken irq restoration in kvm_wait")
replaced "local_irq_restore() when IRQ enabled" with "local_irq_enable()
when IRQ enabled" to suppress a warnning.

Although there is no similar debugging warnning for doing local_irq_enable()
when IRQ enabled as doing local_irq_restore() in the same IRQ situation.  But
doing local_irq_enable() when IRQ enabled is no less broken as doing
local_irq_restore() and we'd better avoid it.

Cc: Mark Rutland <mark.rutland@....com>
Cc: Peter Zijlstra (Intel) <peterz@...radead.org>
Signed-off-by: Lai Jiangshan <laijs@...ux.alibaba.com>
---

The original debugging warnning was introduced in commit 997acaf6b4b5
("lockdep: report broken irq restoration").  I think a similar debugging
check and warnning should also be added to "local_irq_enable() when IRQ
enabled" and even maybe "local_irq_disable() when IRQ disabled" to detect
something this:

    | local_irq_save(flags);
    | local_irq_disable();
    | local_irq_restore(flags);
    | local_irq_enable();

Or even we can do the check in lockdep+TRACE_IRQFLAGS:

In lockdep_hardirqs_on_prepare(), lockdep_hardirqs_enabled() was checked
(and exit) before checking DEBUG_LOCKS_WARN_ON(!irqs_disabled()), so lockdep
can't give any warning for these kind of situations.  If we did the check
in lockdep, we would have found the problem before, and we don't need
997acaf6b4b5.

Any thought? Mark? Peter?

 arch/x86/kernel/kvm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index a26643dc6bd6..b656456c3a94 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -884,10 +884,11 @@ static void kvm_wait(u8 *ptr, u8 val)
 	} else {
 		local_irq_disable();

+		/* safe_halt() will enable IRQ */
 		if (READ_ONCE(*ptr) == val)
 			safe_halt();
-
-		local_irq_enable();
+		else
+			local_irq_enable();
 	}
 }

-- 
2.19.1.6.gb485710b