linux-kernel - Re: [PATCH] KVM: x86: Set BHI_NO in guest when host is not affected by BHI

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: <CABgObfZU_uLAPzDV--n67H3Hq6OKxUO=FQa2MH3CjdgTQR8pJg@mail.gmail.com>
Date: Thu, 11 Apr 2024 16:46:32 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Alexandre Chartre <alexandre.chartre@...cle.com>
Cc: Andrew Cooper <andrew.cooper3@...rix.com>, x86@...nel.org, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, daniel.sneddon@...ux.intel.com, 
	pawan.kumar.gupta@...ux.intel.com, tglx@...utronix.de, konrad.wilk@...cle.com, 
	peterz@...radead.org, gregkh@...uxfoundation.org, seanjc@...gle.com, 
	dave.hansen@...ux.intel.com, nik.borisov@...e.com, kpsingh@...nel.org, 
	longman@...hat.com, bp@...en8.de
Subject: Re: [PATCH] KVM: x86: Set BHI_NO in guest when host is not affected
 by BHI

On Thu, Apr 11, 2024 at 4:34 PM Alexandre Chartre
<alexandre.chartre@...cle.com> wrote:
> Still, we could enumerate CPUs which don't have eIBRS independently of NO_BHI
> (if we have a list of such CPUs) and set X86_BUG_BHI for cpus with eIBRS.
>
> So in arch/x86/kernel/cpu/common.c, replace:
>
>         /* When virtualized, eIBRS could be hidden, assume vulnerable */
>         if (!(ia32_cap & ARCH_CAP_BHI_NO) &&
>             !cpu_matches(cpu_vuln_whitelist, NO_BHI) &&
>             (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
>              boot_cpu_has(X86_FEATURE_HYPERVISOR)))
>                 setup_force_cpu_bug(X86_BUG_BHI);
>
> with something like:
>
>         if (!(ia32_cap & ARCH_CAP_BHI_NO) &&
>             !cpu_matches(cpu_vuln_whitelist, NO_BHI) &&
>             (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
>             !cpu_matches(cpu_vuln_whitelist, NO_EIBRS)))
>                 setup_force_cpu_bug(X86_BUG_BHI);

No, that you cannot do because the hypervisor can and will fake the
family/model/stepping.

However, looking again at the original patch you submitted, I think
the review was confusing host and guest sides. If the host is "not
affected", i.e. the host *genuinely* does not have eIBRS, it can set
BHI_NO and that will bypass the "always mitigate in a guest" bit.

I think that's robust and boot_cpu_has_bug(X86_BUG_BHI) is the right
way to do it.

If a VM migration pool has both !eIBRS and eIBRS machines, it will
combine the two; on one hand it will not set the eIBRS bit (bit 1), on
the other hand it will not set BHI_NO either, and it will set the
hypervisor bit. The result is that the guest *will* use mitigations.

To double check, from the point of view of a nested hypervisor, you
could set BHI_NO in a nested guest:
* if the nested hypervisor has BHI_NO passed from the outer level
* or if its CPUID passes cpu_matches(cpu_vuln_whitelist, NO_BHI)
* but it won't matter whether the nested hypervisor lacks eIBRS,
because that bit is not reliable in a VM

The logic you'd use in KVM therefore is:

   (ia32_cap & ARCH_CAP_BHI_NO) ||
   cpu_matches(cpu_vuln_whitelist, NO_BHI) ||
   (!boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) &&
    !boot_cpu_has(X86_FEATURE_HYPERVISOR)))

but that is exactly !boot_cpu_has_bug(X86_BUG_BHI) and is therefore
what Alexandre's patch does.

So I'll wait for further comments but I think the patch is correct.

Paolo