linux-kernel - Re: [PATCH] KVM: x86: Track supported ARCH_CAPABILITIES in kvm

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]

Date:   Mon, 22 May 2023 14:23:28 -0700
From:   Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To:     Xiaoyao Li <xiaoyao.li@...el.com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Chao Gao <chao.gao@...el.com>, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
        Jim Mattson <jmattson@...gle.com>,
        antonio.gomez.iglesias@...ux.intel.com,
        Daniel Sneddon <daniel.sneddon@...ux.intel.com>
Subject: Re: [PATCH] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps

On Tue, May 23, 2023 at 03:31:44AM +0800, Xiaoyao Li wrote:
> On 5/23/2023 1:43 AM, Sean Christopherson wrote:
> > > >    6. Performance aside, KVM should not be speculating (ha!) on what the guest
> > > >       will and will not do, and should instead honor whatever behavior is presented
> > > >       to the guest.  If the guest CPU model indicates that VERW flushes buffers,
> > > >       then KVM damn well needs to let VERW flush buffers.
> > > The current implementation allows guests to have VERW flush buffers when
> > > they enumerate FB_CLEAR. It only restricts the flush behavior when the
> > > guest is trying to mitigate against a vulnerability(like MDS) on a
> > > hardware that is not affected. I guess its common for guests to be
> > > running with older gen configuration on a newer hardware.
> > Right, I'm saying that that behavior is wrong.  KVM shouldn't assume the guest
> > the guest will do things a certain way and should instead honor the "architectural"
> > definition, in quotes because I realize there probably is no architectural
> > definition for any of this.
> > 
> > It might be that the code does (unintentionally?) honor the "architecture", i.e.
> > this code might actually be accurrate with respect to when the guest can expect
> > VERW to flush buffers.  But the comment is so, so wrong.
> 
> The comment is wrong and the code is wrong in some case as well.
> 
> If none of ARCH_CAP_FB_CLEAR, ARCH_CAP_MDS_NO, ARCH_CAP_TAA_NO,
> ARCH_CAP_PSDP_NO, ARCH_CAP_FBSDP_NO and ARCH_CAP_SBDR_SSDP_NO are exposed to
> VM, the VM is type of "affected by MDS".
> 
> And accroding to the page https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/processor-mmio-stale-data-vulnerabilities.html
> 
> if the VM enumerates support for both L1D_FLUSH and MD_CLEAR, it implicitly
> enumerates FB_CLEAR as part of their MD_CLEAR support.

This is the excerpt from the link that you mentioned:

  "For processors that are affected by MDS and support L1D_FLUSH
  operations and MD_CLEAR operations, the VERW instruction flushes fill
  buffers."

You are missing an important information here "For the processors
_affected_ by MDS". On such processors ...

> However, the code will leave vmx->disable_fb_clear as 1 if hardware supports
> it, and VERW intruction doesn't clear FB in the VM, which conflicts
> "architectural" definition.

... Fill buffer clear is not enabled at all:

  vmx_setup_fb_clear_ctrl()
  {
  	u64 msr;
  
  	if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES) &&
  	    !boot_cpu_has_bug(X86_BUG_MDS) &&
  	    !boot_cpu_has_bug(X86_BUG_TAA)) {
  		rdmsrl(MSR_IA32_ARCH_CAPABILITIES, msr);
  		if (msr & ARCH_CAP_FB_CLEAR_CTRL)
  			vmx_fb_clear_ctrl_available = true;
  	}
  }