linux-kernel - Re: [RESEND PATCH v2 4/5] kvm: Remove ept_identity_pagetable from struct 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:	Tue, 15 Jul 2014 18:39:25 +0800
From:	Tang Chen <tangchen@...fujitsu.com>
To:	Gleb Natapov <gleb@...nel.org>
CC:	<mtosatti@...hat.com>, <nadav.amit@...il.com>,
	<kvm@...r.kernel.org>, <laijs@...fujitsu.com>,
	<isimatu.yasuaki@...fujitsu.com>, <guz.fnst@...fujitsu.com>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH v2 4/5] kvm: Remove ept_identity_pagetable from
 struct kvm_arch.

On 07/14/2014 10:27 PM, Gleb Natapov wrote:
......
>>>>   	if (likely(kvm->arch.ept_identity_pagetable_done))
>>>>   		return 1;
>>>> -	ret = 0;
>>>>   	identity_map_pfn = kvm->arch.ept_identity_map_addr>>   PAGE_SHIFT;
>>>> +
>>>> +	mutex_lock(&kvm->slots_lock);
>>> Why move this out of alloc_identity_pagetable()?
>>>
>>
>> Referring to the original code, I think mutex_lock(&kvm->slots_lock) is used
>> to protect kvm->arch.ept_identity_pagetable. If two or more threads try to
>> modify it at the same time, the mutex ensures that the identity table is
>> only
>> allocated once.
>>
>> Now, we dropped kvm->arch.ept_identity_pagetable. And use
>> kvm->arch.ept_identity_pagetable_done
>> to check if the identity table is allocated and initialized. So we should
>> protect
>> memory slot operation in alloc_identity_pagetable() and
>> kvm->arch.ept_identity_pagetable_done
>> with this mutex.
>>
>> Of course, I can see that the name "slots_lock" indicates that it may be
>> used
>> to protect the memory slot operation only. Maybe move it out here is not
>> suitable.
>>
>> If I'm wrong, please tell me.
>>
> No, you are right that besides memory slot creation slots_lock protects checking
> of ept_identity_pagetable here, but after you patch ept_identity_pagetable_done is
> tested outside of slots_lock so the allocation can happen twice, no?

Oh, yes. Will fix it in the next version.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/