[PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[Li Xinhai]

This patchset fix the misuse of parenet anon_vma, which mainly caused by
child vma's vm_next and vm_prev are left same as its parent after
duplicate vma. Finally, code reached parent vma's neighbor by referring
pointer of child vma and executed wrong logic.

The first two patches fix relevant issues, and the third patch sets vm_next
and vm_prev to NULL when duplicate vma to prevent potential misuse in future. 

Li Xinhai (3):
  mm: don't prepare anon_vma if vma has VM_WIPEONFORK
  Revert "mm/rmap.c: reuse mergeable anon_vma as parent when fork"
  mm: set vm_next and vm_prev to NULL in vm_area_dup()

 kernel/fork.c | 10 ++++++----
 mm/rmap.c     | 13 -------------
 2 files changed, 6 insertions(+), 17 deletions(-)

-- 
1.8.3.1

[PATCH 1/3] mm: don't prepare anon_vma if vma has VM_WIPEONFORK

[Li Xinhai]

In dup_mmap(), anon_vma_prepare() is called for vma has VM_WIPEONFORK,
and parameter 'tmp' (i.e., the new vma of child) has same ->vm_next and
->vm_prev as its parent vma. That allows anon_vma used by parent been
mistakenly shared by child (find_mergeable_anon_vma() will do this reuse
work).

Besides this issue, call anon_vma_prepare() should be avoided because we
don't copy page for this vma. Preparing anon_vma will be handled during
fault.

Fixes: d2cd9ede6e19 ("mm,fork: introduce MADV_WIPEONFORK")
Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Matthew Wilcox <willy@infradead.org>
---
 kernel/fork.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 0808095..1bbd49a 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -552,10 +552,12 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
 		if (retval)
 			goto fail_nomem_anon_vma_fork;
 		if (tmp->vm_flags & VM_WIPEONFORK) {
-			/* VM_WIPEONFORK gets a clean slate in the child. */
+			/*
+			 * VM_WIPEONFORK gets a clean slate in the child.
+			 * Don't prepare anon_vma until fault since we don't
+			 * copy page for current vma.
+			 */
 			tmp->anon_vma = NULL;
-			if (anon_vma_prepare(tmp))
-				goto fail_nomem_anon_vma_fork;
 		} else if (anon_vma_fork(tmp, mpnt))
 			goto fail_nomem_anon_vma_fork;
 		tmp->vm_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
-- 
1.8.3.1

[PATCH 2/3] Revert "mm/rmap.c: reuse mergeable anon_vma as parent when fork"

[Li Xinhai]

This reverts commit 4e4a9eb921332b9d1edd99f76998f99f36b195f7
In dup_mmap(), anon_vma_fork() is called for attaching anon_vma and
parameter 'tmp'  (i.e., the new vma of child) has same ->vm_next and
->vm_prev as its parent vma. That causes the anon_vma used by parent been
mistakenly shared by child (In anon_vma_clone(), the code added by that
commit will do this reuse work).

Besides this issue, the design of reusing anon_vma from vma which has
gone through fork should be avoided ([1]). So, this patch reverts that
commit and maintains the consistent logic of reusing anon_vma for
fork/split/merge vma.

[1] commit d0e9fe1758f2 ("Simplify and comment on anon_vma re-use for
    anon_vma_prepare()") explains the test of "list_is_singular()".

Fixes: 4e4a9eb92133 ("mm/rmap.c: reuse mergeable anon_vma as parent when fork")
Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/rmap.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index b3e3819..861435b 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -269,19 +269,6 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
 {
 	struct anon_vma_chain *avc, *pavc;
 	struct anon_vma *root = NULL;
-	struct vm_area_struct *prev = dst->vm_prev, *pprev = src->vm_prev;
-
-	/*
-	 * If parent share anon_vma with its vm_prev, keep this sharing in in
-	 * child.
-	 *
-	 * 1. Parent has vm_prev, which implies we have vm_prev.
-	 * 2. Parent and its vm_prev have the same anon_vma.
-	 */
-	if (!dst->anon_vma && src->anon_vma &&
-	    pprev && pprev->anon_vma == src->anon_vma)
-		dst->anon_vma = prev->anon_vma;
-
 
 	list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
 		struct anon_vma *anon_vma;
-- 
1.8.3.1

[PATCH 3/3] mm: set vm_next and vm_prev to NULL in vm_area_dup()

[Li Xinhai]

Set ->vm_next and ->vm_prev to NULL to prevent potential misuse from
the new duplicated vma.

Currently, only in fork path there are misuse for handling anon_vma. No
other bugs been revealed with this patch applied.

Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 1bbd49a..eb3a148 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -361,6 +361,7 @@ struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig)
 	if (new) {
 		*new = *orig;
 		INIT_LIST_HEAD(&new->anon_vma_chain);
+		new->vm_next = new->vm_prev = NULL;
 	}
 	return new;
 }
@@ -561,7 +562,6 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
 		} else if (anon_vma_fork(tmp, mpnt))
 			goto fail_nomem_anon_vma_fork;
 		tmp->vm_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
-		tmp->vm_next = tmp->vm_prev = NULL;
 		file = tmp->vm_file;
 		if (file) {
 			struct inode *inode = file_inode(file);
-- 
1.8.3.1

Re: [PATCH 1/3] mm: don't prepare anon_vma if vma has VM_WIPEONFORK

[Li Xinhai]

sorry, should send to Rik van Riel<riel@surriel.com>

On 2020-02-08 at 16:35 Li Xinhai wrote:
>In dup_mmap(), anon_vma_prepare() is called for vma has VM_WIPEONFORK,
>and parameter 'tmp' (i.e., the new vma of child) has same ->vm_next and
>->vm_prev as its parent vma. That allows anon_vma used by parent been
>mistakenly shared by child (find_mergeable_anon_vma() will do this reuse
>work).
>
>Besides this issue, call anon_vma_prepare() should be avoided because we
>don't copy page for this vma. Preparing anon_vma will be handled during
>fault.
>
>Fixes: d2cd9ede6e19 ("mm,fork: introduce MADV_WIPEONFORK")
>Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
>Cc: Rik van Riel <riel@redhat.com>
>Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>Cc: Matthew Wilcox <willy@infradead.org>
>---
> kernel/fork.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/kernel/fork.c b/kernel/fork.c
>index 0808095..1bbd49a 100644
>--- a/kernel/fork.c
>+++ b/kernel/fork.c
>@@ -552,10 +552,12 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
> if (retval)
> goto fail_nomem_anon_vma_fork;
> if (tmp->vm_flags & VM_WIPEONFORK) {
>-	/* VM_WIPEONFORK gets a clean slate in the child. */
>+	/*
>+	* VM_WIPEONFORK gets a clean slate in the child.
>+	* Don't prepare anon_vma until fault since we don't
>+	* copy page for current vma.
>+	*/
> tmp->anon_vma = NULL;
>-	if (anon_vma_prepare(tmp))
>-	goto fail_nomem_anon_vma_fork;
> } else if (anon_vma_fork(tmp, mpnt))
> goto fail_nomem_anon_vma_fork;
> tmp->vm_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
>--
>1.8.3.1
>

Re: [PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[Andrew Morton]

On Sat,  8 Feb 2020 08:35:25 +0000 Li Xinhai <lixinhai.lxh@gmail.com> wrote:

> This patchset fix the misuse of parenet anon_vma, which mainly caused by
> child vma's vm_next and vm_prev are left same as its parent after
> duplicate vma. Finally, code reached parent vma's neighbor by referring
> pointer of child vma and executed wrong logic.
> 
> The first two patches fix relevant issues, and the third patch sets vm_next
> and vm_prev to NULL when duplicate vma to prevent potential misuse in future. 

What are the runtime effects of this bug?  How is the bug triggered?

Re: [PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[Li Xinhai]

On 2020-02-10 at 08:52 Andrew Morton wrote:
>On Sat,  8 Feb 2020 08:35:25 +0000 Li Xinhai <lixinhai.lxh@gmail.com> wrote:
>
>> This patchset fix the misuse of parenet anon_vma, which mainly caused by
>> child vma's vm_next and vm_prev are left same as its parent after
>> duplicate vma. Finally, code reached parent vma's neighbor by referring
>> pointer of child vma and executed wrong logic.
>>
>> The first two patches fix relevant issues, and the third patch sets vm_next
>> and vm_prev to NULL when duplicate vma to prevent potential misuse in future.
>
>What are the runtime effects of this bug?  How is the bug triggered? 

Effects of the first one is that causes ramp code to check both parent and
child's page table, although a page couldn't be mapped by both parent
and child, because child vma has WIPEONFORK so all pages mapped by child
are 'new' and not relevant to parent.

Effects of the second one is that the relationship of anon_vma of parent and
child are totally convoluted. It would cause 'son', 'grandson', ..., etc, to share
'parent' anon_vma, which disobey the design rule of  reusing anon_vma (the
rule to be followed is that reusing should among vma of same process, and
vma should not gone through fork).

So, both issues should cause unnecessary rmap walking and have unexpected
complexity.

These two issues would not be directly visible, I used debugging code to check
the anon_vma pointers of parent and child when inspecting the suspicious
implementation of issue #2, then find the problem.

Re: [PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[Andrew Morton]

On Sat,  8 Feb 2020 08:35:25 +0000 Li Xinhai <lixinhai.lxh@gmail.com> wrote:

> This patchset fix the misuse of parenet anon_vma, which mainly caused by
> child vma's vm_next and vm_prev are left same as its parent after
> duplicate vma. Finally, code reached parent vma's neighbor by referring
> pointer of child vma and executed wrong logic.
> 
> The first two patches fix relevant issues, and the third patch sets vm_next
> and vm_prev to NULL when duplicate vma to prevent potential misuse in future. 
> 
> Li Xinhai (3):
>   mm: don't prepare anon_vma if vma has VM_WIPEONFORK
>   Revert "mm/rmap.c: reuse mergeable anon_vma as parent when fork"
>   mm: set vm_next and vm_prev to NULL in vm_area_dup()
> 
>  kernel/fork.c | 10 ++++++----
>  mm/rmap.c     | 13 -------------
>  2 files changed, 6 insertions(+), 17 deletions(-)

Can we please have some review input on this series?

Re: [PATCH 1/3] mm: don't prepare anon_vma if vma has VM_WIPEONFORK

[Kirill A. Shutemov]

On Sat, Feb 08, 2020 at 08:35:26AM +0000, Li Xinhai wrote:
> In dup_mmap(), anon_vma_prepare() is called for vma has VM_WIPEONFORK,
> and parameter 'tmp' (i.e., the new vma of child) has same ->vm_next and
> ->vm_prev as its parent vma. That allows anon_vma used by parent been
> mistakenly shared by child (find_mergeable_anon_vma() will do this reuse
> work).
> 
> Besides this issue, call anon_vma_prepare() should be avoided because we
> don't copy page for this vma. Preparing anon_vma will be handled during
> fault.
> 
> Fixes: d2cd9ede6e19 ("mm,fork: introduce MADV_WIPEONFORK")
> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Matthew Wilcox <willy@infradead.org>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

Re: [PATCH 2/3] Revert "mm/rmap.c: reuse mergeable anon_vma as parent when fork"

[Kirill A. Shutemov]

On Sat, Feb 08, 2020 at 08:35:27AM +0000, Li Xinhai wrote:
> This reverts commit 4e4a9eb921332b9d1edd99f76998f99f36b195f7
> In dup_mmap(), anon_vma_fork() is called for attaching anon_vma and
> parameter 'tmp'  (i.e., the new vma of child) has same ->vm_next and
> ->vm_prev as its parent vma. That causes the anon_vma used by parent been
> mistakenly shared by child (In anon_vma_clone(), the code added by that
> commit will do this reuse work).
> 
> Besides this issue, the design of reusing anon_vma from vma which has
> gone through fork should be avoided ([1]). So, this patch reverts that
> commit and maintains the consistent logic of reusing anon_vma for
> fork/split/merge vma.
> 
> [1] commit d0e9fe1758f2 ("Simplify and comment on anon_vma re-use for
>     anon_vma_prepare()") explains the test of "list_is_singular()".

I read the description few time, but I cannot say I understood the
problem completely. Do you have a test-case to demonstrate the issue?

IIUC, re-using anon_vma across fork is wrong, but within the process is
fine, right?

Maybe we should just check that dst->vm_mm matches src->vm_mm before
re-using anon_vma?

-- 
 Kirill A. Shutemov

Re: [PATCH 3/3] mm: set vm_next and vm_prev to NULL in vm_area_dup()

[Kirill A. Shutemov]

On Sat, Feb 08, 2020 at 08:35:28AM +0000, Li Xinhai wrote:
> Set ->vm_next and ->vm_prev to NULL to prevent potential misuse from
> the new duplicated vma.
> 
> Currently, only in fork path there are misuse for handling anon_vma. No
> other bugs been revealed with this patch applied.
> 
> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Johannes Weiner <hannes@cmpxchg.org>

Looks okay to me.

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

Re: [PATCH 2/3] Revert "mm/rmap.c: reuse mergeable anon_vma as parent when fork"

[Li Xinhai]

On 2020-04-02 at 21:59 Kirill A. Shutemov wrote:
>On Sat, Feb 08, 2020 at 08:35:27AM +0000, Li Xinhai wrote:
>> This reverts commit 4e4a9eb921332b9d1edd99f76998f99f36b195f7
>> In dup_mmap(), anon_vma_fork() is called for attaching anon_vma and
>> parameter 'tmp'  (i.e., the new vma of child) has same ->vm_next and
>> ->vm_prev as its parent vma. That causes the anon_vma used by parent been
>> mistakenly shared by child (In anon_vma_clone(), the code added by that
>> commit will do this reuse work).
>>
>> Besides this issue, the design of reusing anon_vma from vma which has
>> gone through fork should be avoided ([1]). So, this patch reverts that
>> commit and maintains the consistent logic of reusing anon_vma for
>> fork/split/merge vma.
>>
>> [1] commit d0e9fe1758f2 ("Simplify and comment on anon_vma re-use for
>>     anon_vma_prepare()") explains the test of "list_is_singular()".
>
>I read the description few time, but I cannot say I understood the
>problem completely. Do you have a test-case to demonstrate the issue?
>
>IIUC, re-using anon_vma across fork is wrong, but within the process is
>fine, right?
> 

Yes, re-using anon_vma within the process is fine. But if a vma has gone through
fork(), then that vma's anon_vma should not be shared with its neighbor vma.
As explained in [1], when vma gone through fork(), the check for 
list_is_singular(vma->anon_vma_chain) will be false, and don't share anon_vma.

With current issue, one example can clarify more:
parent process do below two steps
1. p_vma_1 is created and p_anon_vma_1 is prepared;
2. p_vma_2 is created and share p_anon_vma_1; (this is allowed, becaues p_vma_1
didn't go through fork());
parent process do fork():
3. c_vma_1 is dup from p_vma_1, and has its own c_anon_vma_1 prepared; at this point,
c_vma_1->anon_vma_chain has two items, one for p_anon_vma_1 and one for
c_anon_vma_1;
4. c_vma_2 is dup from p_vma_2, it is not allowed to share c_anon_vma_1, because 
c_vma_1->anon_vma_chain has two items. 

>Maybe we should just check that dst->vm_mm matches src->vm_mm before
>re-using anon_vma? 

This don't help, iin fork() path, dst->vm_mm always don't match src->vm_mm. 

>
>--
> Kirill A. Shutemov

Re: [PATCH 2/3] Revert "mm/rmap.c: reuse mergeable anon_vma as parent when fork"

[Li Xinhai]

On 2020-04-03 at 11:08 Li Xinhai wrote:
>On 2020-04-02 at 21:59 Kirill A. Shutemov wrote:
>>On Sat, Feb 08, 2020 at 08:35:27AM +0000, Li Xinhai wrote:
>>> This reverts commit 4e4a9eb921332b9d1edd99f76998f99f36b195f7
>>> In dup_mmap(), anon_vma_fork() is called for attaching anon_vma and
>>> parameter 'tmp'  (i.e., the new vma of child) has same ->vm_next and
>>> ->vm_prev as its parent vma. That causes the anon_vma used by parent been
>>> mistakenly shared by child (In anon_vma_clone(), the code added by that
>>> commit will do this reuse work).
>>>
>>> Besides this issue, the design of reusing anon_vma from vma which has
>>> gone through fork should be avoided ([1]). So, this patch reverts that
>>> commit and maintains the consistent logic of reusing anon_vma for
>>> fork/split/merge vma.
>>>
>>> [1] commit d0e9fe1758f2 ("Simplify and comment on anon_vma re-use for
>>>     anon_vma_prepare()") explains the test of "list_is_singular()".
>>
>>I read the description few time, but I cannot say I understood the
>>problem completely. Do you have a test-case to demonstrate the issue?
>>
>>IIUC, re-using anon_vma across fork is wrong, but within the process is
>>fine, right?
>>
>
>Yes, re-using anon_vma within the process is fine. But if a vma has gone through
>fork(), then that vma's anon_vma should not be shared with its neighbor vma.
>As explained in [1], when vma gone through fork(), the check for
>list_is_singular(vma->anon_vma_chain) will be false, and don't share anon_vma.
>
>With current issue, one example can clarify more:
>parent process do below two steps
>1. p_vma_1 is created and p_anon_vma_1 is prepared;
>2. p_vma_2 is created and share p_anon_vma_1; (this is allowed, becaues p_vma_1
>didn't go through fork());
>parent process do fork():
>3. c_vma_1 is dup from p_vma_1, and has its own c_anon_vma_1 prepared; at this point,
>c_vma_1->anon_vma_chain has two items, one for p_anon_vma_1 and one for
>c_anon_vma_1;
>4. c_vma_2 is dup from p_vma_2, it is not allowed to share c_anon_vma_1, because 
>c_vma_1->anon_vma_chain has two items. 
>
>>Maybe we should just check that dst->vm_mm matches src->vm_mm before
>>re-using anon_vma?
>
>This don't help, iin fork() path, dst->vm_mm always don't match src->vm_mm. 
> 

If ->vm_next and ->vm_prev have been set as NULL in vm_area_dup(), the existing
code will crash immediately by dereference NULL point (i.e., prev is NULL, and use
prev->anon_vma), instead of getting involved in parent anon_vma.

I don't choose to fix this NULL pointer dereference, but revert the commit because
of the reason as described in the last paragraph of commit message.

Andrew, please consider merge this patch as the existing code has no value to
remain.

>>
>>--
>> Kirill A. Shutemov 

Re: [PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[Michal Hocko]

On Mon 10-02-20 10:15:53, Li Xinhai wrote:
[...]
> >What are the runtime effects of this bug?  How is the bug triggered? 
> 
> Effects of the first one is that causes ramp code to check both parent and
> child's page table, although a page couldn't be mapped by both parent
> and child, because child vma has WIPEONFORK so all pages mapped by child
> are 'new' and not relevant to parent.
> 
> Effects of the second one is that the relationship of anon_vma of parent and
> child are totally convoluted. It would cause 'son', 'grandson', ..., etc, to share
> 'parent' anon_vma, which disobey the design rule of  reusing anon_vma (the
> rule to be followed is that reusing should among vma of same process, and
> vma should not gone through fork).
> 
> So, both issues should cause unnecessary rmap walking and have unexpected
> complexity.
> 
> These two issues would not be directly visible, I used debugging code to check
> the anon_vma pointers of parent and child when inspecting the suspicious
> implementation of issue #2, then find the problem.

I am still not completely clear on the user effect. Does this allow an
adversary to generate too long anon_vma chains that wouldn't normally
happen or some other nasties that would make backporting to older
kernels really necessary? Because from my current understanding it would
only make WIPEONFORK vmas rmap walks less efficient. Or is there any
other functional issue?
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[Li Xinhai]

On 2020-04-20 at 20:06 Michal Hocko wrote:
>On Mon 10-02-20 10:15:53, Li Xinhai wrote:
>[...]
>> >What are the runtime effects of this bug?  How is the bug triggered?
>>
>> Effects of the first one is that causes ramp code to check both parent and
>> child's page table, although a page couldn't be mapped by both parent
>> and child, because child vma has WIPEONFORK so all pages mapped by child
>> are 'new' and not relevant to parent.
>>
>> Effects of the second one is that the relationship of anon_vma of parent and
>> child are totally convoluted. It would cause 'son', 'grandson', ..., etc, to share
>> 'parent' anon_vma, which disobey the design rule of  reusing anon_vma (the
>> rule to be followed is that reusing should among vma of same process, and
>> vma should not gone through fork).
>>
>> So, both issues should cause unnecessary rmap walking and have unexpected
>> complexity.
>>
>> These two issues would not be directly visible, I used debugging code to check
>> the anon_vma pointers of parent and child when inspecting the suspicious
>> implementation of issue #2, then find the problem.
>
>I am still not completely clear on the user effect. Does this allow an
>adversary to generate too long anon_vma chains that wouldn't normally
>happen or some other nasties that would make backporting to older
>kernels really necessary? 
It allows for generating long anon_vma chain by keep forking child process, although
the memory consumed for anon_vma structure wouldn't increase accordingly. I don't
see other impact of those two issues. 

> Because from my current understanding it would
>only make WIPEONFORK vmas rmap walks less efficient. Or is there any
>other functional issue? 
#1 issue is for case WIPEONFORK, #2 issue for all other cases of anonymous VMA dup
(i.e., when parent vmas is currently sharing anon_vma among themself, child vma will
share its parent vma's anon_vma).
Both cause inefficient rmap walking, no other functional issues.

In my understanding, this patchset can be backported to stable kernel. E.g., If someone mmap
a big vm area, and split that vma to several adjacent vmas(e.g., by mprotect), then
sharing anon_vma at parent process level is achivied. After that, begin fork child, maybe
choose to keep forking child from the top parent process, or let child process keep forking
its own child. At this moment, sharing anon_vma among children and parent is achieved
and has long chain.

>--
>Michal Hocko
>SUSE Labs

Re: [PATCH 0/3] mm: Fix misuse of parent anon_vma in dup_mmap path

[Michal Hocko]

On Tue 21-04-20 11:53:02, Li Xinhai wrote:
> On 2020-04-20 at 20:06 Michal Hocko wrote:
[...]
> > Because from my current understanding it would
> >only make WIPEONFORK vmas rmap walks less efficient. Or is there any
> >other functional issue? 
> #1 issue is for case WIPEONFORK, #2 issue for all other cases of anonymous VMA dup
> (i.e., when parent vmas is currently sharing anon_vma among themself, child vma will
> share its parent vma's anon_vma).

OK, it was not really clear to me that also !WIPEONFORK vmas are
affected. Thanks for the clarification.

-- 
Michal Hocko
SUSE Labs