linux-kernel - Re: [PATCH] KVM: VMX: expose the host's ARCH

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180302214212.GB13606@flask>
Date:   Fri, 2 Mar 2018 22:42:12 +0100
From:   Radim Krčmář <rkrcmar@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        David Woodhouse <dwmw@...zon.co.uk>,
        KarimAllah Ahmed <karahmed@...zon.de>
Subject: Re: [PATCH] KVM: VMX: expose the host's ARCH_CAPABILITIES MSR to
 userspace

2018-03-02 10:36+0100, Paolo Bonzini:
> On 01/03/2018 22:39, Radim Krčmář wrote:
> > [Resent after removing g@...r.us.oracle.com.]
> > 
> > 2018-02-26 17:13-0500, Konrad Rzeszutek Wilk:
> >> On Sat, Feb 24, 2018 at 01:52:26AM +0100, Paolo Bonzini wrote:
> >>> Use the new MSR feature framework to expose the ARCH_CAPABILITIES MSR to
> >>> userspace.  This way, userspace can access the capabilities even if it
> >>> does not have the permissions to read MSRs.
> >>
> >> ... That is good but could you expand a bit of why it would want this?
> >>
> >> I am 99% sure it is due to the lovely spectre_v2 mitigation but
> >> could you include that in the commit message so that in say a year
> >> folks would know what this is?
> > 
> > Userspace can currently get the MSR by creating a VCPU and reading its
> > MSR_IA32_ARCH_CAPABILITIES, because it is set from the hardware MSR.
> > 
> > I thought that "permissions to read MSRs" talked about hardware MSRs, so
> > the purpose of this patch would be a better interface, but I don't see
> > how if we keep the auto-setting on VCPU creation.
> 
> Yeah, it's mostly about a better interface and being able to do checks
> before creating the VCPU.  The commit message was written before I
> noticed the auto-setting on VCPU creation, and I failed to update it.

Ok, sounds good.  I've deferred it to rc5 as I think we'll want to use
this to replace the auto setting:  I would not bet that it is going to
be safe to expose future bits, so having the userspace always sanitize
the capabilities would be safer (and more in line with what we do with
other MSRs).  i.e. this patch would also

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 051dab74e4e9..86ea4a83af1f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5740,9 +5740,6 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
 		++vmx->nmsrs;
 	}
 
-	if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
-		rdmsrl(MSR_IA32_ARCH_CAPABILITIES, vmx->arch_capabilities);
-
 	vm_exit_controls_init(vmx, vmcs_config.vmexit_ctrl);
 
 	/* 22.2.1, 20.8.1 */