linux-kernel - Re: [PATCH v1] mm: drop NULL return check of pte_offset_map

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] [day] [month] [year] [list]

Date:   Fri, 19 May 2017 15:15:59 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Naoya Horiguchi <n-horiguchi@...jp.nec.com>, linux-mm@...ck.org
Cc:     Mel Gorman <mgorman@...e.de>, Johannes Weiner <hannes@...xchg.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Rik van Riel <riel@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] mm: drop NULL return check of pte_offset_map_lock()

On 05/18/2017 08:42 AM, Naoya Horiguchi wrote:
> pte_offset_map_lock() finds and takes ptl, and returns pte.
> But some callers return without unlocking the ptl when pte == NULL,
> which seems weird.
> 
> Git history said that !pte check in change_pte_range() was introduced in
> commit 1ad9f620c3a2 ("mm: numa: recheck for transhuge pages under lock
> during protection changes") and still remains after commit 175ad4f1e7a2
> ("mm: mprotect: use pmd_trans_unstable instead of taking the pmd_lock")
> which partially reverts 1ad9f620c3a2. So I think that it's just dead code.
> 
> Many other caller of pte_offset_map_lock() never check NULL return, so
> let's do likewise.
> 
> Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>

Acked-by: Vlastimil Babka <vbabka@...e.cz>

> ---
>  mm/memory.c   | 2 --
>  mm/mprotect.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git v4.11-rc6-mmotm-2017-04-13-14-50/mm/memory.c v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/memory.c
> index 8ae6700..c17fad1d 100644
> --- v4.11-rc6-mmotm-2017-04-13-14-50/mm/memory.c
> +++ v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/memory.c
> @@ -4040,8 +4040,6 @@ static int __follow_pte_pmd(struct mm_struct *mm, unsigned long address,
>  		goto out;
>  
>  	ptep = pte_offset_map_lock(mm, pmd, address, ptlp);
> -	if (!ptep)
> -		goto out;
>  	if (!pte_present(*ptep))
>  		goto unlock;
>  	*ptepp = ptep;
> diff --git v4.11-rc6-mmotm-2017-04-13-14-50/mm/mprotect.c v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/mprotect.c
> index 8fd010f..d60a1ee 100644
> --- v4.11-rc6-mmotm-2017-04-13-14-50/mm/mprotect.c
> +++ v4.11-rc6-mmotm-2017-04-13-14-50_patched/mm/mprotect.c
> @@ -58,8 +58,6 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>  	 * reading.
>  	 */
>  	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> -	if (!pte)
> -		return 0;
>  
>  	/* Get target node for single threaded private VMAs */
>  	if (prot_numa && !(vma->vm_flags & VM_SHARED) &&
>