linux-kernel - Re: [PATCH v10 24/27] KVM: x86: Enable CET virtualization for VMX and advertise to userspace

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZkuD1uglu5WFSoxR@google.com>
Date: Mon, 20 May 2024 10:09:42 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Weijiang Yang <weijiang.yang@...el.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, rick.p.edgecombe@...el.com, pbonzini@...hat.com, 
	dave.hansen@...el.com, x86@...nel.org, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, peterz@...radead.org, chao.gao@...el.com, 
	mlevitsk@...hat.com, john.allen@....com
Subject: Re: [PATCH v10 24/27] KVM: x86: Enable CET virtualization for VMX and
 advertise to userspace

On Mon, May 20, 2024, Weijiang Yang wrote:
> On 5/17/2024 10:26 PM, Sean Christopherson wrote:
> > On Fri, May 17, 2024, Thomas Gleixner wrote:
> > > On Thu, May 16 2024 at 07:39, Sean Christopherson wrote:
> > > > On Thu, May 16, 2024, Weijiang Yang wrote:
> > > > > We synced the issue internally, and got conclusion that KVM should honor host
> > > > > IBT config.  In this case IBT bit in boot_cpu_data should be honored.  With
> > > > > this policy, it can avoid CPUID confusion to guest side due to host ibt=off
> > > > > config.
> > > > What was the reasoning?  CPUID confusion is a weak justification, eg. it's not
> > > > like the guest has visibility into the host kernel, and raw CPUID will still show
> > > > IBT support in the host.
> > > > 
> > > > On the other hand, I can definitely see folks wanting to expose IBT to guests
> > > > when running non-complaint host kernels, especially when live migration is in
> > > > play, i.e. when hiding IBT from the guest will actively cause problems.
> > > I have to disagree here violently.
> > > 
> > > If the exposure of a CPUID bit to a guest requires host side support,
> > > e.g. in xstate handling, then exposing it to a guest is simply not
> > > possible.
> > Ya, I don't disagree, I just didn't realize that CET_USER would be cleared in the
> > supported xfeatures mask.
> 
> For host side support, fortunately,  this patch already has some checks for
> that. But for userspace CPUID config, it allows IBT to be exposed alone.
> 
> IIUC, this series tries to tie IBT to SHSTK feature, i.e., IBT cannot be
> exposed as an independent feature to guest without exposing SHSTK at the same
> time. If it is, then below patch is not needed anymore:
> https://lore.kernel.org/all/20240219074733.122080-3-weijiang.yang@intel.com/

That's a question for the x86 maintainers.  Specifically, do they want to allow
enabling XFEATURE_CET_USER even if userspace shadow stack support is disabled.

I don't think it impacts KVM, at least not directly.  Regardless of what decision
the kernel makes, KVM needs to disable IBT and SHSTK if CET_USER _or_ CET_KERNEL
is missing, which KVM already does via:

	if ((kvm_caps.supported_xss & (XFEATURE_MASK_CET_USER |
	     XFEATURE_MASK_CET_KERNEL)) !=
	    (XFEATURE_MASK_CET_USER | XFEATURE_MASK_CET_KERNEL)) {
		kvm_cpu_cap_clear(X86_FEATURE_SHSTK);
		kvm_cpu_cap_clear(X86_FEATURE_IBT);
		kvm_caps.supported_xss &= ~(XFEATURE_MASK_CET_USER |
					    XFEATURE_MASK_CET_KERNEL);
	}

> I'd check and clear IBT bit from CPUID when userspace enables only IBT via
> KVM_SET_CPUID2.

No.  It is userspace's responsibility to provide a sane CPUID model for the guest.
KVM needs to ensure that *KVM* doesn't treat IBT as supported if the kernel doesn't
allow XFEATURE_CET_USER, but userspace can advertise whatever it wants to the guest
(and gets to keep the pieces if it does something funky).