linux-kernel - Re: [RFC v5 03/11] mm: Introduce pte_spinlock for FAULT_FLAG

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, 8 Aug 2017 14:16:06 +0200
From:   Laurent Dufour <ldufour@...ux.vnet.ibm.com>
To:     Anshuman Khandual <khandual@...ux.vnet.ibm.com>,
        paulmck@...ux.vnet.ibm.com, peterz@...radead.org,
        akpm@...ux-foundation.org, kirill@...temov.name,
        ak@...ux.intel.com, mhocko@...nel.org, dave@...olabs.net,
        jack@...e.cz, Matthew Wilcox <willy@...radead.org>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        haren@...ux.vnet.ibm.com, npiggin@...il.com, bsingharora@...il.com,
        Tim Chen <tim.c.chen@...ux.intel.com>
Subject: Re: [RFC v5 03/11] mm: Introduce pte_spinlock for
 FAULT_FLAG_SPECULATIVE

On 08/08/2017 12:35, Anshuman Khandual wrote:
> On 06/16/2017 11:22 PM, Laurent Dufour wrote:
>> When handling page fault without holding the mmap_sem the fetch of the
>> pte lock pointer and the locking will have to be done while ensuring
>> that the VMA is not touched in our back.
> 
> It does not change things from whats happening right now, where do we
> check that VMA has not changed by now ?

This patch is preparing the use done later in this series, the goal is to
introduce the service and the check which are relevant.
Later when the VMA check will be added this service is changed.
The goal is to ease the review.

> 
>>
>> So move the fetch and locking operations in a dedicated function.
>>
>> Signed-off-by: Laurent Dufour <ldufour@...ux.vnet.ibm.com>
>> ---
>>  mm/memory.c | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 40834444ea0d..f1132f7931ef 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2240,6 +2240,13 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
>>  	pte_unmap_unlock(vmf->pte, vmf->ptl);
>>  }
>>  
>> +static bool pte_spinlock(struct vm_fault *vmf)
>> +{
>> +	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
>> +	spin_lock(vmf->ptl);
>> +	return true;
>> +}
>> +
> 
> Moving them together makes sense but again if blocks are redundant when
> it returns true all the time.
> 
>>  static bool pte_map_lock(struct vm_fault *vmf)
>>  {
>>  	vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl);
>> @@ -3552,8 +3559,8 @@ static int do_numa_page(struct vm_fault *vmf)
>>  	 * validation through pte_unmap_same(). It's of NUMA type but
>>  	 * the pfn may be screwed if the read is non atomic.
>>  	 */
>> -	vmf->ptl = pte_lockptr(vma->vm_mm, vmf->pmd);
>> -	spin_lock(vmf->ptl);
>> +	if (!pte_spinlock(vmf))
>> +		return VM_FAULT_RETRY;
>>  	if (unlikely(!pte_same(*vmf->pte, vmf->orig_pte))) {
>>  		pte_unmap_unlock(vmf->pte, vmf->ptl);
>>  		goto out;
>> @@ -3745,8 +3752,8 @@ static int handle_pte_fault(struct vm_fault *vmf)
>>  	if (pte_protnone(vmf->orig_pte) && vma_is_accessible(vmf->vma))
>>  		return do_numa_page(vmf);
>>  
>> -	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
>> -	spin_lock(vmf->ptl);
>> +	if (!pte_spinlock(vmf))
>> +		return VM_FAULT_RETRY;
>>  	entry = vmf->orig_pte;
>>  	if (unlikely(!pte_same(*vmf->pte, entry)))
>>  		goto unlock;
>>
>