vCPU creation in kvm_vm_ioctl_create_vcpu() may fail after kvm_arch_vcpu_create() -> vmx_vcpu_create() already succeeded. In such case kvm_vm_ioctl_create_vcpu() destroys the newly created vCPU in the failure path. However, that leaves a side effect: the IPIv pid_table entry remains configured with this vCPU's pi_desc address. As a result, when another vCPU sends an IPI to the APIC ID of this failed-to-create vCPU, it will cause HW to write to this (freed!) pi_desc memory. [*] An easy fix would be to clear the pid_table entry in vmx_vcpu_free(). However that would be still problematic, for the following reason: userspace may try to create a vCPU with the same vcpu_id as an existing one; vmx_vcpu_create() will succeed, and only after that kvm_vm_ioctl_create_vcpu() will check for the duplicate vcpu_id and fail with -EEXIST and then free the vCPU in the failure path. So in this failure path, vmx_vcpu_free() would clear the pid_table entry for that already existing good vCPU, i.e. effectively disable IPIv for that vCPU. So instead fix the issue by moving the pid_table entry setup from .vcpu_create() to the newly introduced .vcpu_postcreate() callback called at the end of kvm_vm_ioctl_create_vcpu(), when we are sure that the vCPU creation succeeded. [*] Although, since this memory is freed into the kvm_vcpu_cache kmem cache which is only used for allocating kvm_vcpus, _maybe_ this memory will only be reused for pi_desc of another vCPU, not for anything else. So _maybe_ this will only result in delivering the IPI to a wrong vCPU (possibly of another VM) in the worst case, not in a random corruption of kernel memory. Fixes: d588bb9be1da ("KVM: VMX: enable IPI virtualization") Signed-off-by: Dmytro Maluka --- arch/x86/include/asm/kvm-x86-ops.h | 1 + arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/vmx/main.c | 9 +++++++++ arch/x86/kvm/vmx/vmx.c | 11 +++++++---- arch/x86/kvm/vmx/x86_ops.h | 1 + arch/x86/kvm/x86.c | 1 + 6 files changed, 20 insertions(+), 4 deletions(-) diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h index 83dc5086138b..aecbe9c54004 100644 --- a/arch/x86/include/asm/kvm-x86-ops.h +++ b/arch/x86/include/asm/kvm-x86-ops.h @@ -25,6 +25,7 @@ KVM_X86_OP_OPTIONAL(vm_destroy) KVM_X86_OP_OPTIONAL(vm_pre_destroy) KVM_X86_OP_OPTIONAL_RET0(vcpu_precreate) KVM_X86_OP(vcpu_create) +KVM_X86_OP_OPTIONAL(vcpu_postcreate) KVM_X86_OP(vcpu_free) KVM_X86_OP(vcpu_reset) KVM_X86_OP(prepare_switch_to_guest) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 5f6c1ce9673b..3ed9d8fc3b96 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1803,6 +1803,7 @@ struct kvm_x86_ops { /* Create, but do not attach this VCPU */ int (*vcpu_precreate)(struct kvm *kvm); int (*vcpu_create)(struct kvm_vcpu *vcpu); + void (*vcpu_postcreate)(struct kvm_vcpu *vcpu); void (*vcpu_free)(struct kvm_vcpu *vcpu); void (*vcpu_reset)(struct kvm_vcpu *vcpu, bool init_event); diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index 83d9921277ea..bdf5ba3155f6 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -78,6 +78,14 @@ static int vt_vcpu_create(struct kvm_vcpu *vcpu) return vmx_vcpu_create(vcpu); } +static void vt_vcpu_postcreate(struct kvm_vcpu *vcpu) +{ + if (is_td_vcpu(vcpu)) + return; + + vmx_vcpu_postcreate(vcpu); +} + static void vt_vcpu_free(struct kvm_vcpu *vcpu) { if (is_td_vcpu(vcpu)) { @@ -898,6 +906,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .vcpu_precreate = vt_op(vcpu_precreate), .vcpu_create = vt_op(vcpu_create), + .vcpu_postcreate = vt_op(vcpu_postcreate), .vcpu_free = vt_op(vcpu_free), .vcpu_reset = vt_op(vcpu_reset), diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index cc75feec05da..ae6cef9c13e0 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7752,10 +7752,6 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu) vmx->ve_info = page_to_virt(page); } - if (vmx_can_use_ipiv(vcpu)) - WRITE_ONCE(to_kvm_vmx(vcpu->kvm)->pid_table[vcpu->vcpu_id], - __pa(&vmx->vt.pi_desc) | PID_TABLE_ENTRY_VALID); - return 0; free_vmcs: @@ -7767,6 +7763,13 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu) return err; } +void vmx_vcpu_postcreate(struct kvm_vcpu *vcpu) +{ + if (vmx_can_use_ipiv(vcpu)) + WRITE_ONCE(to_kvm_vmx(vcpu->kvm)->pid_table[vcpu->vcpu_id], + __pa(&to_vmx(vcpu)->vt.pi_desc) | PID_TABLE_ENTRY_VALID); +} + #define L1TF_MSG_SMT "L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n" #define L1TF_MSG_L1D "L1TF CPU bug present and virtualization mitigation disabled, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n" diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index 409858074246..defc2c3a37c8 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -21,6 +21,7 @@ int vmx_vm_init(struct kvm *kvm); void vmx_vm_destroy(struct kvm *kvm); int vmx_vcpu_precreate(struct kvm *kvm); int vmx_vcpu_create(struct kvm_vcpu *vcpu); +void vmx_vcpu_postcreate(struct kvm_vcpu *vcpu); int vmx_vcpu_pre_run(struct kvm_vcpu *vcpu); fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags); void vmx_vcpu_free(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index afcac1042947..c9f512a30fec 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -12873,6 +12873,7 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) return; vcpu_load(vcpu); kvm_synchronize_tsc(vcpu, NULL); + kvm_x86_call(vcpu_postcreate)(vcpu); vcpu_put(vcpu); /* poll control enabled by default */ -- 2.55.0.141.g00534a21ce-goog