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: <ngbxelfw4lvipsvnoykqo4sonuyjqhuyoh5yogvc6btqj4w6cr@y2jpmnyjphmc>
Date: Tue, 28 Oct 2025 22:23:02 +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 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.

>  	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
>