linux-kernel - Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP

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: <20160713203006.GB16130@potion>
Date:	Wed, 13 Jul 2016 22:30:07 +0200
From:	Radim Krčmář <rkrcmar@...hat.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP

2016-07-12 21:20+0200, Paolo Bonzini:
> UMIP (User-Mode Instruction Prevention) is a feature of future
> Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
> and SMSW from user-mode processes.
> 
> On Intel systems it's *almost* possible to emulate it; it slows
> down the instructions when they're executed in ring 0, but they
> are really never executed in practice.  The catch is that SMSW
> doesn't cause a vmexit, and hence SMSW will not fault.
> 
> When UMIP is enabled but not supported by the host, descriptor table
> exits are enabled, and the emulator takes care of injecting a #GP when
> any of SLDT, SGDT, STR, SIDT are encountered.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> @@ -3967,6 +3968,14 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  		(to_vmx(vcpu)->rmode.vm86_active ?
>  		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
>  
> +	if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
> +		vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
> +			      SECONDARY_EXEC_DESC);

If UMIP support is not exposed in CPUID, we ought to #GP(0), because it
is a write to reserved bits.  It could also mean that the vm control is
not supported.

> +		hw_cr4 &= ~X86_CR4_UMIP;
> +	} else
> +		vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
> +				SECONDARY_EXEC_DESC);

I think we don't have to do anything when the CPU supports UMIP,

 if (!boot_cpu_has(X86_FEATURE_UMIP) {
   if ((cr4 & X86_CR4_UMIP)) { ... } else ...
 }

And we could then return true in vmx_umip_emulated() when
boot_cpu_has(X86_FEATURE_UMIP).
(Just for self-documentation, because occurrence of X86_FEATURE_UMIP is
 most likely a subset of SECONDARY_EXEC_DESC.)

> @@ -8597,7 +8627,8 @@ static bool vmx_xsaves_supported(void)
>  
>  static bool vmx_umip_emulated(void)
>  {
> -	return false;
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_DESC;
>  }