diff mbox series

[v2,15/26] userfaultfd: wp: drop _PAGE_UFFD_WP properly when fork

Message ID 20190212025632.28946-16-peterx@redhat.com (mailing list archive)
State New, archived
Headers show
Series userfaultfd: write protection support | expand

Commit Message

Peter Xu Feb. 12, 2019, 2:56 a.m. UTC
UFFD_EVENT_FORK support for uffd-wp should be already there, except
that we should clean the uffd-wp bit if uffd fork event is not
enabled.  Detect that to avoid _PAGE_UFFD_WP being set even if the VMA
is not being tracked by VM_UFFD_WP.  Do this for both small PTEs and
huge PMDs.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 mm/huge_memory.c | 8 ++++++++
 mm/memory.c      | 8 ++++++++
 2 files changed, 16 insertions(+)

Comments

Jerome Glisse Feb. 21, 2019, 6:06 p.m. UTC | #1
On Tue, Feb 12, 2019 at 10:56:21AM +0800, Peter Xu wrote:
> UFFD_EVENT_FORK support for uffd-wp should be already there, except
> that we should clean the uffd-wp bit if uffd fork event is not
> enabled.  Detect that to avoid _PAGE_UFFD_WP being set even if the VMA
> is not being tracked by VM_UFFD_WP.  Do this for both small PTEs and
> huge PMDs.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

This patch must be earlier in the serie, before the patch that introduce
the userfaultfd API so that bisect can not end up on version where this
can happen.

Otherwise the patch itself is:

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  mm/huge_memory.c | 8 ++++++++
>  mm/memory.c      | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 817335b443c2..fb2234cb595a 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -938,6 +938,14 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  	ret = -EAGAIN;
>  	pmd = *src_pmd;
>  
> +	/*
> +	 * Make sure the _PAGE_UFFD_WP bit is cleared if the new VMA
> +	 * does not have the VM_UFFD_WP, which means that the uffd
> +	 * fork event is not enabled.
> +	 */
> +	if (!(vma->vm_flags & VM_UFFD_WP))
> +		pmd = pmd_clear_uffd_wp(pmd);
> +
>  #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
>  	if (unlikely(is_swap_pmd(pmd))) {
>  		swp_entry_t entry = pmd_to_swp_entry(pmd);
> diff --git a/mm/memory.c b/mm/memory.c
> index b5d67bafae35..c2035539e9fd 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -788,6 +788,14 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  		pte = pte_mkclean(pte);
>  	pte = pte_mkold(pte);
>  
> +	/*
> +	 * Make sure the _PAGE_UFFD_WP bit is cleared if the new VMA
> +	 * does not have the VM_UFFD_WP, which means that the uffd
> +	 * fork event is not enabled.
> +	 */
> +	if (!(vm_flags & VM_UFFD_WP))
> +		pte = pte_clear_uffd_wp(pte);
> +
>  	page = vm_normal_page(vma, addr, pte);
>  	if (page) {
>  		get_page(page);
> -- 
> 2.17.1
>
Peter Xu Feb. 22, 2019, 9:09 a.m. UTC | #2
On Thu, Feb 21, 2019 at 01:06:31PM -0500, Jerome Glisse wrote:
> On Tue, Feb 12, 2019 at 10:56:21AM +0800, Peter Xu wrote:
> > UFFD_EVENT_FORK support for uffd-wp should be already there, except
> > that we should clean the uffd-wp bit if uffd fork event is not
> > enabled.  Detect that to avoid _PAGE_UFFD_WP being set even if the VMA
> > is not being tracked by VM_UFFD_WP.  Do this for both small PTEs and
> > huge PMDs.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> This patch must be earlier in the serie, before the patch that introduce
> the userfaultfd API so that bisect can not end up on version where this
> can happen.

Yes it should be now? Since the API will be introduced until patch
21/26 ("userfaultfd: wp: add the writeprotect API to userfaultfd
ioctl").

> 
> Otherwise the patch itself is:
> 
> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

Unless I found anything I've missed above... I'll temporarily pick
this R-b for now then.

Thanks,
Jerome Glisse Feb. 22, 2019, 3:36 p.m. UTC | #3
On Fri, Feb 22, 2019 at 05:09:19PM +0800, Peter Xu wrote:
> On Thu, Feb 21, 2019 at 01:06:31PM -0500, Jerome Glisse wrote:
> > On Tue, Feb 12, 2019 at 10:56:21AM +0800, Peter Xu wrote:
> > > UFFD_EVENT_FORK support for uffd-wp should be already there, except
> > > that we should clean the uffd-wp bit if uffd fork event is not
> > > enabled.  Detect that to avoid _PAGE_UFFD_WP being set even if the VMA
> > > is not being tracked by VM_UFFD_WP.  Do this for both small PTEs and
> > > huge PMDs.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > 
> > This patch must be earlier in the serie, before the patch that introduce
> > the userfaultfd API so that bisect can not end up on version where this
> > can happen.
> 
> Yes it should be now? Since the API will be introduced until patch
> 21/26 ("userfaultfd: wp: add the writeprotect API to userfaultfd
> ioctl").

No i was confuse when reading this patch i had the feeling it was
after the ioctl ignore my comment.

> 
> > 
> > Otherwise the patch itself is:
> > 
> > Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
> 
> Unless I found anything I've missed above... I'll temporarily pick
> this R-b for now then.

It is fine, the patch ordering was my confusion.

Cheers,
Jérôme
Mike Rapoport Feb. 25, 2019, 6:19 p.m. UTC | #4
On Tue, Feb 12, 2019 at 10:56:21AM +0800, Peter Xu wrote:
> UFFD_EVENT_FORK support for uffd-wp should be already there, except
> that we should clean the uffd-wp bit if uffd fork event is not
> enabled.  Detect that to avoid _PAGE_UFFD_WP being set even if the VMA
> is not being tracked by VM_UFFD_WP.  Do this for both small PTEs and
> huge PMDs.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
>  mm/huge_memory.c | 8 ++++++++
>  mm/memory.c      | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 817335b443c2..fb2234cb595a 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -938,6 +938,14 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  	ret = -EAGAIN;
>  	pmd = *src_pmd;
> 
> +	/*
> +	 * Make sure the _PAGE_UFFD_WP bit is cleared if the new VMA
> +	 * does not have the VM_UFFD_WP, which means that the uffd
> +	 * fork event is not enabled.
> +	 */
> +	if (!(vma->vm_flags & VM_UFFD_WP))
> +		pmd = pmd_clear_uffd_wp(pmd);
> +
>  #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
>  	if (unlikely(is_swap_pmd(pmd))) {
>  		swp_entry_t entry = pmd_to_swp_entry(pmd);
> diff --git a/mm/memory.c b/mm/memory.c
> index b5d67bafae35..c2035539e9fd 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -788,6 +788,14 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  		pte = pte_mkclean(pte);
>  	pte = pte_mkold(pte);
> 
> +	/*
> +	 * Make sure the _PAGE_UFFD_WP bit is cleared if the new VMA
> +	 * does not have the VM_UFFD_WP, which means that the uffd
> +	 * fork event is not enabled.
> +	 */
> +	if (!(vm_flags & VM_UFFD_WP))
> +		pte = pte_clear_uffd_wp(pte);
> +
>  	page = vm_normal_page(vma, addr, pte);
>  	if (page) {
>  		get_page(page);
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 817335b443c2..fb2234cb595a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -938,6 +938,14 @@  int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 	ret = -EAGAIN;
 	pmd = *src_pmd;
 
+	/*
+	 * Make sure the _PAGE_UFFD_WP bit is cleared if the new VMA
+	 * does not have the VM_UFFD_WP, which means that the uffd
+	 * fork event is not enabled.
+	 */
+	if (!(vma->vm_flags & VM_UFFD_WP))
+		pmd = pmd_clear_uffd_wp(pmd);
+
 #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
 	if (unlikely(is_swap_pmd(pmd))) {
 		swp_entry_t entry = pmd_to_swp_entry(pmd);
diff --git a/mm/memory.c b/mm/memory.c
index b5d67bafae35..c2035539e9fd 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -788,6 +788,14 @@  copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 		pte = pte_mkclean(pte);
 	pte = pte_mkold(pte);
 
+	/*
+	 * Make sure the _PAGE_UFFD_WP bit is cleared if the new VMA
+	 * does not have the VM_UFFD_WP, which means that the uffd
+	 * fork event is not enabled.
+	 */
+	if (!(vm_flags & VM_UFFD_WP))
+		pte = pte_clear_uffd_wp(pte);
+
 	page = vm_normal_page(vma, addr, pte);
 	if (page) {
 		get_page(page);