Re: [PATCH 3/4] KVM: Add SMAP support when setting CR4

[Paolo Bonzini <pbonzini@redhat.com>]

Il 27/03/2014 13:25, Feng Wu ha scritto:
> +void update_permission_bitmask(struct kvm_vcpu *vcpu,
>  		struct kvm_mmu *mmu, bool ept)
>  {
>  	unsigned bit, byte, pfec;
>  	u8 map;
> -	bool fault, x, w, u, wf, uf, ff, smep;
> +	bool fault, x, w, u, wf, uf, ff, smep, smap;
>
>  	smep = kvm_read_cr4_bits(vcpu, X86_CR4_SMEP);
> +	smap = kvm_read_cr4_bits(vcpu, X86_CR4_SMAP);
>  	for (byte = 0; byte < ARRAY_SIZE(mmu->permissions); ++byte) {
>  		pfec = byte << 1;
>  		map = 0;
> @@ -3617,11 +3618,26 @@ static void update_permission_bitmask(struct kvm_vcpu *vcpu,
>  				w |= !is_write_protection(vcpu) && !uf;
>  				/* Disallow supervisor fetches of user code if cr4.smep */
>  				x &= !(smep && u && !uf);
> +
> +				/*
> +				 * SMAP:kernel-mode data accesses from user-mode
> +				 * mappings should fault. A fault is considered
> +				 * as a SMAP violation if all of the following
> +				 * conditions are ture:
> +				 *   - X86_CR4_SMAP is set in CR4
> +				 *   - An user page is accessed
> +				 *   - !(CPL<3 && X86_EFLAGS_AC is set)
> +				 *   - Page fault in kernel mode
> +				 */
> +				smap = smap && u && !uf &&
> +					!((kvm_x86_ops->get_cpl(vcpu) < 3) &&
> +					((kvm_x86_ops->get_rflags(vcpu) &
> +					X86_EFLAGS_AC) == 1));

Unfortunately this doesn't work.

The reason is that changing X86_EFLAGS_AC doesn't trigger 
update_permission_bitmask.  So the value of CPL < 3 && AC = 1 must not 
be checked in update_permission_bitmask; instead, it must be included in 
the index into the permissions array.  You can reuse the PFERR_RSVD_MASK 
bit, like

	smapf = pfec & PFERR_RSVD_MASK;
	...
		smap = smap && smapf u && !uf;

The VCPU can then be passed to permission_fault in order to get the 
value of the CPL and the AC bit.

Please test nested virtualization too.  I think PFERR_RSVD_MASK should 
be removed in translate_nested_gpa.

Paolo