Re: [PATCH] KVM: SEV: Track SNP launch state and disallow invalid userspace interactions

[Sean Christopherson <seanjc@google.com>]

On Thu, Feb 26, 2026, Sean Christopherson wrote:
> 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.

Circling back to this (writing changelogs), I don't think there's actually a
novel bug with respect to KVM_SEV_SNP_LAUNCH_FINISH racing with KVM_CREATE_VCPU.

kvm_for_each_vcpu() operates on online_vcpus, LAUNCH_FINISH (all SEV+ sub-ioctls)
holds kvm->mutex, and fully onlining a vCPU in kvm_vm_ioctl_create_vcpu() is done
under kvm->mutex.  So AFAICT, there's no difference between an in-progress vCPU
and a vCPU that is created entirely after LAUNCH_FINISH.

It's probably worth preventing as a hardening measure, but I don't think there's
an actual bug to be fixed.