linux-kernel - Re: [PATCH 5/9] userfaultfd: add minor fault registration mode

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: <20210121184938.GD260413@xz-x1>
Date:   Thu, 21 Jan 2021 13:49:38 -0500
From:   Peter Xu <peterx@...hat.com>
To:     Axel Rasmussen <axelrasmussen@...gle.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Anshuman Khandual <anshuman.khandual@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Chinwen Chang <chinwen.chang@...iatek.com>,
        Huang Ying <ying.huang@...el.com>,
        Ingo Molnar <mingo@...hat.com>, Jann Horn <jannh@...gle.com>,
        Jerome Glisse <jglisse@...hat.com>,
        Lokesh Gidra <lokeshgidra@...gle.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Michal Koutný <mkoutny@...e.com>,
        Michel Lespinasse <walken@...gle.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Nicholas Piggin <npiggin@...il.com>, Shaohua Li <shli@...com>,
        Shawn Anastasio <shawn@...stas.io>,
        Steven Rostedt <rostedt@...dmis.org>,
        Steven Price <steven.price@....com>,
        Vlastimil Babka <vbabka@...e.cz>, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        Adam Ruprecht <ruprecht@...gle.com>,
        Cannon Matthews <cannonmatthews@...gle.com>,
        "Dr . David Alan Gilbert" <dgilbert@...hat.com>,
        David Rientjes <rientjes@...gle.com>,
        Oliver Upton <oupton@...gle.com>
Subject: Re: [PATCH 5/9] userfaultfd: add minor fault registration mode

Hi, Axel,

On Fri, Jan 15, 2021 at 11:04:47AM -0800, Axel Rasmussen wrote:
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index c63ccdae3eab..7aa1461e1a8b 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -71,6 +71,11 @@ static inline bool userfaultfd_wp(struct vm_area_struct *vma)
>  	return vma->vm_flags & VM_UFFD_WP;
>  }
>  
> +static inline bool userfaultfd_minor(struct vm_area_struct *vma)
> +{
> +	return vma->vm_flags & VM_UFFD_MINOR;
> +}
> +
>  static inline bool userfaultfd_pte_wp(struct vm_area_struct *vma,
>  				      pte_t pte)
>  {
> @@ -85,7 +90,7 @@ static inline bool userfaultfd_huge_pmd_wp(struct vm_area_struct *vma,
>  
>  static inline bool userfaultfd_armed(struct vm_area_struct *vma)
>  {
> -	return vma->vm_flags & (VM_UFFD_MISSING | VM_UFFD_WP);
> +	return vma->vm_flags & (VM_UFFD_MISSING | VM_UFFD_WP | VM_UFFD_MINOR);
>  }

Maybe move the __VM_UFFD_FLAGS into this header so use it too here?

[...]

> diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h
> index 5f2d88212f7c..1cc2cd8a5279 100644
> --- a/include/uapi/linux/userfaultfd.h
> +++ b/include/uapi/linux/userfaultfd.h
> @@ -19,15 +19,19 @@
>   * means the userland is reading).
>   */
>  #define UFFD_API ((__u64)0xAA)
> +#define UFFD_API_REGISTER_MODES (UFFDIO_REGISTER_MODE_MISSING |	\
> +				 UFFDIO_REGISTER_MODE_WP |	\
> +				 UFFDIO_REGISTER_MODE_MINOR)
>  #define UFFD_API_FEATURES (UFFD_FEATURE_PAGEFAULT_FLAG_WP |	\
>  			   UFFD_FEATURE_EVENT_FORK |		\
>  			   UFFD_FEATURE_EVENT_REMAP |		\
> -			   UFFD_FEATURE_EVENT_REMOVE |	\
> +			   UFFD_FEATURE_EVENT_REMOVE |		\
>  			   UFFD_FEATURE_EVENT_UNMAP |		\
>  			   UFFD_FEATURE_MISSING_HUGETLBFS |	\
>  			   UFFD_FEATURE_MISSING_SHMEM |		\
>  			   UFFD_FEATURE_SIGBUS |		\
> -			   UFFD_FEATURE_THREAD_ID)
> +			   UFFD_FEATURE_THREAD_ID |		\
> +			   UFFD_FEATURE_MINOR_FAULT_HUGETLBFS)

I'd remove the "_FAULT" to align with the missing features...

> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 61d6346ed009..2b3741d6130c 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4377,6 +4377,37 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
>  		}
>  	}
>  
> +	/* Check for page in userfault range. */
> +	if (!new_page && userfaultfd_minor(vma)) {
> +		u32 hash;
> +		struct vm_fault vmf = {
> +			.vma = vma,
> +			.address = haddr,
> +			.flags = flags,
> +			/*
> +			 * Hard to debug if it ends up being used by a callee
> +			 * that assumes something about the other uninitialized
> +			 * fields... same as in memory.c
> +			 */
> +		};
> +
> +		unlock_page(page);
> +
> +		/*
> +		 * hugetlb_fault_mutex and i_mmap_rwsem must be dropped before
> +		 * handling userfault.  Reacquire after handling fault to make
> +		 * calling code simpler.
> +		 */
> +
> +		hash = hugetlb_fault_mutex_hash(mapping, idx);
> +		mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> +		i_mmap_unlock_read(mapping);
> +		ret = handle_userfault(&vmf, VM_UFFD_MINOR);
> +		i_mmap_lock_read(mapping);
> +		mutex_lock(&hugetlb_fault_mutex_table[hash]);
> +		goto out;

I figured it easier if the whole chunk be put into the else block right after
find_lock_page(); will that work the same?

It's just not obviously clear on when we'll go into this block otherwise,
basically the dependency of new_page variable and when it's unset.

Thanks,

-- 
Peter Xu