From ce6ea7b33e0075335b1eb3b227a21a98e3196e41 Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Fri, 29 May 2026 20:35:38 +0200 Subject: [PATCH] KVM: SEV: Read start/end indices of PSC requests exactly once per #VMGEXIT Rework Page State Change (PSC) handling to read the guest-provided start and end indices exactly once, at the beginning of the request. Re-reading the indices is "fine", _if_ the guest is well-behaved. KVM _should_ be safe against concurrent guest modification of the indices, but there is zero reason to introduce unnecessary risk. Reviewed-by: Tom Lendacky Reviewed-by: Michael Roth Signed-off-by: Sean Christopherson Message-ID: <20260501202250.2115252-14-seanjc@google.com> Signed-off-by: Paolo Bonzini Message-ID: <20260529183549.1104619-14-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- arch/x86/kvm/svm/sev.c | 86 +++++++++++++++++++++++------------------- arch/x86/kvm/svm/svm.h | 1 + 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 9f6543cebedf..4ebe0d449789 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -3841,7 +3841,7 @@ struct psc_buffer { struct psc_entry entries[]; } __packed; -static int snp_begin_psc(struct vcpu_svm *svm); +static int snp_do_psc(struct vcpu_svm *svm); static void snp_complete_psc(struct vcpu_svm *svm, u64 psc_ret) { @@ -3874,6 +3874,7 @@ static void __snp_complete_one_psc(struct vcpu_svm *svm) guest_psc->entries[idx].cur_page = entry.pagesize ? 512 : 1; } + sev_es->psc.cur_idx = idx; guest_psc->hdr.cur_entry = idx; } @@ -3889,37 +3890,19 @@ static int snp_complete_one_psc(struct kvm_vcpu *vcpu) __snp_complete_one_psc(svm); /* Handle the next range (if any). */ - return snp_begin_psc(svm); + return snp_do_psc(svm); } -static int snp_begin_psc(struct vcpu_svm *svm) +static int snp_do_psc(struct vcpu_svm *svm) { struct vcpu_sev_es_state *sev_es = &svm->sev_es; struct psc_buffer *guest_psc = sev_es->ghcb_sa; struct kvm_vcpu *vcpu = &svm->vcpu; struct psc_entry entry_start; - u16 idx, idx_start, idx_end, max_nr_entries; int npages; bool huge; u64 gfn; - - if (!user_exit_on_hypercall(vcpu->kvm, KVM_HC_MAP_GPA_RANGE)) { - snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC); - return 1; - } - - /* - * GHCB v2 requires the scratch area to reside within the GHCB itself, - * and PSC requests are only supported for GHCB v2+. Thus it should be - * impossible to exceed the max PSC entry count (which is derived from - * the size of the shared GHCB buffer). - */ - max_nr_entries = (sev_es->ghcb_sa_len - sizeof(struct psc_hdr)) / - sizeof(struct psc_entry); - if (WARN_ON_ONCE(max_nr_entries > VMGEXIT_PSC_MAX_COUNT)) { - snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC); - return 1; - } + u16 idx; next_range: /* There should be no other PSCs in-flight at this point. */ @@ -3928,21 +3911,8 @@ next_range: return 1; } - /* - * The PSC descriptor buffer can be modified by a misbehaved guest after - * validation, so take care to only use validated copies of values used - * for things like array indexing. - */ - idx_start = READ_ONCE(guest_psc->hdr.cur_entry); - idx_end = READ_ONCE(guest_psc->hdr.end_entry); - - if (idx_end >= max_nr_entries) { - snp_complete_psc(svm, VMGEXIT_PSC_ERROR_INVALID_HDR); - return 1; - } - /* Find the start of the next range which needs processing. */ - for (idx = idx_start; idx <= idx_end; idx++) { + for (idx = sev_es->psc.cur_idx; idx <= sev_es->psc.end_idx; idx++) { entry_start = READ_ONCE(guest_psc->entries[idx]); gfn = entry_start.gfn; @@ -3979,7 +3949,7 @@ next_range: guest_psc->hdr.cur_entry++; } - if (idx > idx_end) { + if (idx > sev_es->psc.end_idx) { /* Nothing more to process. */ snp_complete_psc(svm, 0); return 1; @@ -3994,7 +3964,7 @@ next_range: * ranges/operations and can be combined into a single * KVM_HC_MAP_GPA_RANGE exit. */ - while (++idx <= idx_end) { + while (++idx <= sev_es->psc.end_idx) { struct psc_entry entry = READ_ONCE(guest_psc->entries[idx]); if (entry.operation != entry_start.operation || @@ -4044,6 +4014,46 @@ next_range: BUG(); } +static int snp_begin_psc(struct vcpu_svm *svm) +{ + struct vcpu_sev_es_state *sev_es = &svm->sev_es; + struct psc_buffer *guest_psc = sev_es->ghcb_sa; + u16 max_nr_entries; + + if (!user_exit_on_hypercall(svm->vcpu.kvm, KVM_HC_MAP_GPA_RANGE)) { + snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC); + return 1; + } + + /* + * GHCB v2 requires the scratch area to reside within the GHCB itself, + * and PSC requests are only supported for GHCB v2+. Thus it should be + * impossible to exceed the max PSC entry count (which is derived from + * the size of the shared GHCB buffer). + */ + max_nr_entries = (sev_es->ghcb_sa_len - sizeof(struct psc_hdr)) / + sizeof(struct psc_entry); + if (WARN_ON_ONCE(max_nr_entries > VMGEXIT_PSC_MAX_COUNT)) { + snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC); + return 1; + } + + /* + * The PSC descriptor buffer can be modified by a misbehaved guest after + * validation, so take care to only use validated copies of values used + * for things like array indexing. + */ + sev_es->psc.cur_idx = READ_ONCE(guest_psc->hdr.cur_entry); + sev_es->psc.end_idx = READ_ONCE(guest_psc->hdr.end_entry); + + if (sev_es->psc.end_idx >= max_nr_entries) { + snp_complete_psc(svm, VMGEXIT_PSC_ERROR_INVALID_HDR); + return 1; + } + + return snp_do_psc(svm); +} + /* * Invoked as part of svm_vcpu_reset() processing of an init event. */ diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index 06192bc9c107..5137416be593 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -259,6 +259,7 @@ struct vcpu_sev_es_state { /* SNP Page-State-Change buffer entries currently being processed */ struct { u16 cur_idx; + u16 end_idx; u16 batch_size; bool is_2m; } psc; -- 2.47.3