All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peter Gonda <pgonda@google.com>, Marc Orr <marcorr@google.com>,
	Nathan Tempelman <natet@google.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Tom Lendacky <thomas.lendacky@amd.com>
Subject: [PATCH 2/6] KVM: SEV: Explicitly document that there are no TOCTOU races in copy ASID
Date: Tue,  9 Nov 2021 21:50:57 +0000	[thread overview]
Message-ID: <20211109215101.2211373-3-seanjc@google.com> (raw)
In-Reply-To: <20211109215101.2211373-1-seanjc@google.com>

Deliberately grab the source's SEV info for COPY_ENC_CONTEXT_FROM outside
of kvm->lock and document that doing so is safe due to SEV/SEV-ES info,
e.g. ASID, active, etc... being "write-once" and set atomically with
respect to kvm->lock.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index eeec499e4372..6d14e2595c96 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1737,9 +1737,9 @@ int svm_unregister_enc_region(struct kvm *kvm,
 
 int svm_vm_copy_asid_from(struct kvm *kvm, unsigned int source_fd)
 {
+	struct kvm_sev_info *mirror_sev, *source_sev;
 	struct file *source_kvm_file;
 	struct kvm *source_kvm;
-	struct kvm_sev_info source_sev, *mirror_sev;
 	int ret;
 
 	source_kvm_file = fget(source_fd);
@@ -1762,9 +1762,6 @@ int svm_vm_copy_asid_from(struct kvm *kvm, unsigned int source_fd)
 		goto e_source_unlock;
 	}
 
-	memcpy(&source_sev, &to_kvm_svm(source_kvm)->sev_info,
-	       sizeof(source_sev));
-
 	/*
 	 * The mirror kvm holds an enc_context_owner ref so its asid can't
 	 * disappear until we're done with it
@@ -1785,14 +1782,25 @@ int svm_vm_copy_asid_from(struct kvm *kvm, unsigned int source_fd)
 		goto e_mirror_unlock;
 	}
 
+	/*
+	 * Referencing the source's sev_info without holding the source's lock
+	 * is safe as SEV/SEV-ES activation is a one-way, "atomic" operation.
+	 * SEV state, e.g. the ASID, is modified under kvm->lock, and cannot be
+	 * changed after SEV is marked active (here or in normal activation).
+	 * That same atomicity also prevents TOC-TOU issues with respect to
+	 * related sanity checks on source_kvm.
+	 */
+	source_sev = &to_kvm_svm(source_kvm)->sev_info;
+
 	/* Set enc_context_owner and copy its encryption context over */
 	mirror_sev = &to_kvm_svm(kvm)->sev_info;
 	mirror_sev->enc_context_owner = source_kvm;
+	mirror_sev->asid = source_sev->asid;
 	mirror_sev->active = true;
-	mirror_sev->asid = source_sev.asid;
-	mirror_sev->fd = source_sev.fd;
-	mirror_sev->es_active = source_sev.es_active;
-	mirror_sev->handle = source_sev.handle;
+	mirror_sev->asid = source_sev->asid;
+	mirror_sev->fd = source_sev->fd;
+	mirror_sev->es_active = source_sev->es_active;
+	mirror_sev->handle = source_sev->handle;
 	/*
 	 * Do not copy ap_jump_table. Since the mirror does not share the same
 	 * KVM contexts as the original, and they may have different
-- 
2.34.0.rc0.344.g81b53c2807-goog


  parent reply	other threads:[~2021-11-09 21:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-09 21:50 [PATCH 0/6] KVM: SEV: Bug fix, cleanups and enhancements Sean Christopherson
2021-11-09 21:50 ` [PATCH 1/6] KVM: SEV: Disallow COPY_ENC_CONTEXT_FROM if target has created vCPUs Sean Christopherson
2021-11-15 19:51   ` Peter Gonda
2021-11-15 23:35     ` Sean Christopherson
2021-11-09 21:50 ` Sean Christopherson [this message]
2021-11-16 11:38   ` [PATCH 2/6] KVM: SEV: Explicitly document that there are no TOCTOU races in copy ASID Paolo Bonzini
2021-11-09 21:50 ` [PATCH 3/6] KVM: SEV: Set sev_info.active after initial checks in sev_guest_init() Sean Christopherson
2021-11-09 21:50 ` [PATCH 4/6] KVM: SEV: WARN if SEV-ES is marked active but SEV is not Sean Christopherson
2021-11-09 21:51 ` [PATCH 5/6] KVM: SEV: Drop a redundant setting of sev->asid during initialization Sean Christopherson
2021-11-09 21:51 ` [PATCH 6/6] KVM: SEV: Fix typo in and tweak name of cmd_allowed_from_miror() Sean Christopherson
2021-11-16 12:35 ` [PATCH 0/6] KVM: SEV: Bug fix, cleanups and enhancements Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211109215101.2211373-3-seanjc@google.com \
    --to=seanjc@google.com \
    --cc=brijesh.singh@amd.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcorr@google.com \
    --cc=natet@google.com \
    --cc=pbonzini@redhat.com \
    --cc=pgonda@google.com \
    --cc=thomas.lendacky@amd.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.