linux-kernel - Re: [PATCH v16 36/51] KVM: nSVM: Save/load CET Shadow Stack state to/from vmcb12/vmcb02

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <tamhy4gqijflouthniyre3w5r4ywjuzvlaeavvgyrfifozdi3g@zcd432svuw5i>
Date: Tue, 9 Dec 2025 00:48:18 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Tom Lendacky <thomas.lendacky@....com>, 
	Mathias Krause <minipli@...ecurity.net>, John Allen <john.allen@....com>, 
	Rick Edgecombe <rick.p.edgecombe@...el.com>, Chao Gao <chao.gao@...el.com>, 
	Binbin Wu <binbin.wu@...ux.intel.com>, Xiaoyao Li <xiaoyao.li@...el.com>, 
	Maxim Levitsky <mlevitsk@...hat.com>, Zhang Yi Z <yi.z.zhang@...ux.intel.com>, Xin Li <xin@...or.com>
Subject: Re: [PATCH v16 36/51] KVM: nSVM: Save/load CET Shadow Stack state
 to/from vmcb12/vmcb02

On Tue, Oct 28, 2025 at 10:23:02PM +0000, Yosry Ahmed wrote:
> On Fri, Sep 19, 2025 at 03:32:43PM -0700, Sean Christopherson wrote:
> > Transfer the three CET Shadow Stack VMCB fields (S_CET, ISST_ADDR, and
> > SSP) on VMRUN, #VMEXIT, and loading nested state (saving nested state
> > simply copies the entire save area).  SVM doesn't provide a way to
> > disallow L1 from enabling Shadow Stacks for L2, i.e. KVM *must* provide
> > nested support before advertising SHSTK to userspace.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> > ---
> >  arch/x86/kvm/svm/nested.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 826473f2d7c7..a6443feab252 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -636,6 +636,14 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> >  		vmcb_mark_dirty(vmcb02, VMCB_DT);
> >  	}
> >  
> > +	if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK) &&
> > +	    (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_CET)))) {
> > +		vmcb02->save.s_cet  = vmcb12->save.s_cet;
> > +		vmcb02->save.isst_addr = vmcb12->save.isst_addr;
> > +		vmcb02->save.ssp = vmcb12->save.ssp;
> > +		vmcb_mark_dirty(vmcb02, VMCB_CET);
> > +	}
> > +
> 
> According to the APM, there are some consistency checks that should be
> done on CET related fields in the VMCB12. Specifically from
> "Canonicalization and Consistency Checks. " in 15.5.1 in the APM Volume
> 2 (24593—Rev. 3.42—March 2024):
> 
> • Any reserved bit is set in S_CET
> • CR4.CET=1 when CR0.WP=0
> • CR4.CET=1 and U_CET.SS=1 when EFLAGS.VM=1
> • Any reserved bit set in U_CET (SEV-ES only):
>   - VMRUN results in VMEXIT(INVALID)
>   - VMEXIT forces reserved bits to 0
> 
> Most consistency checks are done in __nested_vmcb_check_save(), but it
> only operates on the cached save area, which does not have everything
> you need. You'll probably need to add the needed fields to the cached
> save area, or move the consistency checks elsewhere.
> 
> Related to this, I am working on patches to copy everything we use from
> vmcb12->save to the cache area to minimize directly accessing vmcb12
> from the guest memory as much as possible. So I already intend to add
> other fields to the cached save area.
> 
> There's also a couple of other missing consistency checks that I will
> send patches for, which also need fields currently not in the cached
> save area.

I don't really care that much, but I think this fell through the cracks.

Regarding the cached save area, the series I was talking about is
already out [*], and I am preparing to send a newer version. It puts the
fields used here in the cache, so it should be straightforward to add
the consistency checks on top of it.

[*]https://lore.kernel.org/kvm/20251110222922.613224-1-yosry.ahmed@linux.dev/

> 
> >  	kvm_set_rflags(vcpu, vmcb12->save.rflags | X86_EFLAGS_FIXED);
> >  
> >  	svm_set_efer(vcpu, svm->nested.save.efer);
> > @@ -1044,6 +1052,12 @@ void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
> >  	to_save->rsp = from_save->rsp;
> >  	to_save->rip = from_save->rip;
> >  	to_save->cpl = 0;
> > +
> > +	if (kvm_cpu_cap_has(X86_FEATURE_SHSTK)) {
> > +		to_save->s_cet  = from_save->s_cet;
> > +		to_save->isst_addr = from_save->isst_addr;
> > +		to_save->ssp = from_save->ssp;
> > +	}
> >  }
> >  
> >  void svm_copy_vmloadsave_state(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
> > @@ -1111,6 +1125,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> >  	vmcb12->save.dr6    = svm->vcpu.arch.dr6;
> >  	vmcb12->save.cpl    = vmcb02->save.cpl;
> >  
> > +	if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK)) {
> > +		vmcb12->save.s_cet	= vmcb02->save.s_cet;
> > +		vmcb12->save.isst_addr	= vmcb02->save.isst_addr;
> > +		vmcb12->save.ssp	= vmcb02->save.ssp;
> > +	}
> > +
> >  	vmcb12->control.int_state         = vmcb02->control.int_state;
> >  	vmcb12->control.exit_code         = vmcb02->control.exit_code;
> >  	vmcb12->control.exit_code_hi      = vmcb02->control.exit_code_hi;
> > -- 
> > 2.51.0.470.ga7dc726c21-goog
> >