* [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes
@ 2021-03-31 3:19 Sean Christopherson
2021-03-31 3:19 ` [PATCH 1/3] KVM: SVM: Use online_vcpus, not created_vcpus, to iterate over vCPUs Sean Christopherson
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Sean Christopherson @ 2021-03-31 3:19 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel, Brijesh Singh, Tom Lendacky
Misc bug fixes in SEV/SEV-ES to protect against a malicious userspace.
All found by inspection, I didn't actually crash the host to to prove that
userspace could hose the kernel in any of these cases. Boot tested an SEV
guest, though the SEV-ES side of patch 2 is essentially untested as I
don't have an SEV-ES setup at this time.
Sean Christopherson (3):
KVM: SVM: Use online_vcpus, not created_vcpus, to iterate over vCPUs
KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes
KVM: SVM: Do not allow SEV/SEV-ES initialization after vCPUs are
created
arch/x86/kvm/svm/sev.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
--
2.31.0.291.g576ba9dcdaf-goog
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] KVM: SVM: Use online_vcpus, not created_vcpus, to iterate over vCPUs
2021-03-31 3:19 [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Sean Christopherson
@ 2021-03-31 3:19 ` Sean Christopherson
2021-03-31 3:19 ` [PATCH 2/3] KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes Sean Christopherson
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2021-03-31 3:19 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel, Brijesh Singh, Tom Lendacky
Use the kvm_for_each_vcpu() helper to iterate over vCPUs when encrypting
VMSAs for SEV, which effectively switches to use online_vcpus instead of
created_vcpus. This fixes a possible null-pointer dereference as
created_vcpus does not guarantee a vCPU exists, since it is updated at
the very beginning of KVM_CREATE_VCPU. created_vcpus exists to allow the
bulk of vCPU creation to run in parallel, while still correctly
restricting the max number of max vCPUs.
Fixes: ad73109ae7ec ("KVM: SVM: Provide support to launch and run an SEV-ES guest")
Cc: stable@vger.kernel.org
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 83e00e524513..6481d7165701 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -564,6 +564,7 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
{
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
struct sev_data_launch_update_vmsa *vmsa;
+ struct kvm_vcpu *vcpu;
int i, ret;
if (!sev_es_guest(kvm))
@@ -573,8 +574,8 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
if (!vmsa)
return -ENOMEM;
- for (i = 0; i < kvm->created_vcpus; i++) {
- struct vcpu_svm *svm = to_svm(kvm->vcpus[i]);
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ struct vcpu_svm *svm = to_svm(vcpu);
/* Perform some pre-encryption checks against the VMSA */
ret = sev_es_sync_vmsa(svm);
--
2.31.0.291.g576ba9dcdaf-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes
2021-03-31 3:19 [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Sean Christopherson
2021-03-31 3:19 ` [PATCH 1/3] KVM: SVM: Use online_vcpus, not created_vcpus, to iterate over vCPUs Sean Christopherson
@ 2021-03-31 3:19 ` Sean Christopherson
2021-03-31 3:19 ` [PATCH 3/3] KVM: SVM: Do not allow SEV/SEV-ES initialization after vCPUs are created Sean Christopherson
2021-03-31 9:37 ` [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Paolo Bonzini
3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2021-03-31 3:19 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel, Brijesh Singh, Tom Lendacky
Set sev->es_active only after the guts of KVM_SEV_ES_INIT succeeds. If
the command fails, e.g. because SEV is already active or there are no
available ASIDs, then es_active will be left set even though the VM is
not fully SEV-ES capable.
Refactor the code so that "es_active" is passed on the stack instead of
being prematurely shoved into sev_info, both to avoid having to unwind
sev_info and so that it's more obvious what actually consumes es_active
in sev_guest_init() and its helpers.
Fixes: ad73109ae7ec ("KVM: SVM: Provide support to launch and run an SEV-ES guest")
Cc: stable@vger.kernel.org
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6481d7165701..97d42a007b86 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -87,7 +87,7 @@ static bool __sev_recycle_asids(int min_asid, int max_asid)
return true;
}
-static int sev_asid_new(struct kvm_sev_info *sev)
+static int sev_asid_new(bool es_active)
{
int pos, min_asid, max_asid;
bool retry = true;
@@ -98,8 +98,8 @@ static int sev_asid_new(struct kvm_sev_info *sev)
* SEV-enabled guests must use asid from min_sev_asid to max_sev_asid.
* SEV-ES-enabled guest can use from 1 to min_sev_asid - 1.
*/
- min_asid = sev->es_active ? 0 : min_sev_asid - 1;
- max_asid = sev->es_active ? min_sev_asid - 1 : max_sev_asid;
+ min_asid = es_active ? 0 : min_sev_asid - 1;
+ max_asid = es_active ? min_sev_asid - 1 : max_sev_asid;
again:
pos = find_next_zero_bit(sev_asid_bitmap, max_sev_asid, min_asid);
if (pos >= max_asid) {
@@ -179,13 +179,14 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)
static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
{
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ bool es_active = argp->id == KVM_SEV_ES_INIT;
int asid, ret;
ret = -EBUSY;
if (unlikely(sev->active))
return ret;
- asid = sev_asid_new(sev);
+ asid = sev_asid_new(es_active);
if (asid < 0)
return ret;
@@ -194,6 +195,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
goto e_free;
sev->active = true;
+ sev->es_active = es_active;
sev->asid = asid;
INIT_LIST_HEAD(&sev->regions_list);
@@ -204,16 +206,6 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
-static int sev_es_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
-{
- if (!sev_es)
- return -ENOTTY;
-
- to_kvm_svm(kvm)->sev_info.es_active = true;
-
- return sev_guest_init(kvm, argp);
-}
-
static int sev_bind_asid(struct kvm *kvm, unsigned int handle, int *error)
{
struct sev_data_activate *data;
@@ -1128,12 +1120,15 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
mutex_lock(&kvm->lock);
switch (sev_cmd.id) {
+ case KVM_SEV_ES_INIT:
+ if (!sev_es) {
+ r = -ENOTTY;
+ goto out;
+ }
+ fallthrough;
case KVM_SEV_INIT:
r = sev_guest_init(kvm, &sev_cmd);
break;
- case KVM_SEV_ES_INIT:
- r = sev_es_guest_init(kvm, &sev_cmd);
- break;
case KVM_SEV_LAUNCH_START:
r = sev_launch_start(kvm, &sev_cmd);
break;
--
2.31.0.291.g576ba9dcdaf-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] KVM: SVM: Do not allow SEV/SEV-ES initialization after vCPUs are created
2021-03-31 3:19 [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Sean Christopherson
2021-03-31 3:19 ` [PATCH 1/3] KVM: SVM: Use online_vcpus, not created_vcpus, to iterate over vCPUs Sean Christopherson
2021-03-31 3:19 ` [PATCH 2/3] KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes Sean Christopherson
@ 2021-03-31 3:19 ` Sean Christopherson
2021-03-31 9:37 ` [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Paolo Bonzini
3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2021-03-31 3:19 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel, Brijesh Singh, Tom Lendacky
Reject KVM_SEV_INIT and KVM_SEV_ES_INIT if they are attempted after one
or more vCPUs have been created. KVM assumes a VM is tagged SEV/SEV-ES
prior to vCPU creation, e.g. init_vmcb() needs to mark the VMCB as SEV
enabled, and svm_create_vcpu() needs to allocate the VMSA. At best,
creating vCPUs before SEV/SEV-ES init will lead to unexpected errors
and/or behavior, and at worst it will crash the host, e.g.
sev_launch_update_vmsa() will dereference a null svm->vmsa pointer.
Fixes: 1654efcbc431 ("KVM: SVM: Add KVM_SEV_INIT command")
Fixes: ad73109ae7ec ("KVM: SVM: Provide support to launch and run an SEV-ES guest")
Cc: stable@vger.kernel.org
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 97d42a007b86..824bc7d22e77 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -182,6 +182,9 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
bool es_active = argp->id == KVM_SEV_ES_INIT;
int asid, ret;
+ if (kvm->created_vcpus)
+ return -EINVAL;
+
ret = -EBUSY;
if (unlikely(sev->active))
return ret;
--
2.31.0.291.g576ba9dcdaf-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes
2021-03-31 3:19 [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Sean Christopherson
` (2 preceding siblings ...)
2021-03-31 3:19 ` [PATCH 3/3] KVM: SVM: Do not allow SEV/SEV-ES initialization after vCPUs are created Sean Christopherson
@ 2021-03-31 9:37 ` Paolo Bonzini
3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2021-03-31 9:37 UTC (permalink / raw)
To: Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel, Brijesh Singh, Tom Lendacky
On 31/03/21 05:19, Sean Christopherson wrote:
> Misc bug fixes in SEV/SEV-ES to protect against a malicious userspace.
> All found by inspection, I didn't actually crash the host to to prove that
> userspace could hose the kernel in any of these cases. Boot tested an SEV
> guest, though the SEV-ES side of patch 2 is essentially untested as I
> don't have an SEV-ES setup at this time.
>
> Sean Christopherson (3):
> KVM: SVM: Use online_vcpus, not created_vcpus, to iterate over vCPUs
> KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes
> KVM: SVM: Do not allow SEV/SEV-ES initialization after vCPUs are
> created
>
> arch/x86/kvm/svm/sev.c | 37 ++++++++++++++++++-------------------
> 1 file changed, 18 insertions(+), 19 deletions(-)
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-31 9:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31 3:19 [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Sean Christopherson
2021-03-31 3:19 ` [PATCH 1/3] KVM: SVM: Use online_vcpus, not created_vcpus, to iterate over vCPUs Sean Christopherson
2021-03-31 3:19 ` [PATCH 2/3] KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes Sean Christopherson
2021-03-31 3:19 ` [PATCH 3/3] KVM: SVM: Do not allow SEV/SEV-ES initialization after vCPUs are created Sean Christopherson
2021-03-31 9:37 ` [PATCH 0/3] KVM: SVM: SEV{-ES} bug fixes Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).