From c5991239b148d40d4981901968b3a6a838ff517b Mon Sep 17 00:00:00 2001 From: Chaithanya Lagisetty Date: Tue, 8 Sep 2026 05:37:22 +0000 Subject: [PATCH 1/3] x86/svm: Avoid sign extension of SVM_EVTINJ_VALID in 64-bit expressions SVM_EVTINJ_VALID is defined as (1 << 31), i.e. as an int with the sign bit set. Whenever it is used in a 64-bit expression the int is sign extended, so bits 63:32 end up set as well. Two 64-bit users are affected. verify_exception_info() uses the macro as a mask against a 64-bit value when it sanity checks hypervisor supplied exception information: u64 info = ghcb->save.sw_exit_info_2; ... if ((info & SVM_EVTINJ_VALID) && The mask is 0xffffffff80000000, so the check also passes when any bit of the error code in 63:32 is set while the valid bit itself is clear. svm_vmgexit_inject_exception() builds the value that KVM hands to an SEV-ES guest in GHCB SW_EXITINFO2: u64 data = SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_EXEPT | vector; For vector == X86_TRAP_GP this yields 0xffffffff8000030d instead of 0x000000008000030d. The field uses the EVENTINJ format, which holds the error code in bits 63:32, so KVM asks the guest to inject #GP with an error code of 0xffffffff. Linux guests only consume the error code when SVM_EVTINJ_VALID_ERR is set, which it is not here, so they are not affected in practice, but the value KVM writes is not the one it intends. Use BIT() instead, as is already done for most single bit definitions in this header. Every other user applies the macro to a u32, where the resulting bits are unchanged. Opportunistically give SVM_EVTINJ_VALID_ERR the same treatment, there's zero reason to open code BIT(). Fixes: 597cfe48212a ("x86/boot/compressed/64: Setup a GHCB-based VC Exception handler") Fixes: c3392d0ab714 ("KVM: SVM: Provide helpers to set the error code") Signed-off-by: Chaithanya Lagisetty Link: https://patch.msgid.link/20260908053722.3917934-1-nagachaithanya9911@gmail.com [sean: use BIT() for SVM_EVTINJ_VALID_ERR as well] Signed-off-by: Sean Christopherson --- arch/x86/include/asm/svm.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index aa63431ba92c..e883241b5e1e 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -637,8 +637,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) From bf260dc9e7b8bfcd06c1a8e739797b1cf0917c72 Mon Sep 17 00:00:00 2001 From: Yosry Ahmed Date: Thu, 3 Sep 2026 22:32:58 +0000 Subject: [PATCH 2/3] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch When the pCPU where the VMCB was most recently used is switched, reset the ASID generation in both VMCBs, triggering new ASID allocation for the immediate VMRUN as well as the next VMRUN on the other VMCB. The ASID is shared between vmcb01 and vmcb02, and gets flushed on every nested transition. However, since pCPU tracking is done per VMCB, it is possible for one VMCB to allocate a new ASID when migrated to a new pCPU, and then the other VMCB reuses that ASID on the old pCPU. This can result in the same ASID being used by multiple vCPUs on the old pCPU. Example scenario: - vCPU runs on pCPU A, vmcb01 is active, asid=1. - vCPU migrates to pCPU B, vmcb01 pCPU changes, new asid=2. - Another vCPU runs on pCPU A and allocates asid=2 as well. - vCPU migrates back to pCPU A, and then switches to vmcb02 before it runs again with vmcb01. - No pCPU switch is detected for vmcb02, so VMRUN is done with asid=2. - Two vCPUs end up using asid=2 on the same pCPU. Keep the VMCB dirtying to the active VMCB only. Clean bits are tracked by a pCPU for each VMCB, so do not unnecessarily dirty a VMCB if its pCPU does not change. Additionally, initialize the tracked pCPU for vmcb02 to -1 on nested enablement as hardening, so that a new ASID allocation is always triggered when nested is disabled and re-enabled. No performance regression was noticed when overcommitting L1 vCPUs in L0 (to force rescheduling), pinning L1 <-> L2 vCPUs, and running CPUID in a tight loop bouncing between 2 vCPUs in L2. An alternative (and perhaps more proper) fix would be tracking the ASID per-VMCB instead (e.g. [1]). However, that's a more involved change, and it would result in having different ASIDs for L1 and L2 without actually properly maintaining them. It would probably work because all TLB flushes target the current VMCB, and the other VMCB is always flushed on nested transitions, but the code ends up in an arguably more fragile state. Punt a proper clean fix to an incoming (and overdue) overhaul of SVM's ASID usage [2]. [1] https://lore.kernel.org/lkml/20250205182402.2147495-2-yosry.ahmed@linux.dev [2] https://lore.kernel.org/kvm/20260728003557.1136583-1-yosry@kernel.org Cc: stable@vger.kernel.org Reported-by: Stefan Teodorescu Signed-off-by: Yosry Ahmed Reviewed-by: Nikunj A Dadhania Link: https://patch.msgid.link/20260903223258.486034-1-yosry@kernel.org Signed-off-by: Sean Christopherson --- arch/x86/kvm/svm/nested.c | 1 + arch/x86/kvm/svm/svm.c | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 73f37b050d0a..0c55c71fc601 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -1494,6 +1494,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/svm.c b/arch/x86/kvm/svm/svm.c index 7d59d301e1e5..2332814cd203 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -3765,14 +3765,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)) From 003892d6e802a1c08ad03e8ea793453465cae7f4 Mon Sep 17 00:00:00 2001 From: Jacky Li Date: Wed, 23 Sep 2026 20:18:12 +0000 Subject: [PATCH 3/3] KVM: SEV: Return INVALID_INPUT on SNP req/resp buffer access failure Currently, snp_handle_(ext_)guest_req() returns -EIO when kvm_{read/clear}_guest() fails while accessing guest-provided buffers. Returning -EIO causes KVM_RUN to exit to userspace, likely killing the VM. Fix this by returning GHCB_HV_RESP_MALFORMED_INPUT with sub-error code GHCB_ERR_INVALID_INPUT to the guest and resuming the vCPU. Per the GHCB specification, guest-provided GPA buffers that cannot be accessed by the hypervisor (e.g. private pages) should be treated as guest input errors. Because kvm_{read/write/clear}_guest() only returns -EFAULT on failure, treating this failure as an invalid input aligns with the definition of -EFAULT ("Bad address"). Returning GHCB_ERR_INVALID_INPUT also matches existing SNP handling in KVM, which already returns this error code for unaligned or overlapping buffers. It also aligns with other hypercall implementations in KVM (e.g. Hyper-V returning INVALID_HYPERCALL_INPUT on kvm_read_guest() failures in kvm_hv_flush_tlb()). Additionally, tickle the response buffer before issuing the PSP command so that an invalid resp_gpa fails upfront with GHCB_ERR_INVALID_INPUT, i.e. to avoid terminating the guest if it provides a bad buffer. Do not return an error to the guest if the post-firmware write fails, as the firmware has already incremented the VMPCK sequence number. Exiting to userspace avoids creating an ambiguous failure where the guest has no way to know whether to advance its VMPCK sequence number. I.e. the "tickle" is a best effort compromise between terminating the guest on bad input, and horribly confusing the guest on bad *host userspace* behavior. Fixes: 88caf544c930 ("KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event") Fixes: 74458e4859d8 ("KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event") Signed-off-by: Jacky Li Link: https://patch.msgid.link/20260923-snp-invalid-input-v2-1-c00e0dd077ca@google.com [sean: massage comments and changelog regarding the tickle] Signed-off-by: Sean Christopherson --- arch/x86/kvm/svm/sev.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 5705723f1f41..7a1563cb471a 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -4221,6 +4221,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)) @@ -4228,8 +4229,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); @@ -4244,6 +4258,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; @@ -4295,8 +4316,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 @@ -4335,8 +4358,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);