[PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Jinjiang Tu]

On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
anon_vma->root is stale due to missing memory barrier, leading to
lock and unlock two different anon_vma->root, thus leading to a anon_vma
will never be unlocked, and another anon_vma couldn't be locked anymore.

The race is as follows:

    THREAD A                             THREAD B
__anon_vma_prepare                __anon_vma_prepare
 anon_vma = anon_vma_alloc();
 // writes may out of order here
 vma->anon_vma = anon_vma;
                                   anon_vma = find_mergeable_anon_vma(vma);
                                   anon_vma_lock_write(anon_vma);
                                     // may still see the old root
                                     down_write(&anon_vma->root->rwsem);
                                   anon_vma_unlock_write(anon_vma);
                                     // see the new root, never unlock old
                                     up_write(&anon_vma->root->rwsem);

thread A triggers page fault and calls __anon_vma_prepare() to prepare
anon_vma for the faulting vma. __anon_vma_prepare() allocates and
initializes a new anon_vma, and then publishes it to the vma with a plain
store. anon_vma_prepare() only requires the mmap_lock to be held for
reading, so two threads can fault on adjacent VMAs at the same time. While
thread A publishes a new anon_vma, thread B could finds the anon_vma via
find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.

However, due to missing barrier, thread B can observe the published pointer
but a stale anon_vma->root because the stores from anon_vma_alloc() aren't
yet visible. What's the value of the stale anon_vma->root? __put_anon_vma()
doesn't clear anon_vma->root, so the root of the new allocated anon_vma
may point to a valid anon_vma.

As a result, thread B can call anon_vma_lock_write() with the old root,
and call anon_vma_unlock_write() with the new root, leading to a anon_vma
will never be unlocked, and another anon_vma couldn't be locked anymore
(it's count is dropped from 0 to -1 due to wrong unlock).

To fix it, change the plain store `vma->anon_vma = anon_vma` to store
release, so that the fields of anon_vma are visible before anon_vma is
published to vma->anon_vma.

We don't need a read barrier at read side for thread B. The load of
anon_vma and anon_vma->root have address-dependency. According to
Documentation/memory-barriers.txt and some investigations, only Alpha
needs address-dependency barriers and it has been handled by READ_ONCE().

This issue needs two adjacent VMAs aren't merged but are compatible for
anon_vma. We reproduced this issue in v5.10 with KSM enabled. The kernel
doesn't merge commit cf7e7a3503df ("mm: prevent KSM from breaking VMA
merging for new VMAs"), so there are many adjacent VMAs that aren't merged
but are compatible for anon_vma.

Without this fix, our production environment could reproduce this issue
about 2-5 times each month. After adding a smp_mb() before
anon_vma_lock_write(anon_vma) in __anon_vma_prepare(), which is different
to this patch, this issue hasn't be reproduced for one month.

Cc: stable@vger.kernel.org
Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")
Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 mm/rmap.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index d1819fd69938..a868e835eadb 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
 	/* page_table_lock to protect against threads */
 	spin_lock(&mm->page_table_lock);
 	if (likely(!vma->anon_vma)) {
-		vma->anon_vma = anon_vma;
+		/*
+		 * The fields of anon_vma must be visible before anon_vma
+		 * is published to vma->anon_vma.
+		 */
+		smp_store_release(&vma->anon_vma, anon_vma);
 		anon_vma_chain_assign(vma, avc, anon_vma);
 		anon_rmap_tree_insert(avc, anon_vma);
 		anon_vma->num_active_vmas++;
-- 
2.43.0

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Rik van Riel]

On Sat, 2026-09-05 at 14:18 +0800, Jinjiang Tu wrote:
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index d1819fd69938..a868e835eadb 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct
> *vma)
>  	/* page_table_lock to protect against threads */
>  	spin_lock(&mm->page_table_lock);
>  	if (likely(!vma->anon_vma)) {
> -		vma->anon_vma = anon_vma;
> +		/*
> +		 * The fields of anon_vma must be visible before
> anon_vma
> +		 * is published to vma->anon_vma.
> +		 */
> +		smp_store_release(&vma->anon_vma, anon_vma);
>  		anon_vma_chain_assign(vma, avc, anon_vma);
>  		anon_rmap_tree_insert(avc, anon_vma);
>  		anon_vma->num_active_vmas++;

Nice catch on this bug!

This is perhaps a dumb question, but does this
write side barrier need to pair with a read
side barrier, to ensure the reads are also
correctly ordered?

-- 
All Rights Reversed.

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Jinjiang Tu]

在 2026/9/5 21:22, Rik van Riel 写道:
> On Sat, 2026-09-05 at 14:18 +0800, Jinjiang Tu wrote:
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index d1819fd69938..a868e835eadb 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct
>> *vma)
>>   	/* page_table_lock to protect against threads */
>>   	spin_lock(&mm->page_table_lock);
>>   	if (likely(!vma->anon_vma)) {
>> -		vma->anon_vma = anon_vma;
>> +		/*
>> +		 * The fields of anon_vma must be visible before
>> anon_vma
>> +		 * is published to vma->anon_vma.
>> +		 */
>> +		smp_store_release(&vma->anon_vma, anon_vma);
>>   		anon_vma_chain_assign(vma, avc, anon_vma);
>>   		anon_rmap_tree_insert(avc, anon_vma);
>>   		anon_vma->num_active_vmas++;
> Nice catch on this bug!
>
> This is perhaps a dumb question, but does this
> write side barrier need to pair with a read
> side barrier, to ensure the reads are also
> correctly ordered?

Hi, I have described it in the changelog:

We don't need a read barrier at read side for thread B. The load of
anon_vma and anon_vma->root have address-dependency. According to
Documentation/memory-barriers.txt and some investigations, only Alpha
needs address-dependency barriers and it has been handled by READ_ONCE().

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Andrew Morton]

On Sat, 5 Sep 2026 14:18:19 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:

> On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
> anon_vma->root is stale due to missing memory barrier, leading to
> lock and unlock two different anon_vma->root, thus leading to a anon_vma
> will never be unlocked, and another anon_vma couldn't be locked anymore.
> 
> The race is as follows:
>
> ...
> 
> Without this fix, our production environment could reproduce this issue
> about 2-5 times each month.

That's important info.  Can you tell us more?  How was this observed by
operations people?  A copy-n-paste of the kernel messages would be helpful.

This will help downstream people to decide whether this patch fixes a
thing they're seeing happen.

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Jinjiang Tu]

在 2026/9/6 7:21, Andrew Morton 写道:
> On Sat, 5 Sep 2026 14:18:19 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
>
>> On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
>> anon_vma->root is stale due to missing memory barrier, leading to
>> lock and unlock two different anon_vma->root, thus leading to a anon_vma
>> will never be unlocked, and another anon_vma couldn't be locked anymore.
>>
>> The race is as follows:
>>
>> ...
>>
>> Without this fix, our production environment could reproduce this issue
>> about 2-5 times each month.
> That's important info.  Can you tell us more?  How was this observed by
> operations people?  A copy-n-paste of the kernel messages would be helpful.
>
> This will help downstream people to decide whether this patch fixes a
> thing they're seeing happen.

We can see a task that tries to grab anon_vma lock triggers hungtask.

[2434968.289510] INFO: task main:2354726 blocked for more than 120 seconds.
[2434968.289516]       Tainted: G            E     5.10.0-0021.aarch64 #1
[2434968.289517] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[2434968.289519] task:main            state:D stack:    0 pid:2354726 ppid:2350673 flags:0x00000a01
[2434968.289523] Call trace:
[2434968.289531]  __switch_to+0x7c/0xbc
[2434968.289540]  __schedule+0x3b4/0x8a0
[2434968.289542]  schedule+0x50/0xe0
[2434968.289545]  rwsem_down_write_slowpath+0x3cc/0x6cc
[2434968.289547]  down_write+0x60/0x260
[2434968.289551]  __anon_vma_prepare+0x6c/0x210
[2434968.289555]  do_anonymous_page+0x258/0x660
[2434968.289557]  handle_pte_fault+0x188/0x214
[2434968.289559]  __handle_mm_fault+0x1b0/0x380
[2434968.289561]  handle_mm_fault+0xf4/0x284
[2434968.289563]  do_page_fault+0x19c/0x494
[2434968.289565]  do_translation_fault+0xcc/0xf8
[2434968.289569]  do_mem_abort+0x48/0xac
[2434968.289570]  el0_da+0x44/0x80
[2434968.289572]  el0_sync_handler+0x88/0xb4
[2434968.289573]  el0_sync+0x160/0x180

After analyzing the vmcore, we found the anon_vma->root->rwsem.count is -1,
and there is another anon_vma whose anon_vma->root->rwsem.count is 1, and
the anon_vma->root->rwsem.owner shows the lock is held, but the stack of
the task shows the task doesn't hold the anon_vma lock.

>

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Lorenzo Stoakes (ARM)]

On Mon, Sep 07, 2026 at 10:21:34AM +0800, Jinjiang Tu wrote:
>
> 在 2026/9/6 7:21, Andrew Morton 写道:
> > On Sat, 5 Sep 2026 14:18:19 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
> >
> > > On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
> > > anon_vma->root is stale due to missing memory barrier, leading to
> > > lock and unlock two different anon_vma->root, thus leading to a anon_vma
> > > will never be unlocked, and another anon_vma couldn't be locked anymore.
> > >
> > > The race is as follows:
> > >
> > > ...
> > >
> > > Without this fix, our production environment could reproduce this issue
> > > about 2-5 times each month.
> > That's important info.  Can you tell us more?  How was this observed by
> > operations people?  A copy-n-paste of the kernel messages would be helpful.
> >
> > This will help downstream people to decide whether this patch fixes a
> > thing they're seeing happen.
>
> We can see a task that tries to grab anon_vma lock triggers hungtask.
>
> [2434968.289510] INFO: task main:2354726 blocked for more than 120 seconds.
> [2434968.289516]       Tainted: G            E     5.10.0-0021.aarch64 #1
> [2434968.289517] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [2434968.289519] task:main            state:D stack:    0 pid:2354726 ppid:2350673 flags:0x00000a01
> [2434968.289523] Call trace:
> [2434968.289531]  __switch_to+0x7c/0xbc
> [2434968.289540]  __schedule+0x3b4/0x8a0
> [2434968.289542]  schedule+0x50/0xe0
> [2434968.289545]  rwsem_down_write_slowpath+0x3cc/0x6cc
> [2434968.289547]  down_write+0x60/0x260
> [2434968.289551]  __anon_vma_prepare+0x6c/0x210
> [2434968.289555]  do_anonymous_page+0x258/0x660
> [2434968.289557]  handle_pte_fault+0x188/0x214
> [2434968.289559]  __handle_mm_fault+0x1b0/0x380
> [2434968.289561]  handle_mm_fault+0xf4/0x284
> [2434968.289563]  do_page_fault+0x19c/0x494
> [2434968.289565]  do_translation_fault+0xcc/0xf8
> [2434968.289569]  do_mem_abort+0x48/0xac
> [2434968.289570]  el0_da+0x44/0x80
> [2434968.289572]  el0_sync_handler+0x88/0xb4
> [2434968.289573]  el0_sync+0x160/0x180
>
> After analyzing the vmcore, we found the anon_vma->root->rwsem.count is -1,
> and there is another anon_vma whose anon_vma->root->rwsem.count is 1, and
> the anon_vma->root->rwsem.owner shows the lock is held, but the stack of
> the task shows the task doesn't hold the anon_vma lock.

Can we have these details in the commit message on respin please? Thanks :)

>
> >

--
Cheers, Lorenzo

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Jinjiang Tu]

在 2026/9/7 20:47, Lorenzo Stoakes (ARM) 写道:
> On Mon, Sep 07, 2026 at 10:21:34AM +0800, Jinjiang Tu wrote:
>> 在 2026/9/6 7:21, Andrew Morton 写道:
>>> On Sat, 5 Sep 2026 14:18:19 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
>>>
>>>> On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
>>>> anon_vma->root is stale due to missing memory barrier, leading to
>>>> lock and unlock two different anon_vma->root, thus leading to a anon_vma
>>>> will never be unlocked, and another anon_vma couldn't be locked anymore.
>>>>
>>>> The race is as follows:
>>>>
>>>> ...
>>>>
>>>> Without this fix, our production environment could reproduce this issue
>>>> about 2-5 times each month.
>>> That's important info.  Can you tell us more?  How was this observed by
>>> operations people?  A copy-n-paste of the kernel messages would be helpful.
>>>
>>> This will help downstream people to decide whether this patch fixes a
>>> thing they're seeing happen.
>> We can see a task that tries to grab anon_vma lock triggers hungtask.
>>
>> [2434968.289510] INFO: task main:2354726 blocked for more than 120 seconds.
>> [2434968.289516]       Tainted: G            E     5.10.0-0021.aarch64 #1
>> [2434968.289517] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [2434968.289519] task:main            state:D stack:    0 pid:2354726 ppid:2350673 flags:0x00000a01
>> [2434968.289523] Call trace:
>> [2434968.289531]  __switch_to+0x7c/0xbc
>> [2434968.289540]  __schedule+0x3b4/0x8a0
>> [2434968.289542]  schedule+0x50/0xe0
>> [2434968.289545]  rwsem_down_write_slowpath+0x3cc/0x6cc
>> [2434968.289547]  down_write+0x60/0x260
>> [2434968.289551]  __anon_vma_prepare+0x6c/0x210
>> [2434968.289555]  do_anonymous_page+0x258/0x660
>> [2434968.289557]  handle_pte_fault+0x188/0x214
>> [2434968.289559]  __handle_mm_fault+0x1b0/0x380
>> [2434968.289561]  handle_mm_fault+0xf4/0x284
>> [2434968.289563]  do_page_fault+0x19c/0x494
>> [2434968.289565]  do_translation_fault+0xcc/0xf8
>> [2434968.289569]  do_mem_abort+0x48/0xac
>> [2434968.289570]  el0_da+0x44/0x80
>> [2434968.289572]  el0_sync_handler+0x88/0xb4
>> [2434968.289573]  el0_sync+0x160/0x180
>>
>> After analyzing the vmcore, we found the anon_vma->root->rwsem.count is -1,
>> and there is another anon_vma whose anon_vma->root->rwsem.count is 1, and
>> the anon_vma->root->rwsem.owner shows the lock is held, but the stack of
>> the task shows the task doesn't hold the anon_vma lock.
> Can we have these details in the commit message on respin please? Thanks :)

Will update it in v2. Thanks.

>
> --
> Cheers, Lorenzo

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Lorenzo Stoakes (ARM)]

On Tue, Sep 08, 2026 at 10:08:01AM +0800, Jinjiang Tu wrote:
> 在 2026/9/7 20:47, Lorenzo Stoakes (ARM) 写道:
> > > After analyzing the vmcore, we found the anon_vma->root->rwsem.count is -1,
> > > and there is another anon_vma whose anon_vma->root->rwsem.count is 1, and
> > > the anon_vma->root->rwsem.owner shows the lock is held, but the stack of
> > > the task shows the task doesn't hold the anon_vma lock.
> > Can we have these details in the commit message on respin please? Thanks :)
>
> Will update it in v2. Thanks.

Thanks! :)

--
Cheers, Lorenzo

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Lance Yang]

On Sat, Sep 05, 2026 at 02:18:19PM +0800, Jinjiang Tu wrote:
>On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
>anon_vma->root is stale due to missing memory barrier, leading to
>lock and unlock two different anon_vma->root, thus leading to a anon_vma
>will never be unlocked, and another anon_vma couldn't be locked anymore.
>
>The race is as follows:
>
>    THREAD A                             THREAD B
>__anon_vma_prepare                __anon_vma_prepare
> anon_vma = anon_vma_alloc();
> // writes may out of order here
> vma->anon_vma = anon_vma;
>                                   anon_vma = find_mergeable_anon_vma(vma);
>                                   anon_vma_lock_write(anon_vma);
>                                     // may still see the old root
>                                     down_write(&anon_vma->root->rwsem);
>                                   anon_vma_unlock_write(anon_vma);
>                                     // see the new root, never unlock old
>                                     up_write(&anon_vma->root->rwsem);
>
>thread A triggers page fault and calls __anon_vma_prepare() to prepare
>anon_vma for the faulting vma. __anon_vma_prepare() allocates and
>initializes a new anon_vma, and then publishes it to the vma with a plain
>store. anon_vma_prepare() only requires the mmap_lock to be held for
>reading, so two threads can fault on adjacent VMAs at the same time. While
>thread A publishes a new anon_vma, thread B could finds the anon_vma via
>find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.
>
>However, due to missing barrier, thread B can observe the published pointer
>but a stale anon_vma->root because the stores from anon_vma_alloc() aren't
>yet visible. What's the value of the stale anon_vma->root? __put_anon_vma()
>doesn't clear anon_vma->root, so the root of the new allocated anon_vma
>may point to a valid anon_vma.
>
>As a result, thread B can call anon_vma_lock_write() with the old root,
>and call anon_vma_unlock_write() with the new root, leading to a anon_vma
>will never be unlocked, and another anon_vma couldn't be locked anymore
>(it's count is dropped from 0 to -1 due to wrong unlock).
>
>To fix it, change the plain store `vma->anon_vma = anon_vma` to store
>release, so that the fields of anon_vma are visible before anon_vma is
>published to vma->anon_vma.
>
>We don't need a read barrier at read side for thread B. The load of
>anon_vma and anon_vma->root have address-dependency. According to
>Documentation/memory-barriers.txt and some investigations, only Alpha
>needs address-dependency barriers and it has been handled by READ_ONCE().
>
>This issue needs two adjacent VMAs aren't merged but are compatible for
>anon_vma. We reproduced this issue in v5.10 with KSM enabled. The kernel
>doesn't merge commit cf7e7a3503df ("mm: prevent KSM from breaking VMA
>merging for new VMAs"), so there are many adjacent VMAs that aren't merged
>but are compatible for anon_vma.
>
>Without this fix, our production environment could reproduce this issue
>about 2-5 times each month. After adding a smp_mb() before
>anon_vma_lock_write(anon_vma) in __anon_vma_prepare(), which is different
>to this patch, this issue hasn't be reproduced for one month.

Good catch, thanks!

>Cc: stable@vger.kernel.org
>Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")

Shouldn't the tag point to the following ?

Fixes: 012f18004da3 ("mm: always lock the root (oldest) anon_vma")

5c341ee1dfc8 introduced anon_vma->root, but 012f18004da3 made the lock
helpers dereference it. Before that, anon_vma_lock() used anon_vma->lock
directly, so a stale root could not cause this lock/unlock mismatch.

>Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>---

Hmm ... no luck reproducing this locally ... Still, AFAICT the race is
real.

For the read side, no extra barrier needed because the anon_vma->root
dereference is address-dependent on the READ_ONCE() load, IIUC :)

> mm/rmap.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/mm/rmap.c b/mm/rmap.c
>index d1819fd69938..a868e835eadb 100644
>--- a/mm/rmap.c
>+++ b/mm/rmap.c
>@@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
> 	/* page_table_lock to protect against threads */
> 	spin_lock(&mm->page_table_lock);
> 	if (likely(!vma->anon_vma)) {
>-		vma->anon_vma = anon_vma;
>+		/*
>+		 * The fields of anon_vma must be visible before anon_vma
>+		 * is published to vma->anon_vma.
>+		 */

Maybe be more specific:

/*
 * Publish anon_vma only after ->root is visible, otherwise a
 * concurrent fault may dereference a stale ->root when taking
 * the rwsem.
 */

Otherwise, LGTM.

Reviewed-by: Lance Yang <lance.yang@linux.dev>

Cheers, Lance

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Jinjiang Tu]

在 2026/9/7 20:43, Lance Yang 写道:
> On Sat, Sep 05, 2026 at 02:18:19PM +0800, Jinjiang Tu wrote:
>> On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
>> anon_vma->root is stale due to missing memory barrier, leading to
>> lock and unlock two different anon_vma->root, thus leading to a anon_vma
>> will never be unlocked, and another anon_vma couldn't be locked anymore.
>>
>> The race is as follows:
>>
>>     THREAD A                             THREAD B
>> __anon_vma_prepare                __anon_vma_prepare
>> anon_vma = anon_vma_alloc();
>> // writes may out of order here
>> vma->anon_vma = anon_vma;
>>                                    anon_vma = find_mergeable_anon_vma(vma);
>>                                    anon_vma_lock_write(anon_vma);
>>                                      // may still see the old root
>>                                      down_write(&anon_vma->root->rwsem);
>>                                    anon_vma_unlock_write(anon_vma);
>>                                      // see the new root, never unlock old
>>                                      up_write(&anon_vma->root->rwsem);
>>
>> thread A triggers page fault and calls __anon_vma_prepare() to prepare
>> anon_vma for the faulting vma. __anon_vma_prepare() allocates and
>> initializes a new anon_vma, and then publishes it to the vma with a plain
>> store. anon_vma_prepare() only requires the mmap_lock to be held for
>> reading, so two threads can fault on adjacent VMAs at the same time. While
>> thread A publishes a new anon_vma, thread B could finds the anon_vma via
>> find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.
>>
>> However, due to missing barrier, thread B can observe the published pointer
>> but a stale anon_vma->root because the stores from anon_vma_alloc() aren't
>> yet visible. What's the value of the stale anon_vma->root? __put_anon_vma()
>> doesn't clear anon_vma->root, so the root of the new allocated anon_vma
>> may point to a valid anon_vma.
>>
>> As a result, thread B can call anon_vma_lock_write() with the old root,
>> and call anon_vma_unlock_write() with the new root, leading to a anon_vma
>> will never be unlocked, and another anon_vma couldn't be locked anymore
>> (it's count is dropped from 0 to -1 due to wrong unlock).
>>
>> To fix it, change the plain store `vma->anon_vma = anon_vma` to store
>> release, so that the fields of anon_vma are visible before anon_vma is
>> published to vma->anon_vma.
>>
>> We don't need a read barrier at read side for thread B. The load of
>> anon_vma and anon_vma->root have address-dependency. According to
>> Documentation/memory-barriers.txt and some investigations, only Alpha
>> needs address-dependency barriers and it has been handled by READ_ONCE().
>>
>> This issue needs two adjacent VMAs aren't merged but are compatible for
>> anon_vma. We reproduced this issue in v5.10 with KSM enabled. The kernel
>> doesn't merge commit cf7e7a3503df ("mm: prevent KSM from breaking VMA
>> merging for new VMAs"), so there are many adjacent VMAs that aren't merged
>> but are compatible for anon_vma.
>>
>> Without this fix, our production environment could reproduce this issue
>> about 2-5 times each month. After adding a smp_mb() before
>> anon_vma_lock_write(anon_vma) in __anon_vma_prepare(), which is different
>> to this patch, this issue hasn't be reproduced for one month.
> Good catch, thanks!
>
>> Cc: stable@vger.kernel.org
>> Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")
> Shouldn't the tag point to the following ?
>
> Fixes: 012f18004da3 ("mm: always lock the root (oldest) anon_vma")
>
> 5c341ee1dfc8 introduced anon_vma->root, but 012f18004da3 made the lock
> helpers dereference it. Before that, anon_vma_lock() used anon_vma->lock
> directly, so a stale root could not cause this lock/unlock mismatch.

Indeed, will update it in v2.

>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>> ---
> Hmm ... no luck reproducing this locally ... Still, AFAICT the race is
> real.
>
> For the read side, no extra barrier needed because the anon_vma->root
> dereference is address-dependent on the READ_ONCE() load, IIUC :)
>
>> mm/rmap.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index d1819fd69938..a868e835eadb 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
>> 	/* page_table_lock to protect against threads */
>> 	spin_lock(&mm->page_table_lock);
>> 	if (likely(!vma->anon_vma)) {
>> -		vma->anon_vma = anon_vma;
>> +		/*
>> +		 * The fields of anon_vma must be visible before anon_vma
>> +		 * is published to vma->anon_vma.
>> +		 */
> Maybe be more specific:
>
> /*
>   * Publish anon_vma only after ->root is visible, otherwise a
>   * concurrent fault may dereference a stale ->root when taking
>   * the rwsem.
>   */

Although other stale fields of anon_vma might not introduce issues,
the stale values are fragile anyway, so I teed not to emphasize ->root here.

> Otherwise, LGTM.
>
> Reviewed-by: Lance Yang <lance.yang@linux.dev>

Thanks for review.

>
> Cheers, Lance

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Lorenzo Stoakes (ARM)]

Sorry, I looked at this on the weekend, then thought 'I should really not read
kernel mail on the weekend' and abandoned my reply. :)

On Sat, Sep 05, 2026 at 02:18:19PM +0800, Jinjiang Tu wrote:
> On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
> anon_vma->root is stale due to missing memory barrier, leading to
> lock and unlock two different anon_vma->root, thus leading to a anon_vma
> will never be unlocked, and another anon_vma couldn't be locked anymore.
>
> The race is as follows:
>
>     THREAD A                             THREAD B
> __anon_vma_prepare                __anon_vma_prepare
>  anon_vma = anon_vma_alloc();
>  // writes may out of order here

-> may be.

>  vma->anon_vma = anon_vma;
>                                    anon_vma = find_mergeable_anon_vma(vma);
>                                    anon_vma_lock_write(anon_vma);
>                                      // may still see the old root
>                                      down_write(&anon_vma->root->rwsem);
>                                    anon_vma_unlock_write(anon_vma);
>                                      // see the new root, never unlock old
>                                      up_write(&anon_vma->root->rwsem);

The left column is missing a lot of stuff I feel.

I think you need to make it clearer that what's happening is that you have:

	|-----------||-----------|
	|   VMA A   ||   VMA B   |
	|-----------||-----------|

Where VMA A and VMA B are merge compatible _except_ for a property that can
be modified by mprotect() (i.e. fulfills criteria of
anon_vma_compatible()).

There are racing faults on each.

Now VMA A has already tried calling find_mergeable_anon_vma(), which
checked VMA B for merge compatbility and found it was not, yet, faulted in.

Since:

static struct anon_vma *reusable_anon_vma(struct vm_area_struct *old,
					  struct vm_area_struct *a,
					  struct vm_area_struct *b)
{
	if (anon_vma_compatible(a, b)) {
		struct anon_vma *anon_vma = READ_ONCE(old->anon_vma);

		if (anon_vma && list_is_singular(&old->anon_vma_chain))
			return anon_vma; <---- Other VMA MUST have anon_vma.
	}
	return NULL;
}

So, __anon_vma_prepare() for VMA A goes ahead and allocates the anon VMA:

int __anon_vma_prepare(struct vm_area_struct *vma)
{
	...

	anon_vma = find_mergeable_anon_vma(vma);
	allocated = NULL;
	if (!anon_vma) {
		anon_vma = anon_vma_alloc();
		...
	}
	...
}

And sets its fields in anon_vma_alloc:

static inline struct anon_vma *anon_vma_alloc(void)
{
	struct anon_vma *anon_vma;

	anon_vma = kmem_cache_alloc(anon_vma_cachep, GFP_KERNEL);
	if (anon_vma) {
		...
		anon_vma->root = anon_vma;
	}

	return anon_vma;
}

Then it assigns vma->anon_vma and observe the classic gotcha with
acquire/release semantics:

int __anon_vma_prepare(struct vm_area_struct *vma)
{
	...

	<inlined>
	anon_vma = kmem_cache_alloc(anon_vma_cachep, GFP_KERNEL);
	if (anon_vma) {
		...
		anon_vma->root = anon_vma; ----------| Nothing stops this
	}                                            | being reordered
                                                     | to crit sect
                                                     |
	anon_vma_lock_write(anon_vma); --------------|------------
	spin_lock(&mm->page_table_lock);  -----------|------------ Acquire x2
                                                     |           |
	if (likely(!vma->anon_vma)) {                |           v
		vma->anon_vma = anon_vma;            |           ^
		...                                  v           |
	}                                                        |
	spin_unlock(&mm->page_table_lock); -----------------------
	anon_vma_unlock_write(anon_vma); ------------------------- Release x2

	...
}

And so you can end up in the verse situation of a weakly ordered arch
doing:

	vma->anon_vma = anon_vma;
	anon_vma->root = anon_vma;

And this the problem observed is that Thread/VMA B finds this VMA (it now
has vma->anon_vma assigned! So reusable_anon_vma() returns fine) and then:

int __anon_vma_prepare(struct vm_area_struct *vma)
{
	...

	anon_vma = find_mergeable_anon_vma(vma); <--- finds A!

	...
	anon_vma_lock_write(anon_vma); <-- goes to lock vma->anon_vma->root->rwsem
					   but... it's not assigned yet.
}

And guess what:

void __init anon_vma_init(void)
{
	anon_vma_cachep = kmem_cache_create("anon_vma", sizeof(struct anon_vma),
			0, SLAB_TYPESAFE_BY_RCU|SLAB_PANIC|SLAB_ACCOUNT,
			anon_vma_ctor); ^----------------  It's
	anon_vma_chain_cachep = KMEM_CACHE(anon_vma_chain,    our friend...
			SLAB_PANIC|SLAB_ACCOUNT);          SLAB_TYPESAFE_BY_RCU!
}							   so _very_ easy to UAF.
							   Not so much 'stale' just
							   a UAF that might still work.
							   Or the root might still exist...

How hideous all round.

>
> thread A triggers page fault and calls __anon_vma_prepare() to prepare
> anon_vma for the faulting vma. __anon_vma_prepare() allocates and
> initializes a new anon_vma, and then publishes it to the vma with a plain
> store. anon_vma_prepare() only requires the mmap_lock to be held for
> reading, so two threads can fault on adjacent VMAs at the same time. While
> thread A publishes a new anon_vma, thread B could finds the anon_vma via
> find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.
>
> However, due to missing barrier, thread B can observe the published pointer
> but a stale anon_vma->root because the stores from anon_vma_alloc() aren't

Less so stale, more UAF but you might get away with it...

> yet visible. What's the value of the stale anon_vma->root? __put_anon_vma()
> doesn't clear anon_vma->root, so the root of the new allocated anon_vma
> may point to a valid anon_vma.
>
> As a result, thread B can call anon_vma_lock_write() with the old root,
> and call anon_vma_unlock_write() with the new root, leading to a anon_vma
> will never be unlocked, and another anon_vma couldn't be locked anymore
> (it's count is dropped from 0 to -1 due to wrong unlock).
>
> To fix it, change the plain store `vma->anon_vma = anon_vma` to store
> release, so that the fields of anon_vma are visible before anon_vma is
> published to vma->anon_vma.
>
> We don't need a read barrier at read side for thread B. The load of

Well you _have_ to have a barrier of some kind for this to work...

> anon_vma and anon_vma->root have address-dependency. According to
> Documentation/memory-barriers.txt and some investigations, only Alpha
> needs address-dependency barriers and it has been handled by READ_ONCE().

...yup, because you have an address dependency :)

That works because, revising the memory barrier docs, the address
dependency works on the LOAD side only, because it has to deref to knoww
hat it's looking at whereas on the store side things can just live in the
store buffer.

>
> This issue needs two adjacent VMAs aren't merged but are compatible for
> anon_vma. We reproduced this issue in v5.10 with KSM enabled. The kernel
> doesn't merge commit cf7e7a3503df ("mm: prevent KSM from breaking VMA
> merging for new VMAs"), so there are many adjacent VMAs that aren't merged
> but are compatible for anon_vma.

How I hate the 'mergeable but not merged' terminology around this
particular part of anon_vma.

Well in any case I am replacing it all (eventually :) but in the meantime
it's irksome :)

>
> Without this fix, our production environment could reproduce this issue
> about 2-5 times each month. After adding a smp_mb() before
> anon_vma_lock_write(anon_vma) in __anon_vma_prepare(), which is different
> to this patch, this issue hasn't be reproduced for one month.

I don't see how the smp_mb() would make any difference there, I wonder if
you just reduced the race window?

>
> Cc: stable@vger.kernel.org
> Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")

I do wonder if something more recent made this at least more possible.

A decade and a half without it being caught before seems... unlikely :)

I wonder if the VMA locks made this more possible by (significantly)
increasing the ability for racing faults to occur (no mmap read lock
required).

Either that or something increased the race window or this was somehow
accidentally mitigated somehow.

> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>

Very good spot thanks!

I feel the commit message should be reworked a bit to really clarify the
problem, feel free to steal my
explain-to-myself-because-memory-barriers-break-my-brain stuff above :)
also add the stack trace from the subthread.

I also want some changes in comments etc. but broadly I think your solution
is correct.

See attached patch for what I suggest for fixing that all up.

With everything addressed feel free to add:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  mm/rmap.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index d1819fd69938..a868e835eadb 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
>  	/* page_table_lock to protect against threads */
>  	spin_lock(&mm->page_table_lock);
>  	if (likely(!vma->anon_vma)) {
> -		vma->anon_vma = anon_vma;
> +		/*
> +		 * The fields of anon_vma must be visible before anon_vma
> +		 * is published to vma->anon_vma.
> +		 */

_Everything_ like this needs a 'paired with'.

In this case, paired with the READ_ONCE() in reusable_anon_vma() (and a
similar comment there please).

Also over there the comment:

 * NOTE! This runs with mmap_lock held for reading, so it is possible that
 * the anon_vma of 'old' is concurrently in the process of being set up
 * by another page fault trying to merge _that_. But that's ok: if it
 * is being set up, that automatically means that it will be a singleton
 * acceptable for merging, so we can do all of this optimistically. But
 * we do that READ_ONCE() to make sure that we never re-load the pointe

Will need updating.

Actually, see below, I provide a patch showing what edits I think make
sense here.

BTW I actually wondered about doing this with a lock instead.

Like:

  scoped_guard(spinlock, &mm->page_table_lock)
      anon_vma = find_mergeable_anon_vma(vma);

I think that should work across the board as you get the right
acquire/release semantics in both cases (you should then delete that
comment above, remove the READ_ONCE() in reusable_anon_vma() etc.)

BUT.

You are then in a weird situation where it's kinda lockless but we get away
with it because nobody else who touches anon_vma touches fields that might
exhibit this kind of behaviour.

So the barrier is probably still best.

Anyway, adjust the patch as below on respin and update the commit message
as requested above and think this is the right way to go.


> +		smp_store_release(&vma->anon_vma, anon_vma);
>  		anon_vma_chain_assign(vma, avc, anon_vma);
>  		anon_rmap_tree_insert(avc, anon_vma);
>  		anon_vma->num_active_vmas++;
> --
> 2.43.0
>

Cheers, Lorenzo

----8<----
diff --git a/mm/rmap.c b/mm/rmap.c
index d1819fd69938..f3b21aaa34ee 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
 	/* page_table_lock to protect against threads */
 	spin_lock(&mm->page_table_lock);
 	if (likely(!vma->anon_vma)) {
-		vma->anon_vma = anon_vma;
+		/*
+		 * Make anon_vma fields visible before anon_vma is published.
+		 * Paired with an address dependency in reusable_anon_vma().
+		 */
+		smp_store_release(&vma->anon_vma, anon_vma);
 		anon_vma_chain_assign(vma, avc, anon_vma);
 		anon_rmap_tree_insert(avc, anon_vma);
 		anon_vma->num_active_vmas++;
diff --git a/mm/vma.c b/mm/vma.c
index 35e7a64855fa..ec4101250d71 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2094,6 +2094,13 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
  * acceptable for merging, so we can do all of this optimistically. But
  * we do that READ_ONCE() to make sure that we never re-load the pointer.
  *
+ * The READ_ONCE() establishes an address dependency between anon_vma and
+ * any access to its fields, which pairs with the assignment to
+ * vma->anon_vma performed with release semantics in __anon_vma_prepare().
+ *
+ * This is especially important as anon_vma's are SLAB_TYPESAFE_BY_RCU so
+ * accessing an uninitialised anon_vma's fields may result in a UAF.
+ *
  * IOW: that the "list_is_singular()" test on the anon_vma_chain only
  * matters for the 'stable anon_vma' case (ie the thing we want to avoid
  * is to return an anon_vma that is "complex" due to having gone through
@@ -2108,6 +2115,7 @@ static struct anon_vma *reusable_anon_vma(struct vm_area_struct *old,
 					  struct vm_area_struct *b)
 {
 	if (anon_vma_compatible(a, b)) {
+		/* Paired with a memory barrier in __anon_vma_prepare(). */
 		struct anon_vma *anon_vma = READ_ONCE(old->anon_vma);

 		if (anon_vma && list_is_singular(&old->anon_vma_chain))

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Jinjiang Tu]

在 2026/9/7 21:52, Lorenzo Stoakes (ARM) 写道:
> Sorry, I looked at this on the weekend, then thought 'I should really not read
> kernel mail on the weekend' and abandoned my reply. :)
>
> On Sat, Sep 05, 2026 at 02:18:19PM +0800, Jinjiang Tu wrote:
>> On arm64 server, we found  __anon_vma_prepare() reuses anon_vma and
>> anon_vma->root is stale due to missing memory barrier, leading to
>> lock and unlock two different anon_vma->root, thus leading to a anon_vma
>> will never be unlocked, and another anon_vma couldn't be locked anymore.
>>
>> The race is as follows:
>>
>>      THREAD A                             THREAD B
>> __anon_vma_prepare                __anon_vma_prepare
>>   anon_vma = anon_vma_alloc();
>>   // writes may out of order here
> -> may be.
>
>>   vma->anon_vma = anon_vma;
>>                                     anon_vma = find_mergeable_anon_vma(vma);
>>                                     anon_vma_lock_write(anon_vma);
>>                                       // may still see the old root
>>                                       down_write(&anon_vma->root->rwsem);
>>                                     anon_vma_unlock_write(anon_vma);
>>                                       // see the new root, never unlock old
>>                                       up_write(&anon_vma->root->rwsem);
> The left column is missing a lot of stuff I feel.
>
> I think you need to make it clearer that what's happening is that you have:
>
> 	|-----------||-----------|
> 	|   VMA A   ||   VMA B   |
> 	|-----------||-----------|
>
> Where VMA A and VMA B are merge compatible _except_ for a property that can
> be modified by mprotect() (i.e. fulfills criteria of
> anon_vma_compatible()).
>
> There are racing faults on each.
>
> Now VMA A has already tried calling find_mergeable_anon_vma(), which
> checked VMA B for merge compatbility and found it was not, yet, faulted in.
>
> Since:
>
> static struct anon_vma *reusable_anon_vma(struct vm_area_struct *old,
> 					  struct vm_area_struct *a,
> 					  struct vm_area_struct *b)
> {
> 	if (anon_vma_compatible(a, b)) {
> 		struct anon_vma *anon_vma = READ_ONCE(old->anon_vma);
>
> 		if (anon_vma && list_is_singular(&old->anon_vma_chain))
> 			return anon_vma; <---- Other VMA MUST have anon_vma.
> 	}
> 	return NULL;
> }
>
> So, __anon_vma_prepare() for VMA A goes ahead and allocates the anon VMA:
>
> int __anon_vma_prepare(struct vm_area_struct *vma)
> {
> 	...
>
> 	anon_vma = find_mergeable_anon_vma(vma);
> 	allocated = NULL;
> 	if (!anon_vma) {
> 		anon_vma = anon_vma_alloc();
> 		...
> 	}
> 	...
> }
>
> And sets its fields in anon_vma_alloc:
>
> static inline struct anon_vma *anon_vma_alloc(void)
> {
> 	struct anon_vma *anon_vma;
>
> 	anon_vma = kmem_cache_alloc(anon_vma_cachep, GFP_KERNEL);
> 	if (anon_vma) {
> 		...
> 		anon_vma->root = anon_vma;
> 	}
>
> 	return anon_vma;
> }
>
> Then it assigns vma->anon_vma and observe the classic gotcha with
> acquire/release semantics:
>
> int __anon_vma_prepare(struct vm_area_struct *vma)
> {
> 	...
>
> 	<inlined>
> 	anon_vma = kmem_cache_alloc(anon_vma_cachep, GFP_KERNEL);
> 	if (anon_vma) {
> 		...
> 		anon_vma->root = anon_vma; ----------| Nothing stops this
> 	}                                            | being reordered
>                                                       | to crit sect
>                                                       |
> 	anon_vma_lock_write(anon_vma); --------------|------------
> 	spin_lock(&mm->page_table_lock);  -----------|------------ Acquire x2
>                                                       |           |
> 	if (likely(!vma->anon_vma)) {                |           v
> 		vma->anon_vma = anon_vma;            |           ^
> 		...                                  v           |
> 	}                                                        |
> 	spin_unlock(&mm->page_table_lock); -----------------------
> 	anon_vma_unlock_write(anon_vma); ------------------------- Release x2
>
> 	...
> }
>
> And so you can end up in the verse situation of a weakly ordered arch
> doing:
>
> 	vma->anon_vma = anon_vma;
> 	anon_vma->root = anon_vma;
>
> And this the problem observed is that Thread/VMA B finds this VMA (it now
> has vma->anon_vma assigned! So reusable_anon_vma() returns fine) and then:
>
> int __anon_vma_prepare(struct vm_area_struct *vma)
> {
> 	...
>
> 	anon_vma = find_mergeable_anon_vma(vma); <--- finds A!
>
> 	...
> 	anon_vma_lock_write(anon_vma); <-- goes to lock vma->anon_vma->root->rwsem
> 					   but... it's not assigned yet.
> }
>
> And guess what:
>
> void __init anon_vma_init(void)
> {
> 	anon_vma_cachep = kmem_cache_create("anon_vma", sizeof(struct anon_vma),
> 			0, SLAB_TYPESAFE_BY_RCU|SLAB_PANIC|SLAB_ACCOUNT,
> 			anon_vma_ctor); ^----------------  It's
> 	anon_vma_chain_cachep = KMEM_CACHE(anon_vma_chain,    our friend...
> 			SLAB_PANIC|SLAB_ACCOUNT);          SLAB_TYPESAFE_BY_RCU!
> }							   so _very_ easy to UAF.
> 							   Not so much 'stale' just
> 							   a UAF that might still work.
> 							   Or the root might still exist...
>
> How hideous all round.
>
>> thread A triggers page fault and calls __anon_vma_prepare() to prepare
>> anon_vma for the faulting vma. __anon_vma_prepare() allocates and
>> initializes a new anon_vma, and then publishes it to the vma with a plain
>> store. anon_vma_prepare() only requires the mmap_lock to be held for
>> reading, so two threads can fault on adjacent VMAs at the same time. While
>> thread A publishes a new anon_vma, thread B could finds the anon_vma via
>> find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.
>>
>> However, due to missing barrier, thread B can observe the published pointer
>> but a stale anon_vma->root because the stores from anon_vma_alloc() aren't
> Less so stale, more UAF but you might get away with it...
>> yet visible. What's the value of the stale anon_vma->root? __put_anon_vma()
>> doesn't clear anon_vma->root, so the root of the new allocated anon_vma
>> may point to a valid anon_vma.
>>
>> As a result, thread B can call anon_vma_lock_write() with the old root,
>> and call anon_vma_unlock_write() with the new root, leading to a anon_vma
>> will never be unlocked, and another anon_vma couldn't be locked anymore
>> (it's count is dropped from 0 to -1 due to wrong unlock).
>>
>> To fix it, change the plain store `vma->anon_vma = anon_vma` to store
>> release, so that the fields of anon_vma are visible before anon_vma is
>> published to vma->anon_vma.
>>
>> We don't need a read barrier at read side for thread B. The load of
> Well you _have_ to have a barrier of some kind for this to work...
>
>> anon_vma and anon_vma->root have address-dependency. According to
>> Documentation/memory-barriers.txt and some investigations, only Alpha
>> needs address-dependency barriers and it has been handled by READ_ONCE().
> ...yup, because you have an address dependency :)
>
> That works because, revising the memory barrier docs, the address
> dependency works on the LOAD side only, because it has to deref to knoww
> hat it's looking at whereas on the store side things can just live in the
> store buffer.
>
>> This issue needs two adjacent VMAs aren't merged but are compatible for
>> anon_vma. We reproduced this issue in v5.10 with KSM enabled. The kernel
>> doesn't merge commit cf7e7a3503df ("mm: prevent KSM from breaking VMA
>> merging for new VMAs"), so there are many adjacent VMAs that aren't merged
>> but are compatible for anon_vma.
> How I hate the 'mergeable but not merged' terminology around this
> particular part of anon_vma.
>
> Well in any case I am replacing it all (eventually :) but in the meantime
> it's irksome :)
>
>> Without this fix, our production environment could reproduce this issue
>> about 2-5 times each month. After adding a smp_mb() before
>> anon_vma_lock_write(anon_vma) in __anon_vma_prepare(), which is different
>> to this patch, this issue hasn't be reproduced for one month.
> I don't see how the smp_mb() would make any difference there, I wonder if
> you just reduced the race window?

When troubleshooting this issue, we suspected it was a memory barrier problem,
so we added a full memory barrier like below.

diff --git a/mm/rmap.c b/mm/rmap.c
index d1819fd69938..11203f381beb 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -205,6 +205,8 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
                 allocated = anon_vma;
         }
  
+       smp_mb();
+
         anon_vma_lock_write(anon_vma);
         /* page_table_lock to protect against threads */
         spin_lock(&mm->page_table_lock);


smp_mb() ensures that all prior loads and stores are completed
before any subsequent loads and stores, has stricter semantics
than smp_store_release().

I used the strongest smp_mb() barrier to test in the production
environment to confirm whether the issue was related to memory
barriers, and to avoid falsely concluding that it wasn't a memory
barrier issue due to the incorrect use of a weaker barrier.

>> Cc: stable@vger.kernel.org
>> Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")
> I do wonder if something more recent made this at least more possible.
>
> A decade and a half without it being caught before seems... unlikely :)
>
> I wonder if the VMA locks made this more possible by (significantly)
> increasing the ability for racing faults to occur (no mmap read lock
> required).

I mentioned it in the commit message, maybe you missed it.

"We reproduced this issue in v5.10 with KSM enabled. The kernel
doesn't merge commit cf7e7a3503df ("mm: prevent KSM from breaking
VMA merging for new VMAs"), so there are many adjacent VMAs that
aren't merged but are compatible for anon_vma."

>
> Either that or something increased the race window or this was somehow
> accidentally mitigated somehow.
>
>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> Very good spot thanks!
>
> I feel the commit message should be reworked a bit to really clarify the
> problem, feel free to steal my
> explain-to-myself-because-memory-barriers-break-my-brain stuff above :)
> also add the stack trace from the subthread.
>
> I also want some changes in comments etc. but broadly I think your solution
> is correct.
>
> See attached patch for what I suggest for fixing that all up.
>
> With everything addressed feel free to add:
>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
>> ---
>>   mm/rmap.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index d1819fd69938..a868e835eadb 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
>>   	/* page_table_lock to protect against threads */
>>   	spin_lock(&mm->page_table_lock);
>>   	if (likely(!vma->anon_vma)) {
>> -		vma->anon_vma = anon_vma;
>> +		/*
>> +		 * The fields of anon_vma must be visible before anon_vma
>> +		 * is published to vma->anon_vma.
>> +		 */
> _Everything_ like this needs a 'paired with'.
>
> In this case, paired with the READ_ONCE() in reusable_anon_vma() (and a
> similar comment there please).
>
> Also over there the comment:
>
>   * NOTE! This runs with mmap_lock held for reading, so it is possible that
>   * the anon_vma of 'old' is concurrently in the process of being set up
>   * by another page fault trying to merge _that_. But that's ok: if it
>   * is being set up, that automatically means that it will be a singleton
>   * acceptable for merging, so we can do all of this optimistically. But
>   * we do that READ_ONCE() to make sure that we never re-load the pointe
>
> Will need updating.
>
> Actually, see below, I provide a patch showing what edits I think make
> sense here.
>
> BTW I actually wondered about doing this with a lock instead.
>
> Like:
>
>    scoped_guard(spinlock, &mm->page_table_lock)
>        anon_vma = find_mergeable_anon_vma(vma);
>
> I think that should work across the board as you get the right
> acquire/release semantics in both cases (you should then delete that
> comment above, remove the READ_ONCE() in reusable_anon_vma() etc.)
>
> BUT.
>
> You are then in a weird situation where it's kinda lockless but we get away
> with it because nobody else who touches anon_vma touches fields that might
> exhibit this kind of behaviour.
>
> So the barrier is probably still best.
>
> Anyway, adjust the patch as below on respin and update the commit message
> as requested above and think this is the right way to go.

Thanks for review. Will update the commit message and comments in v2.

>
>
>> +		smp_store_release(&vma->anon_vma, anon_vma);
>>   		anon_vma_chain_assign(vma, avc, anon_vma);
>>   		anon_rmap_tree_insert(avc, anon_vma);
>>   		anon_vma->num_active_vmas++;
>> --
>> 2.43.0
>>
> Cheers, Lorenzo
>
> ----8<----
> diff --git a/mm/rmap.c b/mm/rmap.c
> index d1819fd69938..f3b21aaa34ee 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -209,7 +209,11 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
>   	/* page_table_lock to protect against threads */
>   	spin_lock(&mm->page_table_lock);
>   	if (likely(!vma->anon_vma)) {
> -		vma->anon_vma = anon_vma;
> +		/*
> +		 * Make anon_vma fields visible before anon_vma is published.
> +		 * Paired with an address dependency in reusable_anon_vma().
> +		 */
> +		smp_store_release(&vma->anon_vma, anon_vma);
>   		anon_vma_chain_assign(vma, avc, anon_vma);
>   		anon_rmap_tree_insert(avc, anon_vma);
>   		anon_vma->num_active_vmas++;
> diff --git a/mm/vma.c b/mm/vma.c
> index 35e7a64855fa..ec4101250d71 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2094,6 +2094,13 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
>    * acceptable for merging, so we can do all of this optimistically. But
>    * we do that READ_ONCE() to make sure that we never re-load the pointer.
>    *
> + * The READ_ONCE() establishes an address dependency between anon_vma and
> + * any access to its fields, which pairs with the assignment to
> + * vma->anon_vma performed with release semantics in __anon_vma_prepare().
> + *
> + * This is especially important as anon_vma's are SLAB_TYPESAFE_BY_RCU so
> + * accessing an uninitialised anon_vma's fields may result in a UAF.
> + *
>    * IOW: that the "list_is_singular()" test on the anon_vma_chain only
>    * matters for the 'stable anon_vma' case (ie the thing we want to avoid
>    * is to return an anon_vma that is "complex" due to having gone through
> @@ -2108,6 +2115,7 @@ static struct anon_vma *reusable_anon_vma(struct vm_area_struct *old,
>   					  struct vm_area_struct *b)
>   {
>   	if (anon_vma_compatible(a, b)) {
> +		/* Paired with a memory barrier in __anon_vma_prepare(). */
>   		struct anon_vma *anon_vma = READ_ONCE(old->anon_vma);
>
>   		if (anon_vma && list_is_singular(&old->anon_vma_chain))

Re: [PATCH] mm/rmap: fix missing barrier between anon_vma init and vma->anon_vma publish

[Lorenzo Stoakes (ARM)]

On Tue, Sep 08, 2026 at 10:48:29AM +0800, Jinjiang Tu wrote:
>
> 在 2026/9/7 21:52, Lorenzo Stoakes (ARM) 写道:

> >
> > > Without this fix, our production environment could reproduce this issue
> > > about 2-5 times each month. After adding a smp_mb() before
> > > anon_vma_lock_write(anon_vma) in __anon_vma_prepare(), which is different
> > > to this patch, this issue hasn't be reproduced for one month.
> > I don't see how the smp_mb() would make any difference there, I wonder if
> > you just reduced the race window?
>
> When troubleshooting this issue, we suspected it was a memory barrier problem,
> so we added a full memory barrier like below.
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index d1819fd69938..11203f381beb 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -205,6 +205,8 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
>                 allocated = anon_vma;
>         }
> +       smp_mb();
> +
>         anon_vma_lock_write(anon_vma);
>         /* page_table_lock to protect against threads */
>         spin_lock(&mm->page_table_lock);
>
>
> smp_mb() ensures that all prior loads and stores are completed
> before any subsequent loads and stores, has stricter semantics
> than smp_store_release().
>
> I used the strongest smp_mb() barrier to test in the production
> environment to confirm whether the issue was related to memory
> barriers, and to avoid falsely concluding that it wasn't a memory
> barrier issue due to the incorrect use of a weaker barrier.

Ah OK I misunderstood this (memory barriers make this easy :) so this therefore
means you've confirmed the bug fix also, as the release version is definitely
correct (I analysed it through in my reply manually and ran it through a bunch
of AI checks also to be sure).

Nice then :)

>
> > > Cc: stable@vger.kernel.org
> > > Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")
> > I do wonder if something more recent made this at least more possible.
> >
> > A decade and a half without it being caught before seems... unlikely :)
> >
> > I wonder if the VMA locks made this more possible by (significantly)
> > increasing the ability for racing faults to occur (no mmap read lock
> > required).
>
> I mentioned it in the commit message, maybe you missed it.
>
> "We reproduced this issue in v5.10 with KSM enabled. The kernel
> doesn't merge commit cf7e7a3503df ("mm: prevent KSM from breaking
> VMA merging for new VMAs"), so there are many adjacent VMAs that
> aren't merged but are compatible for anon_vma."

Ahh ok interesting.

I do wonder if that is a better Fixes target then? But at the same time,
technically, I guess the old commit is the right one.

So yeah I think let's keep it as you've specified.

> Thanks for review. Will update the commit message and comments in v2.

Great thanks!

--
Cheers, Lorenzo