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

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

[Jinjiang Tu]

On arm64 server, we find that a task trying to grab the anon_vma lock
triggers hungtask.

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

After analyzing the vmcore, we found the anon_vma->root->rwsem.count is -1.
There is another anon_vma whose anon_vma->root->rwsem.count is 1, 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.

After adding more debugging info, we found __anon_vma_prepare() reuses
anon_vma and triggers the UAF of anon_vma->root due to missing memory
barrier, leading to locking and unlocking two different anon_vma->root,
thus leading to an anon_vma will never be unlocked, and another anon_vma
couldn't be locked anymore.

This race requires two adjacent VMAs that are not merged but are
anon_vma-compatible (e.g., they differ in VMA_ACCESS_FLAGS that can be
changed by mprotect()). Two threads fault on each VMA concurrently, both
calling __anon_vma_prepare() with only mmap_lock held for reading.

    THREAD A                             THREAD B
__anon_vma_prepare                __anon_vma_prepare
 find_mergeable_anon_vma() -> NULL
 anon_vma = anon_vma_alloc();
   anon_vma->root = anon_vma;
 // the two stores may be reordered
 vma->anon_vma = anon_vma;
                                   // finds A's 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 find the anon_vma via
find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.

The store to anon_vma->root in anon_vma_alloc() and the store to
vma->anon_vma can be reordered. The anon_vma_lock_write() and spin_lock()
only provide acquire semantics, which do not prevent prior stores from
being reordered after them. The release semantics of the corresponding
spin_unlock() and anon_vma_unlock_write() come too late, the store to
vma->anon_vma is already published before they take effect. As a result,
thread B can observe the following order:

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

The anon_vma slab is SLAB_TYPESAFE_BY_RCU, so a newly allocated anon_vma
may reuse memory from a previously freed one. The constructor
(anon_vma_ctor) does not reset anon_vma->root, and __put_anon_vma()
doesn't clear it either, so the old root value persists until
anon_vma_alloc() overwrites it. If that store isn't visible, thread B
reads a root that points to the old anon_vma and locks it.

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 an anon_vma
will never be unlocked, and another anon_vma couldn't be locked anymore
(its 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.

At read side, 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() in reusable_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 been reproduced for one month.

Cc: stable@vger.kernel.org
Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")
Reviewed-by: Lance Yang <lance.yang@linux.dev>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
Change since v1:
 * correct the fix tag (Lance Yang)
 * rework commit message and update comment (Lorenzo Stoakes)
 * collect Reviewed-by

 mm/rmap.c | 6 +++++-
 mm/vma.c  | 8 ++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

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))
-- 
2.43.0

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

[David Hildenbrand (Arm)]

On 9/8/26 14:29, Jinjiang Tu wrote:
> On arm64 server, we find that a task trying to grab the anon_vma lock
> triggers hungtask.
> 
> INFO: task main:2354726 blocked for more than 120 seconds.
>       Tainted: G            E     5.10.0-0021.aarch64 #1
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:main            state:D stack:    0 pid:2354726 ppid:2350673 flags:0x00000a01
> Call trace:
>  __switch_to+0x7c/0xbc
>  __schedule+0x3b4/0x8a0
>  schedule+0x50/0xe0
>  rwsem_down_write_slowpath+0x3cc/0x6cc
>  down_write+0x60/0x260
>  __anon_vma_prepare+0x6c/0x210
>  do_anonymous_page+0x258/0x660
>  handle_pte_fault+0x188/0x214
>  __handle_mm_fault+0x1b0/0x380
>  handle_mm_fault+0xf4/0x284
>  do_page_fault+0x19c/0x494
>  do_translation_fault+0xcc/0xf8
>  do_mem_abort+0x48/0xac
>  el0_da+0x44/0x80
>  el0_sync_handler+0x88/0xb4
>  el0_sync+0x160/0x180
> 
> After analyzing the vmcore, we found the anon_vma->root->rwsem.count is -1.
> There is another anon_vma whose anon_vma->root->rwsem.count is 1, 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.
> 
> After adding more debugging info, we found __anon_vma_prepare() reuses
> anon_vma and triggers the UAF of anon_vma->root due to missing memory
> barrier, leading to locking and unlocking two different anon_vma->root,
> thus leading to an anon_vma will never be unlocked, and another anon_vma
> couldn't be locked anymore.
> 
> This race requires two adjacent VMAs that are not merged but are
> anon_vma-compatible (e.g., they differ in VMA_ACCESS_FLAGS that can be
> changed by mprotect()). Two threads fault on each VMA concurrently, both
> calling __anon_vma_prepare() with only mmap_lock held for reading.
> 
>     THREAD A                             THREAD B
> __anon_vma_prepare                __anon_vma_prepare
>  find_mergeable_anon_vma() -> NULL
>  anon_vma = anon_vma_alloc();
>    anon_vma->root = anon_vma;
>  // the two stores may be reordered
>  vma->anon_vma = anon_vma;
>                                    // finds A's 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 find the anon_vma via
> find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.
> 
> The store to anon_vma->root in anon_vma_alloc() and the store to
> vma->anon_vma can be reordered. The anon_vma_lock_write() and spin_lock()
> only provide acquire semantics, which do not prevent prior stores from
> being reordered after them. The release semantics of the corresponding
> spin_unlock() and anon_vma_unlock_write() come too late, the store to
> vma->anon_vma is already published before they take effect. As a result,
> thread B can observe the following order:
> 
>     vma->anon_vma = anon_vma;
>     anon_vma->root = anon_vma;
> 
> The anon_vma slab is SLAB_TYPESAFE_BY_RCU, so a newly allocated anon_vma
> may reuse memory from a previously freed one. The constructor
> (anon_vma_ctor) does not reset anon_vma->root, and __put_anon_vma()
> doesn't clear it either, so the old root value persists until
> anon_vma_alloc() overwrites it. If that store isn't visible, thread B
> reads a root that points to the old anon_vma and locks it.
> 
> 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 an anon_vma
> will never be unlocked, and another anon_vma couldn't be locked anymore
> (its 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.
> 
> At read side, 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() in reusable_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 been reproduced for one month.
> 
> Cc: stable@vger.kernel.org
> Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")
> Reviewed-by: Lance Yang <lance.yang@linux.dev>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---
> Change since v1:
>  * correct the fix tag (Lance Yang)
>  * rework commit message and update comment (Lorenzo Stoakes)
>  * collect Reviewed-by
> 
>  mm/rmap.c | 6 +++++-
>  mm/vma.c  | 8 ++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> 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);


Makes perfect sense to me, although I am not that familiar with all the nasty
details of anon_vma merging (in contrast to Lorenzo ;) )

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

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

[Jann Horn]

On Tue, Sep 8, 2026 at 2:57 PM Jinjiang Tu <tujinjiang@huawei.com> wrote:
> 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);

Yes, this should go in. There are remaining theoretical issues (we
really should also be using READ_ONCE() in places like unuse_mm(), see
<https://lore.kernel.org/all/20230726214103.3261108-4-jannh@google.com/>),
but this is the important fix.

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

[Jinjiang Tu]

在 2026/9/9 23:25, Jann Horn 写道:
> On Tue, Sep 8, 2026 at 2:57 PM Jinjiang Tu <tujinjiang@huawei.com> wrote:
>> 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);
> Yes, this should go in. There are remaining theoretical issues (we
> really should also be using READ_ONCE() in places like unuse_mm(), see
> <https://lore.kernel.org/all/20230726214103.3261108-4-jannh@google.com/>),
> but this is the important fix.

Theoretically yes, but I'm curious whether these are actual problems in practice.

In hugepage_vma_revalidate() and unuse_mm(), the code only checks whether anon_vma
is non-NULL — it doesn't dereference anon_vma to access its fields.

In KSM, rmap_item->anon_vma is assigned from vma->anon_vma, and the rmap_item is
then inserted into a stable node. There is a very long time window before
rmap_walk_ksm() could ever observe it.

>

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

[Vlastimil Babka (SUSE)]

On 9/8/26 14:29, Jinjiang Tu wrote:
> On arm64 server, we find that a task trying to grab the anon_vma lock
> triggers hungtask.
> 
> INFO: task main:2354726 blocked for more than 120 seconds.
>       Tainted: G            E     5.10.0-0021.aarch64 #1
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:main            state:D stack:    0 pid:2354726 ppid:2350673 flags:0x00000a01
> Call trace:
>  __switch_to+0x7c/0xbc
>  __schedule+0x3b4/0x8a0
>  schedule+0x50/0xe0
>  rwsem_down_write_slowpath+0x3cc/0x6cc
>  down_write+0x60/0x260
>  __anon_vma_prepare+0x6c/0x210
>  do_anonymous_page+0x258/0x660
>  handle_pte_fault+0x188/0x214
>  __handle_mm_fault+0x1b0/0x380
>  handle_mm_fault+0xf4/0x284
>  do_page_fault+0x19c/0x494
>  do_translation_fault+0xcc/0xf8
>  do_mem_abort+0x48/0xac
>  el0_da+0x44/0x80
>  el0_sync_handler+0x88/0xb4
>  el0_sync+0x160/0x180
> 
> After analyzing the vmcore, we found the anon_vma->root->rwsem.count is -1.
> There is another anon_vma whose anon_vma->root->rwsem.count is 1, 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.
> 
> After adding more debugging info, we found __anon_vma_prepare() reuses
> anon_vma and triggers the UAF of anon_vma->root due to missing memory
> barrier, leading to locking and unlocking two different anon_vma->root,
> thus leading to an anon_vma will never be unlocked, and another anon_vma
> couldn't be locked anymore.
> 
> This race requires two adjacent VMAs that are not merged but are
> anon_vma-compatible (e.g., they differ in VMA_ACCESS_FLAGS that can be
> changed by mprotect()). Two threads fault on each VMA concurrently, both
> calling __anon_vma_prepare() with only mmap_lock held for reading.
> 
>     THREAD A                             THREAD B
> __anon_vma_prepare                __anon_vma_prepare
>  find_mergeable_anon_vma() -> NULL
>  anon_vma = anon_vma_alloc();
>    anon_vma->root = anon_vma;
>  // the two stores may be reordered
>  vma->anon_vma = anon_vma;
>                                    // finds A's 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 find the anon_vma via
> find_mergeable_anon_vma() and then locks anon_vma->root->rwsem.
> 
> The store to anon_vma->root in anon_vma_alloc() and the store to
> vma->anon_vma can be reordered. The anon_vma_lock_write() and spin_lock()
> only provide acquire semantics, which do not prevent prior stores from
> being reordered after them. The release semantics of the corresponding
> spin_unlock() and anon_vma_unlock_write() come too late, the store to
> vma->anon_vma is already published before they take effect. As a result,
> thread B can observe the following order:
> 
>     vma->anon_vma = anon_vma;
>     anon_vma->root = anon_vma;
> 
> The anon_vma slab is SLAB_TYPESAFE_BY_RCU, so a newly allocated anon_vma
> may reuse memory from a previously freed one. The constructor
> (anon_vma_ctor) does not reset anon_vma->root, and __put_anon_vma()
> doesn't clear it either, so the old root value persists until
> anon_vma_alloc() overwrites it. If that store isn't visible, thread B
> reads a root that points to the old anon_vma and locks it.
> 
> 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 an anon_vma
> will never be unlocked, and another anon_vma couldn't be locked anymore
> (its 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.
> 
> At read side, 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() in reusable_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 been reproduced for one month.
> 
> Cc: stable@vger.kernel.org
> Fixes: 5c341ee1dfc8 ("mm: track the root (oldest) anon_vma")
> Reviewed-by: Lance Yang <lance.yang@linux.dev>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>

Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>