linux-kernel - Re: [PATCH v4 5/6] KVM: MMU: combine guest pte read between walk and pte prefetch

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: <4C2F2C5B.9020503@redhat.com>
Date:	Sat, 03 Jul 2010 15:26:03 +0300
From:	Avi Kivity <avi@...hat.com>
To:	Xiao Guangrong <xiaoguangrong@...fujitsu.com>
CC:	Marcelo Tosatti <mtosatti@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	KVM list <kvm@...r.kernel.org>
Subject: Re: [PATCH v4 5/6] KVM: MMU: combine guest pte read between walk
 and pte prefetch

On 07/03/2010 03:16 PM, Xiao Guangrong wrote:
>
> Avi Kivity wrote:
>    
>> On 07/03/2010 01:31 PM, Xiao Guangrong wrote:
>>      
>>>        
>>>> See how the pte is reread inside fetch with mmu_lock held.
>>>>
>>>>
>>>>          
>>> It looks like something is broken in 'fetch' functions, this patch will
>>> fix it.
>>>
>>> Subject: [PATCH] KVM: MMU: fix last level broken in FNAME(fetch)
>>>
>>> We read the guest level out of 'mmu_lock', sometimes, the host mapping is
>>> confusion. Consider this case:
>>>
>>> VCPU0:                                              VCPU1
>>>
>>> Read guest mapping, assume the mapping is:
>>> GLV3 ->   GLV2 ->   GLV1 ->   GFNA,
>>> And in the host, the corresponding mapping is
>>> HLV3 ->   HLV2 ->   HLV1(P=0)
>>>
>>>                                                      Write GLV1 and
>>> cause the
>>>                                                      mapping point to GFNB
>>>                                                      (May occur in
>>> pte_write or
>>>                                                         invlpg path)
>>>
>>> Mapping GLV1 to GFNA
>>>
>>> This issue only occurs in the last indirect mapping, since if the middle
>>> mapping is changed, the mapping will be zapped, then it will be detected
>>> in the FNAME(fetch) path, but when it map the last level, it not checked.
>>>
>>> Fixed by also check the last level.
>>>
>>>
>>>        
>> I don't really see what is fixed.  We already check the gpte.  What's
>> special about the new scenario?
>>
>>      
> I mean is: while we map the last level, we will directly set to the pfn but
> the pfn is got by walk_addr, at this time, the guest mapping may be changed.
>
> What is the 'We already check the gpte' mean? i think i miss something :-(
>    

         if (!direct) {
             r = kvm_read_guest_atomic(vcpu->kvm,
                           gw->pte_gpa[level - 2],
&curr_pte, sizeof(curr_pte));
             if (r || curr_pte != gw->ptes[level - 2]) {
                 kvm_mmu_put_page(shadow_page, sptep);
                 kvm_release_pfn_clean(pfn);
                 sptep = NULL;
                 break;
             }
         }

the code you moved... under what scenario is it not sufficient?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

--
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/