linux-kernel - Re: [PATCH] userfaultfd: hugetlbfs: add UFFDIO

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Feb 2017 12:17:38 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Andrea Arcangeli <aarcange@...hat.com>
Cc:     Mike Kravetz <mike.kravetz@...cle.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        "Dr. David Alan Gilbert" <dgilbert@...hat.com>,
        Hillf Danton <hillf.zj@...baba-inc.com>,
        Pavel Emelyanov <xemul@...allels.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>
Subject: Re: [PATCH] userfaultfd: hugetlbfs: add UFFDIO_COPY support for
 shared mappings

On Fri, 17 Feb 2017 16:52:41 +0100 Andrea Arcangeli <aarcange@...hat.com> wrote:

> Everything else is identical which is great. Mike Rapoport could you
> verify the below hunk is missing in mm?
> 
> Once it'll all be merged upstream then there will be less merge crunch
> as we've been working somewhat in parallel on the same files, so this
> is resulting in more merge rejects than ideal :).
> 
> diff --git a/../mm/mm/userfaultfd.c b/mm/userfaultfd.c
> index 830bed7..3ec9aad 100644
> --- a/../mm/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -199,6 +201,12 @@ static __always_inline ssize_t __mcopy_atomic_hugetlb(struct mm_struct *dst_mm,
>  		dst_vma = find_vma(dst_mm, dst_start);
>  		if (!dst_vma || !is_vm_hugetlb_page(dst_vma))
>  			goto out_unlock;
> +		/*
> +		 * Only allow __mcopy_atomic_hugetlb on userfaultfd
> +		 * registered ranges.
> +		 */
> +		if (!dst_vma->vm_userfaultfd_ctx.ctx)
> +			goto out_unlock;
>  
>  		if (dst_start < dst_vma->vm_start ||
>  		    dst_start + len > dst_vma->vm_end)
> @@ -214,16 +224,10 @@ static __always_inline ssize_t __mcopy_atomic_hugetlb(struct mm_struct *dst_mm,
>  		goto out_unlock;
>  
>  	/*
> -	 * Only allow __mcopy_atomic_hugetlb on userfaultfd registered ranges.
> -	 */
> -	if (!dst_vma->vm_userfaultfd_ctx.ctx)
> -		goto out_unlock;
> -
> -	/*
>  	 * If not shared, ensure the dst_vma has a anon_vma.
>  	 */

I merged this up and a small issue remains:


:	/*
:	 * Validate alignment based on huge page size
:	 */
:	err = -EINVAL;
:	if (dst_start & (vma_hpagesize - 1) || len & (vma_hpagesize - 1))
:		goto out_unlock;
:
:retry:
:	/*
:	 * On routine entry dst_vma is set.  If we had to drop mmap_sem and
:	 * retry, dst_vma will be set to NULL and we must lookup again.
:	 */
:	if (!dst_vma) {
:		err = -ENOENT;
:		dst_vma = find_vma(dst_mm, dst_start);
:		if (!dst_vma || !is_vm_hugetlb_page(dst_vma))
:			goto out_unlock;
:		/*
:		 * Only allow __mcopy_atomic_hugetlb on userfaultfd
:		 * registered ranges.
:		 */
:		if (!dst_vma->vm_userfaultfd_ctx.ctx)
:			goto out_unlock;
:
:		if (dst_start < dst_vma->vm_start ||
:		    dst_start + len > dst_vma->vm_end)
:			goto out_unlock;
:
:		err = -EINVAL;
:		if (vma_hpagesize != vma_kernel_pagesize(dst_vma))
:			goto out_unlock;
:	}
:
:	if (WARN_ON(dst_addr & (vma_hpagesize - 1) ||
:		    (len - copied) & (vma_hpagesize - 1)))
:		goto out_unlock;

The value of `err' here is EINVAL.  That sems appropriate, but it only
happens by sheer luck.

:	/*
:	 * If not shared, ensure the dst_vma has a anon_vma.
:	 */
:	err = -ENOMEM;
:	if (!(dst_vma->vm_flags & VM_SHARED)) {
:		if (unlikely(anon_vma_prepare(dst_vma)))
:			goto out_unlock;
:	}

So...

--- a/mm/userfaultfd.c~userfaultfd-mcopy_atomic-return-enoent-when-no-compatible-vma-found-fix-2-fix
+++ a/mm/userfaultfd.c
@@ -215,6 +215,7 @@ retry:
 			goto out_unlock;
 	}
 
+	err = -EINVAL;
 	if (WARN_ON(dst_addr & (vma_hpagesize - 1) ||
 		    (len - copied) & (vma_hpagesize - 1)))
 		goto out_unlock;
_