| Message ID | a5d3ebb600a91170fc88599d5a575452b3e31036.1617979121.git.thomas.lendacky@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] KVM: SVM: Make sure GHCB is mapped before updating | expand |
On 4/9/21 9:38 AM, Tom Lendacky wrote: > From: Tom Lendacky <thomas.lendacky@amd.com> > > Access to the GHCB is mainly in the VMGEXIT path and it is known that the > GHCB will be mapped. But there are two paths where it is possible the GHCB > might not be mapped. > > The sev_vcpu_deliver_sipi_vector() routine will update the GHCB to inform > the caller of the AP Reset Hold NAE event that a SIPI has been delivered. > However, if a SIPI is performed without a corresponding AP Reset Hold, > then the GHCB might not be mapped (depending on the previous VMEXIT), > which will result in a NULL pointer dereference. > > The svm_complete_emulated_msr() routine will update the GHCB to inform > the caller of a RDMSR/WRMSR operation about any errors. While it is likely > that the GHCB will be mapped in this situation, add a safe guard > in this path to be certain a NULL pointer dereference is not encountered. > > Fixes: f1c6366e3043 ("KVM: SVM: Add required changes to support intercepts under SEV-ES") > Fixes: 647daca25d24 ("KVM: SVM: Add support for booting APs in an SEV-ES guest") > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > > --- > > Changes from v2: > - Removed WARN_ON_ONCE() from the sev_vcpu_deliver_sipi_vector() path > since it is guest triggerable and can crash systems with panic_on_warn > and replaced with pr_warn_once(). I messed up the change-log here, the WARN_ON_ONCE() was dropped and *not* replaced with a pr_warn_once(). Thanks, Tom > > Changes from v1: > - Added the svm_complete_emulated_msr() path as suggested by Sean > Christopherson > - Add a WARN_ON_ONCE() to the sev_vcpu_deliver_sipi_vector() path > --- > arch/x86/kvm/svm/sev.c | 3 +++ > arch/x86/kvm/svm/svm.c | 2 +- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 83e00e524513..0a539f8bc212 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -2105,5 +2105,8 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector) > * the guest will set the CS and RIP. Set SW_EXIT_INFO_2 to a > * non-zero value. > */ > + if (!svm->ghcb) > + return; > + > ghcb_set_sw_exit_info_2(svm->ghcb, 1); > } > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 271196400495..534e52ba6045 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -2759,7 +2759,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, int err) > { > struct vcpu_svm *svm = to_svm(vcpu); > - if (!sev_es_guest(vcpu->kvm) || !err) > + if (!err || !sev_es_guest(vcpu->kvm) || WARN_ON_ONCE(!svm->ghcb)) > return kvm_complete_insn_gp(vcpu, err); > > ghcb_set_sw_exit_info_1(svm->ghcb, 1); >
On Fri, Apr 09, 2021, Tom Lendacky wrote: > From: Tom Lendacky <thomas.lendacky@amd.com> > > Access to the GHCB is mainly in the VMGEXIT path and it is known that the > GHCB will be mapped. But there are two paths where it is possible the GHCB > might not be mapped. > > The sev_vcpu_deliver_sipi_vector() routine will update the GHCB to inform > the caller of the AP Reset Hold NAE event that a SIPI has been delivered. > However, if a SIPI is performed without a corresponding AP Reset Hold, > then the GHCB might not be mapped (depending on the previous VMEXIT), > which will result in a NULL pointer dereference. > > The svm_complete_emulated_msr() routine will update the GHCB to inform > the caller of a RDMSR/WRMSR operation about any errors. While it is likely > that the GHCB will be mapped in this situation, add a safe guard > in this path to be certain a NULL pointer dereference is not encountered. > > Fixes: f1c6366e3043 ("KVM: SVM: Add required changes to support intercepts under SEV-ES") > Fixes: 647daca25d24 ("KVM: SVM: Add support for booting APs in an SEV-ES guest") > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > > --- Reviewed-by: Sean Christopherson <seanjc@google.com>
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 83e00e524513..0a539f8bc212 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2105,5 +2105,8 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector) * the guest will set the CS and RIP. Set SW_EXIT_INFO_2 to a * non-zero value. */ + if (!svm->ghcb) + return; + ghcb_set_sw_exit_info_2(svm->ghcb, 1); } diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 271196400495..534e52ba6045 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -2759,7 +2759,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, int err) { struct vcpu_svm *svm = to_svm(vcpu); - if (!sev_es_guest(vcpu->kvm) || !err) + if (!err || !sev_es_guest(vcpu->kvm) || WARN_ON_ONCE(!svm->ghcb)) return kvm_complete_insn_gp(vcpu, err); ghcb_set_sw_exit_info_1(svm->ghcb, 1);