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  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]
Date:   Wed, 29 Nov 2017 22:23:42 +0100
From:   Radim Krčmář <rkrcmar@...hat.com>
To:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Borislav Petkov <bp@...en8.de>, Alexander Graf <agraf@...e.de>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jan H. Schönherr <jschoenh@...zon.de>,
        KarimAllah Ahmed <karahmed@...zon.de>,
        Jim Mattson <jmattson@...gle.com>
Subject: [PATCH v2 2/3] KVM: x86: drop bogus MWAIT check

The check was added in some iteration while trying to fix a reported OS
X on Core 2 bug, but that bug is elsewhere.

The comment is misleading because the guest can call MWAIT with ECX = 0
even if we enforce CPUID5_ECX_INTERRUPT_BREAK;  the call would have the
exactly the same effect as if the host didn't have the feature.

A problem is that a QEMU feature exposes CPUID5_ECX_INTERRUPT_BREAK on
CPUs that do not support it.  Removing the check changes behavior on
last Pentium 4 lines (Presler, Dempsey, and Tulsa, which had VMX and
MONITOR while missing INTERRUPT_BREAK) when running a guest OS that uses
MWAIT without checking for its presence (QEMU doesn't expose MONITOR).

The only known OS that ignores the MONITOR flag is old Mac OS X and we
allowed it to bug on Core 2 (MWAIT used to throw #UD and only that OS
noticed), so we can save another 20 lines letting it bug on even older
CPUs.  Alternatively, we can return MWAIT exiting by default and let
userspace toggle it.

Signed-off-by: Radim Krčmář <rkrcmar@...hat.com>
---
 arch/x86/kvm/x86.h | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 81f5f50794f6..d15859ec5e92 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -265,8 +265,6 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
 
 static inline bool kvm_mwait_in_guest(void)
 {
-	unsigned int eax, ebx, ecx, edx;
-
 	if (!cpu_has(&boot_cpu_data, X86_FEATURE_MWAIT))
 		return false;
 
@@ -275,29 +273,10 @@ static inline bool kvm_mwait_in_guest(void)
 		/* All AMD CPUs have a working MWAIT implementation */
 		return true;
 	case X86_VENDOR_INTEL:
-		/* Handle Intel below */
-		break;
+		return !boot_cpu_has_bug(X86_BUG_MONITOR);
 	default:
 		return false;
 	}
-
-	if (boot_cpu_has_bug(X86_BUG_MONITOR))
-		return false;
-
-	/*
-	 * Intel CPUs without CPUID5_ECX_INTERRUPT_BREAK are problematic as
-	 * they would allow guest to stop the CPU completely by disabling
-	 * interrupts then invoking MWAIT.
-	 */
-	if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
-		return false;
-
-	cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
-
-	if (!(ecx & CPUID5_ECX_INTERRUPT_BREAK))
-		return false;
-
-	return true;
 }
 
 #endif
-- 
2.14.2

Powered by blists - more mailing lists