linux-kernel - Re: [PATCH 1/4] KVM: MMU: fix permission

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: <56FABECE.40601@linux.intel.com>
Date:	Wed, 30 Mar 2016 01:43:42 +0800
From:	Xiao Guangrong <guangrong.xiao@...ux.intel.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	gleb@...nel.org, mtosatti@...hat.com, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] KVM: MMU: fix permission_fault()

On 03/25/2016 10:21 PM, Paolo Bonzini wrote:
>
>
> On 25/03/2016 14:19, Xiao Guangrong wrote:
>>   	WARN_ON(pfec & (PFERR_PK_MASK | PFERR_RSVD_MASK));
>> -	pfec |= PFERR_PRESENT_MASK;
>> +	errcode = PFERR_PRESENT_MASK;
>>
>>   	if (unlikely(mmu->pkru_mask)) {
>>   		u32 pkru_bits, offset;
>> @@ -193,11 +193,11 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>>   			((pte_access & PT_USER_MASK) << (PFERR_RSVD_BIT - PT_USER_SHIFT));
>>
>>   		pkru_bits &= mmu->pkru_mask >> offset;
>> -		pfec |= -pkru_bits & PFERR_PK_MASK;
>> +		errcode |= -pkru_bits & PFERR_PK_MASK;
>>   		fault |= (pkru_bits != 0);
>>   	}
>>
>> -	return -(uint32_t)fault & pfec;
>> +	return -(uint32_t)fault & errcode;
>>   }
>
> I have another doubt here.
>
> If you get a fault due to U=0, you would not get PFERR_PK_MASK.  This
> is checked implicitly through the pte_user bit which we moved to
> PFERR_RSVD_BIT.  However, if you get a fault due to W=0 _and_
> PKRU.AD=1 or PKRU.WD=1 for the page's protection key, would the PK
> bit be set in the error code?  If not, we would need something like
> this:

Based on the SDM:
PK flag (bit 5).
This flag is 1 if (1) IA32_EFER.LMA = CR4.PKE = 1; (2) the access causing the page-fault exception 
was a data access; (3) the linear address was a user-mode address with protection key i; and (5) the 
PKRU register (see Section 4.6.2) is such that either (a) ADi = 1; or (b) the following all hold: 
(i) WDi = 1; (ii) the access is a write access; and (iii) either CR0.WP = 1 or the access causing 
the page-fault exception was a user-mode access.

So I think PKEY check and ordinary check are independent, i.e, PFEC.PKEY may be
set even if the on permission on the page table is not adequate.