Skip to contentSkip to conversation

Patch #4

PATCH Applied [email protected] 4 participants 7 messages Jun 20, 2018 – Jun 22, 2018

Subsystems (4)
Documentation x86 KVM x86 CPU hotplug
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 4/6 Patch #4

If the L1TF CPU bug is present we allow the KVM module to be loaded as the major of users that use Linux and KVM have trusted guests and do not want a broken setup.

Cloud vendors are the ones that are uncomfortable with CVE 2018-3615 and as such they are the ones that should set disallow_smt to one.

Setting disallow_smt to means that the system administrator also needs to disable SMT (Hyper-threading) in the BIOS, or via the 'nosmt' command line parameter, or via the /sys/devices/system/cpu/smt/control (see commit XYZ).

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 arch/x86/kvm/x86.c                              | 13 +++++++++++++
 kernel/cpu.c                                    |  1 +
 3 files changed, 18 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +4 -0
index 3a5908cd7ef7..dcc1c0313635 100644
@@ -1919,6 +1919,10 @@
 	kvm.enable_vmware_backdoor=[KVM] Support VMware backdoor PV interface.
 				   Default is false (don't support).
 
+	kvm.disallow_smt=[KVM] If the L1TF CPU bug is present and the system has
+			SMT (aka Hyper-Threading) enabled then don't load KVM module.
+			Default is 0 (allow module to be loaded).
+
 	kvm.mmu_audit=	[KVM] This is a R/W parameter which allows audit
 			KVM MMU at runtime.
 			Default is 0 (off)
arch/x86/kvm/x86.c +13 -0
index 0046aa70205a..1065d4e7c5fd 100644
@@ -150,6 +150,10 @@ EXPORT_SYMBOL_GPL(enable_vmware_backdoor);
 static bool __read_mostly force_emulation_prefix = false;
 module_param(force_emulation_prefix, bool, S_IRUGO);
 
+static bool __read_mostly disallow_smt = false;
+module_param(disallow_smt, bool, S_IRUGO);
+
+
 #define KVM_NR_SHARED_MSRS 16
 
 struct kvm_shared_msrs_global {

@@ -6555,6 +6559,15 @@ int kvm_arch_init(void *opaque)
 		goto out;
 	}
 
+	if (boot_cpu_has(X86_BUG_L1TF) && (cpu_smt_control == CPU_SMT_ENABLED)) {
+		printk(KERN_ERR "kvm: SMT enabled with L1TF CPU bug present. Refer to CVE-2018-3615 for details.\n");
+		if (disallow_smt) {
+			r = -EOPNOTSUPP;
+			goto out;
+		}
+		printk(KERN_ERR "Without disabling SMT or setting disallow_smt=1 you risk untrusted guests\n"
+				"being able to snoop the host memory!");
+	}
 	r = -ENOMEM;
 	shared_msrs = alloc_percpu(struct kvm_shared_msrs);
 	if (!shared_msrs) {
kernel/cpu.c +1 -0
index 7edf0a5cfeb9..70770308c968 100644
@@ -935,6 +935,7 @@ EXPORT_SYMBOL(cpu_down);
 
 #ifdef CONFIG_HOTPLUG_SMT
 enum cpuhp_smt_control cpu_smt_control __read_mostly = CPU_SMT_ENABLED;
+EXPORT_SYMBOL_GPL(cpu_smt_control);
 
 static int __init smt_cmdline_disable(char *str)
 {
Signature · 1 line
2.14.3
maintainer
#1 lore raw
Quoted 5 lines · speck for konrad.wilk_at_oracle.com

x86/KVM: Warn user if KVM is loaded SMT and L1TF CPU bug being present.

If the L1TF CPU bug is present we allow the KVM module to be loaded as the major of users that use Linux and KVM have trusted guests and do not want a broken setup.

Why on module load? Doesn't the module get loaded unconditionally? My desktop seems to have it loaded and I'm pretty sure i've never used KVM on it.

Would it not make more sense to do this in kvm_create_vm() ? Then only people actually using KVM get to deal with it.

author
#2 lore raw
Quoted 7 lines · speck for Peter Zijlstra
Quoted 5 lines · speck for konrad.wilk_at_oracle.com

x86/KVM: Warn user if KVM is loaded SMT and L1TF CPU bug being present.

If the L1TF CPU bug is present we allow the KVM module to be loaded as the major of users that use Linux and KVM have trusted guests and do not want a broken setup.

Quoted 6 lines

Why on module load? Doesn't the module get loaded unconditionally? My desktop seems to have it loaded and I'm pretty sure i've never used KVM on it.

Would it not make more sense to do this in kvm_create_vm() ? Then only people actually using KVM get to deal with it.

Excellent suggestion. Let me see about that. Did the text sound scary enough or should I notch it up with more scary warnings?

maintainer
#3 lore raw
In reply to Peter Zijlstra
Quoted 10 lines · speck for Peter Zijlstra
Quoted 5 lines · speck for konrad.wilk_at_oracle.com

x86/KVM: Warn user if KVM is loaded SMT and L1TF CPU bug being present.

If the L1TF CPU bug is present we allow the KVM module to be loaded as the major of users that use Linux and KVM have trusted guests and do not want a broken setup.

Why on module load? Doesn't the module get loaded unconditionally? My desktop seems to have it loaded and I'm pretty sure i've never used KVM on it.

Yes it's loaded based on cpuid.

On non-x86 systems it's only loaded if you actually use /dev/kvm, but x86 has the complication of having to load exactly one of kvm_intel and kvm_amd.

Paolo

Quoted 3 lines

Would it not make more sense to do this in kvm_create_vm() ? Then only people actually using KVM get to deal with it.

maintainer
#4 lore raw
In reply to [email protected]

arch/x86/kvm/x86.c int kvm_arch_init(void *opaque) · line 6570

Quoted 10 lines · speck for konrad.wilk_at_oracle.com
arch/x86/kvm/x86.c +9 -0
+	if (boot_cpu_has(X86_BUG_L1TF) && (cpu_smt_control == CPU_SMT_ENABLED)) {
+		printk(KERN_ERR "kvm: SMT enabled with L1TF CPU bug present. Refer to CVE-2018-3615 for details.\n");
+		if (disallow_smt) {
+			r = -EOPNOTSUPP;
+			goto out;
+		}
+		printk(KERN_ERR "Without disabling SMT or setting disallow_smt=1 you risk untrusted guests\n"
+				"being able to snoop the host memory!");
+	}

Here KERN_ERR is too much, it should be a warning at most, and I'm not sure about that either.

(Also please use pr_* instead of printk).

Quoted 1 line

r = -ENOMEM;

#5 lore raw
In reply to [email protected]

arch/x86/kvm/x86.c int kvm_arch_init(void *opaque) · line 6563

Quoted 2 lines
arch/x86/kvm/x86.c +2 -0
+	if (boot_cpu_has(X86_BUG_L1TF) && (cpu_smt_control == CPU_SMT_ENABLED)) {
+		printk(KERN_ERR "kvm: SMT enabled with L1TF CPU bug present. Refer to CVE-2018-3615 for details.\n");

Doesn't make sense if it's mitigated.

arch/x86/kvm/x86.c int kvm_arch_init(void *opaque) · line 6569

Quoted 6 lines
arch/x86/kvm/x86.c +6 -0
+		if (disallow_smt) {
+			r = -EOPNOTSUPP;
+			goto out;
+		}
+		printk(KERN_ERR "Without disabling SMT or setting disallow_smt=1 you risk untrusted guests\n"
+				"being able to snoop the host memory!");

This is not correct because the snooping can be also prevented in other ways, e.g. using task affinity, cpu sets, interrupt binding etc.

I wouldn't give a specific recipe, it's far too complicated for an kernel message.

Also all shouldn't be KERN_ERR, just _WARN

-Andi

maintainer
#6 lore raw
Quoted 3 lines · speck for Andi Kleen

arch/x86/kvm/x86.c int kvm_arch_init(void *opaque) · line 6563

Quoted 2 lines
arch/x86/kvm/x86.c +2 -0
+	if (boot_cpu_has(X86_BUG_L1TF) && (cpu_smt_control == CPU_SMT_ENABLED)) {
+		printk(KERN_ERR "kvm: SMT enabled with L1TF CPU bug present. Refer to CVE-2018-3615 for details.\n");

Doesn't make sense if it's mitigated.

The SMT case is not being mitigated. That's the case where one vCPU knows that the host pCPU sibling is causing a vmexit, and possibly Linux runs other processes during the vmexit.

KERN_ERR is okay if disallow_smt=1, since you want an explanation for the failure to load the module. If it's allowed I agree that it's by definition not an error...

Paolo

Quoted 13 lines

arch/x86/kvm/x86.c int kvm_arch_init(void *opaque) · line 6569

Quoted 6 lines
arch/x86/kvm/x86.c +6 -0
+		if (disallow_smt) {
+			r = -EOPNOTSUPP;
+			goto out;
+		}
+		printk(KERN_ERR "Without disabling SMT or setting disallow_smt=1 you risk untrusted guests\n"
+				"being able to snoop the host memory!");

This is not correct because the snooping can be also prevented in other ways, e.g. using task affinity, cpu sets, interrupt binding etc.

I wouldn't give a specific recipe, it's far too complicated for an kernel message.

Also all shouldn't be KERN_ERR, just _WARN