linux-kernel - Re: [GIT PULL] KVM changes for Linux 6.14

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: <Z5elOuz1IjFXAtGx@google.com>
Date: Mon, 27 Jan 2025 07:24:42 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [GIT PULL] KVM changes for Linux 6.14

On Sat, Jan 25, 2025, Linus Torvalds wrote:
> On Fri, 24 Jan 2025 at 08:38, Paolo Bonzini <pbonzini@...hat.com> wrote:
> >
> > but you can throw away the <<<< ... ==== part completely, and apply the
> > same change on top of the new implementation:
> >
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index edef30359c19..9f9a29be3beb 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -1177,6 +1177,7 @@ void kvm_set_cpu_caps(void)
> >                 EMULATED_F(NO_SMM_CTL_MSR),
> >                 /* PrefetchCtlMsr */
> >                 F(WRMSR_XX_BASE_NS),
> > +               F(SRSO_USER_KERNEL_NO),
> >                 SYNTHESIZED_F(SBPB),
> >                 SYNTHESIZED_F(IBPB_BRTYPE),
> >                 SYNTHESIZED_F(SRSO_NO),
> 
> Ehh. My resolution ended up being different.
> 
> I did this instead:
> 
>                F(WRMSR_XX_BASE_NS),
>                SYNTHESIZED_F(SBPB),
>                SYNTHESIZED_F(IBPB_BRTYPE),
>                SYNTHESIZED_F(SRSO_NO),
> +              SYNTHESIZED_F(SRSO_USER_KERNEL_NO),
> 
> which (apart from the line ordering) differs from your suggestion in
> F() vs SYNTHESIZED_F().
> 
> That really seemed to be the RightThing(tm) to do from the context of
> the two conflicting commits, but maybe there was some reason that I
> didn't catch that you kept it as a plain "F()".

Heh, I waffled on whether SRSO_USER_KERNEL_NO should be F() or SYNTHESIZED_F()
when the initial commit went in.  I would prefer to keep it F(), though it doesn't
matter terribly at the moment.

The "synthesized" features are for cases where the kernel stuffs X86_FEATURE_xxx
via set_cpu_cap() even when the feature isn't present in CPUID, and it's correct
for KVM to relay the synthesized feature to the guest.

E.g. SRSO_NO is synthesized into cpu_caps for Zen1/2, and in that case the
absense of the SRSO flaw extends to the guest as well.

		if (boot_cpu_data.x86 < 0x19 && !cpu_smt_possible()) {
			setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
			return;
		}

For SRSO_USER_KERNEL_NO, it's currently not force set, i.e. it's a pure reflection
of hardware capabilities.  Treating it as synthesized is effectively a nop with
the current code, but that would change if the kernel were to force set the flag.

If a future commit force set SRSO_USER_KERNEL_NO because of a ucode update that
didn't also modify CPUID behavior, then treating the flag as synthesized would
be desirabled, e.g. so that the guest could also avoid the overhead of mitigating
SRSO.

But if a future commit set the flag for some other reason, e.g. if the kernel
somehow isn't vulnerable even when running on buggy hardware, then enumerating
SRSO_USER_KERNEL_NO to the guest could cause the guest kernel to incorrectly
skips its mitigation.

My vote is to err on the side of caution and go with F().