diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index e0a7549a7c72..83fd49b26a4b 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -641,8 +641,8 @@ static inline void __unused_size_checks(void) #define SVM_EVTINJ_TYPE_EXEPT (3 << SVM_EVTINJ_TYPE_SHIFT) #define SVM_EVTINJ_TYPE_SOFT (4 << SVM_EVTINJ_TYPE_SHIFT) -#define SVM_EVTINJ_VALID (1 << 31) -#define SVM_EVTINJ_VALID_ERR (1 << 11) +#define SVM_EVTINJ_VALID BIT(31) +#define SVM_EVTINJ_VALID_ERR BIT(11) #define SVM_EVTINJ_RESERVED_BITS ~(SVM_EVTINJ_VEC_MASK | SVM_EVTINJ_TYPE_MASK | \ SVM_EVTINJ_VALID_ERR | SVM_EVTINJ_VALID) diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 8845f469adf3..af0ef678433d 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -1502,6 +1502,7 @@ int svm_allocate_nested(struct vcpu_svm *svm) if (!svm->nested.msrpm) goto err_free_vmcb02; + svm->nested.vmcb02.cpu = -1; svm->nested.initialized = true; return 0; diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 93fbfb7d1f65..6427db390b1c 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -4239,6 +4239,7 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_ struct kvm *kvm = svm->vcpu.kvm; struct kvm_sev_info *sev = to_kvm_sev_info(kvm); sev_ret_code fw_err = 0; + u8 tickle = 0; int ret; if (!is_sev_snp_guest(&svm->vcpu)) @@ -4246,8 +4247,21 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_ guard(mutex)(&sev->guest_req_mutex); - if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE)) - return -EIO; + if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE)) { + svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT); + return 1; + } + + /* + * "Tickle" the response buffer to verify it's writable before sending + * the request to firmware, which will modify the VMPCK sequence number. + * Note, this is a best effort check and doesn't guard against TOCTOU + * issues. See below for more information. + */ + if (kvm_write_guest(kvm, resp_gpa, &tickle, sizeof(tickle))) { + svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT); + return 1; + } data.gctx_paddr = __psp_pa(sev->snp_context); data.req_paddr = __psp_pa(sev->guest_req_buf); @@ -4262,6 +4276,13 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_ if (ret && !fw_err) return ret; + /* + * Exit to userspace if writing the response fails, e.g. if the mapping + * changed between the initial tickle and the actual write, to avoid + * creating an ambiguous failure ABI with the guest. Don't return an + * error to the guest because firmware already incremented the VMPCK + * sequence number, but there's no way to communicate that to the guest. + */ if (kvm_write_guest(kvm, resp_gpa, sev->guest_resp_buf, PAGE_SIZE)) return -EIO; @@ -4313,8 +4334,10 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r return -EINVAL; if (kvm_read_guest(kvm, req_gpa + offsetof(struct snp_guest_msg_hdr, msg_type), - &msg_type, 1)) - return -EIO; + &msg_type, 1)) { + svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT); + return 1; + } /* * As per GHCB spec, requests of type MSG_REPORT_REQ also allow for @@ -4353,8 +4376,10 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r * As per GHCB spec (see "SNP Extended Guest Request"), the * certificate table is terminated by 24-bytes of zeroes. */ - if (data_npages && kvm_clear_guest(kvm, data_gpa, 24)) - return -EIO; + if (data_npages && kvm_clear_guest(kvm, data_gpa, 24)) { + svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT); + return 1; + } } return snp_handle_guest_req(svm, req_gpa, resp_gpa); diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index fd6264f003a4..487897ef11dd 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -3875,14 +3875,25 @@ static int pre_svm_run(struct kvm_vcpu *vcpu) struct vcpu_svm *svm = to_svm(vcpu); /* - * If the previous vmrun of the vmcb occurred on a different physical - * cpu, then mark the vmcb dirty and assign a new asid. Hardware's - * vmcb clean bits are per logical CPU, as are KVM's asid assignments. + * If the previous VMRUN of the VMCB occurred on a different physical + * cpu, then mark the VMCB dirty as hardware's clean bits are per pCPU. + * + * Reset the ASID generation in both VMCBs. This will lead to assigning + * a new ASID now, and then again when switching to the other VMCB. + * However, this is needed as the ASID is shared between the VMCBs, and + * otherwise it would be possible to use an ASID allocated on one pCPU + * on another, for example: + * - vCPU migrates from pCPU A to pCPU B, allocates a new ASID. + * - vCPU migrates back to pCPU A, and then switches the VMCB. + * - The new VMCB does not detect a pCPU change and runs on pCPU A with + * the new ASID allocated on pCPU B, which is potentially used by + * another vCPU/VM. */ if (unlikely(svm->current_vmcb->cpu != vcpu->cpu)) { - svm->current_vmcb->asid_generation = 0; vmcb_mark_all_dirty(svm->vmcb); svm->current_vmcb->cpu = vcpu->cpu; + svm->vmcb01.asid_generation = 0; + svm->nested.vmcb02.asid_generation = 0; } if (is_sev_guest(vcpu))