From: Sean Christopherson <seanjc@google.com>
To: Jethro Beekman <jethro@fortanix.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-coco@lists.linux.dev
Subject: Re: [PATCH] KVM: SEV: Track SNP launch state and disallow invalid userspace interactions
Date: Thu, 26 Feb 2026 11:30:59 -0800 [thread overview]
Message-ID: <aaCfcwdA1E4V5qgE@google.com> (raw)
In-Reply-To: <64a01647-2f99-44a8-a183-702d6eb6fd81@fortanix.com>
On Wed, Feb 25, 2026, Jethro Beekman wrote:
> On 2026-02-25 12:21, Sean Christopherson wrote:
> > On Wed, Feb 25, 2026, Jethro Beekman wrote:
> >> On 2026-02-25 12:05, Sean Christopherson wrote:
> >>> On Mon, Jan 19, 2026, Jethro Beekman wrote:
> >>>> Calling any of the SNP_LAUNCH_ ioctls after SNP_LAUNCH_FINISH results in a
> >>>> kernel page fault due to RMP violation. Track SNP launch state and exit early.
> >>>
> >>> What exactly trips the RMP #PF? A backtrace would be especially helpful for
> >>> posterity.
> >>
> >> Here's a backtrace for calling ioctl(KVM_SEV_SNP_LAUNCH_FINISH) twice. Note this is with a modified version of QEMU.
> >
> >> RIP: 0010:sev_es_sync_vmsa+0x54/0x4c0 [kvm_amd]
> >> snp_launch_update_vmsa+0x19d/0x290 [kvm_amd]
> >> snp_launch_finish+0xb6/0x380 [kvm_amd]
> >> sev_mem_enc_ioctl+0x14e/0x720 [kvm_amd]
> >> kvm_arch_vm_ioctl+0x837/0xcf0 [kvm]
> >
> > Ah, it's the VMSA that's being accessed. Can't we just do?
> >
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 723f4452302a..1e40ae592c93 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -882,6 +882,9 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
> > u8 *d;
> > int i;
> >
> > + if (vcpu->arch.guest_state_protected)
> > + return -EINVAL;
> > +
> > /* Check some debug related fields before encrypting the VMSA */
> > if (svm->vcpu.guest_debug || (svm->vmcb->save.dr7 & ~DR7_FIXED_1))
> > return -EINVAL;
>
> I tried relying on guest_state_protected instead of creating new state but I
> don't think it's sufficient. In particular, your proposal may fix
> snp_launch_finish()
But it does fix that case, correct? I don't want to complicate one fix just
because there are other bugs that are similar but yet distinct.
> but I don't believe this addresses the issues in snp_launch_update() and
Do you mean snp_launch_update_vmsa() here? Or am I missing an interaction with
vCPUs in snp_launch_update()?
> sev_vcpu_create().
There are a pile of SEV lifecycle and locking issues, i.e. this is just one of
several flaws. Fixing the locking has been on my todo list for a few months (we
found some "fun" bugs with an internal run of syzkaller), and I'm finally getting
to it. Hopefully I'll post a series early next week.
Somewhat off the cuff, but I think the easiest way to close the race between
KVM_CREATE_VCPU and KVM_SEV_SNP_LAUNCH_FINISH is to reject KVM_SEV_SNP_LAUNCH_FINISH
if a vCPU is being created. Or did I misunderstand the race you're pointing out?
Though unless there's a strong reason not to, I'd prefer to get greedy and block
all of sev_mem_enc_ioctl(), e.g.
11:23:23 ✔ ~/go/src/kernel.org/linux $ gdd
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ea515cf41168..2b1033c0ec54 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2047,8 +2047,8 @@ static int sev_check_source_vcpus(struct kvm *dst, struct kvm *src)
struct kvm_vcpu *src_vcpu;
unsigned long i;
- if (src->created_vcpus != atomic_read(&src->online_vcpus) ||
- dst->created_vcpus != atomic_read(&dst->online_vcpus))
+ if (kvm_is_vcpu_creation_in_progress(src) ||
+ kvm_is_vcpu_creation_in_progress(dst))
return -EBUSY;
if (!sev_es_guest(src))
@@ -2596,6 +2596,11 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
goto out;
}
+ if (kvm_is_vcpu_creation_in_progress(kvm)) {
+ r = -EBUSY;
+ goto out;
+ }
+
/*
* Once KVM_SEV_INIT2 initializes a KVM instance as an SNP guest, only
* allow the use of SNP-specific commands.
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2c7d76262898..60ca5222e1e5 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1032,6 +1032,13 @@ static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id)
return NULL;
}
+static inline bool kvm_is_vcpu_creation_in_progress(struct kvm *kvm)
+{
+ lockdep_assert_held(&kvm->lock);
+
+ return kvm->created_vcpus != atomic_read(&kvm->online_vcpus);
+}
+
void kvm_destroy_vcpus(struct kvm *kvm);
int kvm_trylock_all_vcpus(struct kvm *kvm);
next prev parent reply other threads:[~2026-02-26 19:31 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-19 19:06 [PATCH] KVM: SEV: Track SNP launch state and disallow invalid userspace interactions Jethro Beekman
2026-01-19 19:12 ` Jethro Beekman
2026-02-25 20:05 ` Sean Christopherson
2026-02-25 20:13 ` Jethro Beekman
2026-02-25 20:21 ` Sean Christopherson
2026-02-25 20:30 ` Jethro Beekman
2026-02-26 19:30 ` Sean Christopherson [this message]
2026-03-07 1:51 ` Sean Christopherson
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=aaCfcwdA1E4V5qgE@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jethro@fortanix.com \
--cc=kvm@vger.kernel.org \
--cc=linux-coco@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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.