RE: [PATCH v1] KVM: x86: Validate values set to guest's MSR_IA32_ARCH_CAPABILITIES

[Wang, Wei W]

On Tuesday, April 23, 2024 3:44 AM, Sean Christopherson wrote:
> On Mon, Apr 22, 2024, Wei Wang wrote:
> > If the bits set by userspace to the guest's MSR_IA32_ARCH_CAPABILITIES
> > are not supported by KVM, fails the write. This safeguards against the
> > launch of a guest with a feature set, enumerated via
> > MSR_IA32_ARCH_CAPABILITIES, that surpasses the capabilities supported
> > by KVM.
> 
> I'm not entirely certain KVM cares.  Similar to guest CPUID, advertising features
> to the guest that are unbeknownst may actually make sense in some scenarios,
> e.g.
> if userspace learns of yet another "NO" bit that says a CPU isn't vulnerable to
> some flaw.

I think it might be more appropriate for the guest to see the "NO" bit only when
the host, such as the hardware (i.e., host_arch_capabilities), already supports it.
Otherwise, the guest could be misled by a false "NO" bit. For instance, the guest
might assume it's not vulnerable to a certain flaw as it sees the "NO" bit from the
MSR, even though the enhancement feature isn't actually supported by the host,
and thus bypass a workaround (to the vulnerability) it should have used. This could
arise with a faulty or compromised userspace.
Another scenario pertains to guest live migration: the source platform physically
supports the "NO" bit, but the destination platform does not. If KVM fails the MSR
write here, it could prevent such a live migration from proceeding.

So I think it might be prudent for KVM to perform this check. This is similar to the
MSR_IA32_PERF_CAPABILITIES case that we have implemented.

> 
> ARCH_CAPABILITIES is read-only, i.e. KVM _can't_ shove it into hardware.  So
> as long as KVM treats the value as "untrusted", like KVM does for guest CPUID,
> I think the current behavior is actually ok.

Yes, the value coming from userspace could be considered "untrusted", but should
KVM ensure to expose a trusted/reliable value to the guest?


> 
> > Fixes: 0cf9135b773b ("KVM: x86: Emulate MSR_IA32_ARCH_CAPABILITIES on
> > AMD hosts")
> 
> This goes all the way back to:
> 
>   28c1c9fabf48 ("KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES")
> 
> the above just moved the logic from vmx.c to x86.c.

OK.