* [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends
@ 2026-10-27 14:08 Lorenzo Stoakes (ARM)
2026-10-27 15:34 ` Suren Baghdasaryan
2026-10-27 22:10 ` Jose A. Perez de Azpillaga
0 siblings, 2 replies; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-27 14:08 UTC (permalink / raw)
To: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
Pedro Falcato, Suren Baghdasaryan, Shakeel Butt, SJ Park
Cc: linux-kernel, linux-fsdevel, linux-mm, ljs, stable,
David Hildenbrand (Arm)
Reading /proc/PID/maps under per-VMA locks can skip continuously mapped
address ranges during concurrent VMA splits or merges. This violates the
guarantee in Documentation/filesystems/proc.rst:
"If there is something at a given vaddr during the entirety of the
life of the smaps/maps walk, there will be some output for it."
lock_next_vma() reuses a tree iterator across calls under RCU. The iterator
remembers the range of its last entry. When its node is replaced,
mas_next_slot() walks the tree again at the last index of that range, then
advances past the entry containing that index. If VMA boundaries changed,
this can skip a range that has not been reported:
- A lookup finds [A, C), then mprotect() splits it into [A, B) and [B, C)
before vma_start_read() locks the first VMA. The reader reports [A, B),
but the iterator still covers [A, C). The next call starts from B yet
advances past C, omitting [B, C).
- The reader reports [A, B) and unlocks it. A concurrent mprotect() then
merges it with [B, C). On the next call, the iterator resumes at B - 1,
which now belongs to [A, C), and advances past C. Again, [B, C) is lost.
Checking the mmap_lock sequence only within the next call misses writers
that completed before that call began. Also, a NULL result is currently
accepted without validation, so a race can terminate the walk early.
Even a fresh lookup can skip a mapping. Suppose it finds a gap before
[B, C). A writer inserts an adjacent VMA at the end of that gap, then
merges it into [B, C), replacing the node. The lookup resumes at the old
gap's last index, now inside the merged VMA, and steps past it. The
original [B, C) remains mapped throughout. Repeating the forward lookup
is insufficient: another merge can make it skip the same mapping again,
returning the same later VMA or NULL.
The fresh-lookup race also affects /proc/PID/smaps, numa_maps and
smaps_rollup. Resetting the iterator after gathering each VMA's statistics
does not prevent a merge during the next lookup. Fixing lock_next_vma()
also prevents omitted entries and incomplete rollup statistics.
Tie VMA iterator state with the current mm and the write generation at the
point of last mm speculation in vma_rcu_iter, whose values we call 'rvmi'.
This allows us to neatly abstract the existing priv->iter logic alongside
query_vma_find_by_addr() which uses its own VMA iterator.
Then, provide helpers that act on the rvmi - vma_rcu_iter_[maybe_changed,
may_have_skipped]() to determine whether any VMAs may have
been altered since last mm speculation.
Additionally introduce vma_range_is_empty(), used by
vma_rcu_iter_may_have_skipped(), to determine conclusively whether VMAs
exist in a set range, needing to be traversed.
Original fix generated by an LLM and sent by Josef Bacik in private, then
amended by David Hildenbrand, and Lorenzo Stoakes added additional
abstraction.
Fixes: 5631da56c9a8 ("fs/proc/task_mmu: read proc/pid/maps under per-vma lock")
Fixes: ea3085dd7a4e ("fs/proc/task_mmu: read proc/pid/{smaps|numa_maps} under per-vma lock")
Fixes: 0e985559d72a ("proc/task_mmu: read proc/pid/smaps_rollup under per-vma lock")
Fixes: d9d1c2d81797 ("fs/proc/task_mmu: execute PROCMAP_QUERY ioctl under per-vma locks")
Cc: stable@vger.kernel.org
Co-developed-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
Note that I decided it's best to defer any VMA iterator stuff to a
follow-up - with the abstractions added it's clear what's going on and
easier for backporting.
---
v2:
* Took over the patch to make David's life easier :)
* Introduced vma_rcu_iter tying the VMA iterator, mm and sampled mm seqnum
together, and update lock_next_vma() + callers to use it.
* Moved the sequence out of proc_maps_locking_ctx and the "must initially
be odd" requirement out of the header into vma_rcu_iter_init().
* Added vma_rcu_iter_maybe_changed() and vma_rcu_iter_may_have_skipped()
to replace the open-coded speculation checks.
* Converted vma_range_is_empty() to use a VMA iterator to be consistent.
* Added Fixes: tag for d9d1c2d81797 (PROCMAP_QUERY), as per Jose.
v1:
https://lore.kernel.org/r/20261025-per_vma_maps-v1-1-c4eec45b9387@kernel.org
---
fs/proc/internal.h | 1 +
fs/proc/task_mmu.c | 13 +++++-----
include/linux/mmap_lock.h | 22 ++++++++++++++---
mm/mmap_lock.c | 61 ++++++++++++++++++++++++++++++++++-------------
4 files changed, 72 insertions(+), 25 deletions(-)
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 623bb43ede55..1d95ff577a06 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -393,6 +393,7 @@ struct proc_maps_private {
struct inode *inode;
struct task_struct *task;
struct vma_iterator iter;
+ struct vma_rcu_iter rvmi;
loff_t last_pos;
struct proc_maps_locking_ctx lock_ctx;
#ifdef CONFIG_NUMA
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 74ffa5c99b8e..22361607020d 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -165,7 +165,7 @@ static struct vm_area_struct *get_next_vma(struct proc_maps_private *priv,
return vma_next(&priv->iter);
unlock_ctx_vma(lock_ctx);
- vma = lock_next_vma(lock_ctx->mm, &priv->iter, last_pos);
+ vma = lock_next_vma(&priv->rvmi, last_pos);
if (!IS_ERR_OR_NULL(vma))
lock_ctx->locked_vma = vma;
@@ -270,7 +270,7 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
*/
if (last_addr && !addr_is_sentinel(last_addr))
*ppos = last_addr = priv->last_pos;
- vma_iter_init(&priv->iter, mm, (unsigned long)last_addr);
+ vma_rcu_iter_init(&priv->rvmi, mm, &priv->iter, (unsigned long)last_addr);
hold_task_mempolicy(priv);
/*
* If seq_file had to flush its collected data right after m_next() set
@@ -520,6 +520,7 @@ static struct vm_area_struct *query_vma_find_by_addr(struct proc_maps_locking_ct
struct mm_struct *mm = lock_ctx->mm;
struct vm_area_struct *vma;
struct vma_iterator vmi;
+ struct vma_rcu_iter rvmi;
if (lock_ctx->mmap_locked)
return find_vma(mm, addr);
@@ -527,8 +528,8 @@ static struct vm_area_struct *query_vma_find_by_addr(struct proc_maps_locking_ct
/* Unlock previously locked VMA and find the next one under RCU */
unlock_ctx_vma(lock_ctx);
rcu_read_lock();
- vma_iter_init(&vmi, mm, addr);
- vma = lock_next_vma(mm, &vmi, addr);
+ vma_rcu_iter_init(&rvmi, mm, &vmi, addr);
+ vma = lock_next_vma(&rvmi, addr);
rcu_read_unlock();
if (!vma)
@@ -1421,7 +1422,7 @@ static int show_smaps_rollup(struct seq_file *m, void *v)
rcu_read_lock();
reset_lock_ctx(lock_ctx);
- vma_iter_init(&priv->iter, mm, 0);
+ vma_rcu_iter_init(&priv->rvmi, mm, &priv->iter, 0);
vma = proc_get_vma(m, &pos);
if (unlikely(!vma))
goto empty_set;
@@ -1470,7 +1471,7 @@ static int show_smaps_rollup(struct seq_file *m, void *v)
reset_lock_ctx(lock_ctx);
/* Resume from the last position. */
pos = last_vma_end;
- vma_iter_init(&priv->iter, mm, pos);
+ vma_rcu_iter_init(&priv->rvmi, mm, &priv->iter, pos);
}
vma = proc_get_vma(m, &pos);
}
diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index 11b3ffef3966..9ccbbcbd7aca 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h
@@ -17,6 +17,13 @@ extern int rcuwait_wake_up(struct rcuwait *w);
#define MMAP_LOCK_INITIALIZER(name) \
.mmap_lock = __RWSEM_INITIALIZER((name).mmap_lock),
+/* Tie a VMA iterator to mm write lock state at iterator initialisation. */
+struct vma_rcu_iter {
+ struct mm_struct *mm;
+ struct vma_iterator *vmi;
+ unsigned int mm_wr_seq;
+};
+
DECLARE_TRACEPOINT(mmap_lock_start_locking);
DECLARE_TRACEPOINT(mmap_lock_acquire_returned);
DECLARE_TRACEPOINT(mmap_lock_released);
@@ -509,6 +516,16 @@ static inline void vma_mark_detached(struct vm_area_struct *vma)
struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
unsigned long address);
+static inline void vma_rcu_iter_init(struct vma_rcu_iter *rvmi,
+ struct mm_struct *mm, struct vma_iterator *vmi,
+ unsigned long addr)
+{
+ vma_iter_init(vmi, mm, addr);
+ rvmi->mm = mm;
+ rvmi->vmi = vmi;
+ rvmi->mm_wr_seq = UINT_MAX; /* Start odd so changed by default. */
+}
+
/*
* Locks next vma pointed by the iterator. Confirms the locked vma has not
* been modified and will retry under mmap_lock protection if modification
@@ -516,9 +533,8 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
* Returns either a valid locked VMA, NULL if no more VMAs or -EINTR if the
* process was interrupted.
*/
-struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
- struct vma_iterator *iter,
- unsigned long address);
+struct vm_area_struct *lock_next_vma(struct vma_rcu_iter *rvmi,
+ unsigned long from_addr);
static inline void vma_assert_can_modify(struct vm_area_struct *vma)
{
diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c
index 83b7d3ded0dc..834bbf86e004 100644
--- a/mm/mmap_lock.c
+++ b/mm/mmap_lock.c
@@ -13,6 +13,8 @@
#include <linux/trace_events.h>
#include <linux/local_lock.h>
+#include "internal.h"
+
EXPORT_TRACEPOINT_SYMBOL(mmap_lock_start_locking);
EXPORT_TRACEPOINT_SYMBOL(mmap_lock_acquire_returned);
EXPORT_TRACEPOINT_SYMBOL(mmap_lock_released);
@@ -428,21 +430,53 @@ static struct vm_area_struct *lock_next_vma_under_mmap_lock(struct mm_struct *mm
return vma;
}
-struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
- struct vma_iterator *vmi,
+/* Is there no VMA in [start, last]? */
+static bool vma_range_is_empty(struct mm_struct *mm, unsigned long start,
+ unsigned long last)
+{
+ VMA_ITERATOR(vmi, mm, start);
+
+ return !vma_iter_load(&vmi) && vma_iter_end(&vmi) - 1 >= last;
+}
+
+/* Might the VMA tree have been modified since last mm speculation? */
+static bool vma_rcu_iter_maybe_changed(const struct vma_rcu_iter *rvmi)
+{
+ return (rvmi->mm_wr_seq & 1) || /* Odd implies write-lock. */
+ mmap_lock_speculate_retry(rvmi->mm, rvmi->mm_wr_seq);
+}
+
+/* Could the VMA iterator have skipped a VMA due to the tree changing? */
+static bool vma_rcu_iter_may_have_skipped(const struct vma_rcu_iter *rvmi,
+ unsigned long from, unsigned long to)
+{
+ return vma_rcu_iter_maybe_changed(rvmi) &&
+ !vma_range_is_empty(rvmi->mm, from, to);
+}
+
+struct vm_area_struct *lock_next_vma(struct vma_rcu_iter *rvmi,
unsigned long from_addr)
{
+ struct mm_struct *mm = rvmi->mm;
+ struct vma_iterator *vmi = rvmi->vmi;
struct vm_area_struct *vma;
- unsigned int mm_wr_seq;
- bool mmap_unlocked;
RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "no rcu read lock held");
retry:
- /* Start mmap_lock speculation in case we need to verify the vma later */
- mmap_unlocked = mmap_lock_speculate_try_begin(mm, &mm_wr_seq);
+ /* Only trust the iterator's position if the tree cannot have changed. */
+ if (vma_rcu_iter_maybe_changed(rvmi)) {
+ vma_iter_set(vmi, from_addr);
+ mmap_lock_speculate_try_begin(mm, &rvmi->mm_wr_seq);
+ }
vma = vma_next(vmi);
- if (!vma)
- return NULL;
+ if (!vma) {
+ /* No reason to fallback if nothing left in range. */
+ if (!vma_rcu_iter_may_have_skipped(rvmi, from_addr, ULONG_MAX))
+ return NULL;
+
+ rcu_read_unlock();
+ goto fallback;
+ }
vma = vma_start_read(mm, vma);
if (IS_ERR_OR_NULL(vma)) {
@@ -471,14 +505,9 @@ struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
* installed ahead of it. Otherwise we might observe a gap that should
* not be there.
*/
- if (from_addr < vma->vm_start) {
- /* Verify only if the address space might have changed since vma lookup. */
- if (!mmap_unlocked || mmap_lock_speculate_retry(mm, mm_wr_seq)) {
- vma_iter_set(vmi, from_addr);
- if (vma != vma_next(vmi))
- goto fallback_unlock;
- }
- }
+ if (from_addr < vma->vm_start &&
+ vma_rcu_iter_may_have_skipped(rvmi, from_addr, vma->vm_start - 1))
+ goto fallback_unlock;
return vma;
---
base-commit: 33dbd62d6ff310b8cce8c70101639a6eddbb1b52
change-id: 20261027-fix-proc-pid-maps-missing-vmas-c870d14e1ca9
Cheers,
--
Lorenzo Stoakes (ARM) <ljs@kernel.org>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends
2026-10-27 14:08 [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends Lorenzo Stoakes (ARM)
@ 2026-10-27 15:34 ` Suren Baghdasaryan
2026-10-27 17:48 ` Lorenzo Stoakes (ARM)
2026-10-27 22:10 ` Jose A. Perez de Azpillaga
1 sibling, 1 reply; 5+ messages in thread
From: Suren Baghdasaryan @ 2026-10-27 15:34 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
Pedro Falcato, Shakeel Butt, SJ Park, linux-kernel, linux-fsdevel,
linux-mm, stable, David Hildenbrand (Arm)
On Tue, Oct 27, 2026 at 7:08 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>
> Reading /proc/PID/maps under per-VMA locks can skip continuously mapped
> address ranges during concurrent VMA splits or merges. This violates the
> guarantee in Documentation/filesystems/proc.rst:
>
> "If there is something at a given vaddr during the entirety of the
> life of the smaps/maps walk, there will be some output for it."
>
> lock_next_vma() reuses a tree iterator across calls under RCU. The iterator
> remembers the range of its last entry. When its node is replaced,
> mas_next_slot() walks the tree again at the last index of that range, then
> advances past the entry containing that index. If VMA boundaries changed,
> this can skip a range that has not been reported:
>
> - A lookup finds [A, C), then mprotect() splits it into [A, B) and [B, C)
> before vma_start_read() locks the first VMA. The reader reports [A, B),
> but the iterator still covers [A, C). The next call starts from B yet
> advances past C, omitting [B, C).
>
> - The reader reports [A, B) and unlocks it. A concurrent mprotect() then
> merges it with [B, C). On the next call, the iterator resumes at B - 1,
> which now belongs to [A, C), and advances past C. Again, [B, C) is lost.
>
> Checking the mmap_lock sequence only within the next call misses writers
> that completed before that call began. Also, a NULL result is currently
> accepted without validation, so a race can terminate the walk early.
>
> Even a fresh lookup can skip a mapping. Suppose it finds a gap before
> [B, C). A writer inserts an adjacent VMA at the end of that gap, then
> merges it into [B, C), replacing the node. The lookup resumes at the old
> gap's last index, now inside the merged VMA, and steps past it. The
> original [B, C) remains mapped throughout. Repeating the forward lookup
> is insufficient: another merge can make it skip the same mapping again,
> returning the same later VMA or NULL.
>
> The fresh-lookup race also affects /proc/PID/smaps, numa_maps and
> smaps_rollup. Resetting the iterator after gathering each VMA's statistics
> does not prevent a merge during the next lookup. Fixing lock_next_vma()
> also prevents omitted entries and incomplete rollup statistics.
>
> Tie VMA iterator state with the current mm and the write generation at the
> point of last mm speculation in vma_rcu_iter, whose values we call 'rvmi'.
>
> This allows us to neatly abstract the existing priv->iter logic alongside
> query_vma_find_by_addr() which uses its own VMA iterator.
>
> Then, provide helpers that act on the rvmi - vma_rcu_iter_[maybe_changed,
> may_have_skipped]() to determine whether any VMAs may have
> been altered since last mm speculation.
>
> Additionally introduce vma_range_is_empty(), used by
> vma_rcu_iter_may_have_skipped(), to determine conclusively whether VMAs
> exist in a set range, needing to be traversed.
>
> Original fix generated by an LLM and sent by Josef Bacik in private, then
> amended by David Hildenbrand, and Lorenzo Stoakes added additional
> abstraction.
>
> Fixes: 5631da56c9a8 ("fs/proc/task_mmu: read proc/pid/maps under per-vma lock")
> Fixes: ea3085dd7a4e ("fs/proc/task_mmu: read proc/pid/{smaps|numa_maps} under per-vma lock")
> Fixes: 0e985559d72a ("proc/task_mmu: read proc/pid/smaps_rollup under per-vma lock")
> Fixes: d9d1c2d81797 ("fs/proc/task_mmu: execute PROCMAP_QUERY ioctl under per-vma locks")
> Cc: stable@vger.kernel.org
> Co-developed-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Overall the code looks correct bit I have a couple of questions below.
> ---
> Note that I decided it's best to defer any VMA iterator stuff to a
> follow-up - with the abstractions added it's clear what's going on and
> easier for backporting.
Ok. But once we get VMA iterator validation routine implemented, we
won't need vma_rcu_iter and its helpers, I think. Are you ok with the
fact that most of this change will have to be removed later?
> ---
> v2:
> * Took over the patch to make David's life easier :)
> * Introduced vma_rcu_iter tying the VMA iterator, mm and sampled mm seqnum
> together, and update lock_next_vma() + callers to use it.
> * Moved the sequence out of proc_maps_locking_ctx and the "must initially
> be odd" requirement out of the header into vma_rcu_iter_init().
> * Added vma_rcu_iter_maybe_changed() and vma_rcu_iter_may_have_skipped()
> to replace the open-coded speculation checks.
> * Converted vma_range_is_empty() to use a VMA iterator to be consistent.
> * Added Fixes: tag for d9d1c2d81797 (PROCMAP_QUERY), as per Jose.
>
> v1:
> https://lore.kernel.org/r/20261025-per_vma_maps-v1-1-c4eec45b9387@kernel.org
> ---
> fs/proc/internal.h | 1 +
> fs/proc/task_mmu.c | 13 +++++-----
> include/linux/mmap_lock.h | 22 ++++++++++++++---
> mm/mmap_lock.c | 61 ++++++++++++++++++++++++++++++++++-------------
> 4 files changed, 72 insertions(+), 25 deletions(-)
>
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 623bb43ede55..1d95ff577a06 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -393,6 +393,7 @@ struct proc_maps_private {
> struct inode *inode;
> struct task_struct *task;
> struct vma_iterator iter;
> + struct vma_rcu_iter rvmi;
> loff_t last_pos;
> struct proc_maps_locking_ctx lock_ctx;
> #ifdef CONFIG_NUMA
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 74ffa5c99b8e..22361607020d 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -165,7 +165,7 @@ static struct vm_area_struct *get_next_vma(struct proc_maps_private *priv,
> return vma_next(&priv->iter);
>
> unlock_ctx_vma(lock_ctx);
> - vma = lock_next_vma(lock_ctx->mm, &priv->iter, last_pos);
> + vma = lock_next_vma(&priv->rvmi, last_pos);
> if (!IS_ERR_OR_NULL(vma))
> lock_ctx->locked_vma = vma;
>
> @@ -270,7 +270,7 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
> */
> if (last_addr && !addr_is_sentinel(last_addr))
> *ppos = last_addr = priv->last_pos;
> - vma_iter_init(&priv->iter, mm, (unsigned long)last_addr);
> + vma_rcu_iter_init(&priv->rvmi, mm, &priv->iter, (unsigned long)last_addr);
> hold_task_mempolicy(priv);
> /*
> * If seq_file had to flush its collected data right after m_next() set
> @@ -520,6 +520,7 @@ static struct vm_area_struct *query_vma_find_by_addr(struct proc_maps_locking_ct
> struct mm_struct *mm = lock_ctx->mm;
> struct vm_area_struct *vma;
> struct vma_iterator vmi;
> + struct vma_rcu_iter rvmi;
>
> if (lock_ctx->mmap_locked)
> return find_vma(mm, addr);
> @@ -527,8 +528,8 @@ static struct vm_area_struct *query_vma_find_by_addr(struct proc_maps_locking_ct
> /* Unlock previously locked VMA and find the next one under RCU */
> unlock_ctx_vma(lock_ctx);
> rcu_read_lock();
> - vma_iter_init(&vmi, mm, addr);
> - vma = lock_next_vma(mm, &vmi, addr);
> + vma_rcu_iter_init(&rvmi, mm, &vmi, addr);
> + vma = lock_next_vma(&rvmi, addr);
> rcu_read_unlock();
>
> if (!vma)
> @@ -1421,7 +1422,7 @@ static int show_smaps_rollup(struct seq_file *m, void *v)
> rcu_read_lock();
> reset_lock_ctx(lock_ctx);
>
> - vma_iter_init(&priv->iter, mm, 0);
> + vma_rcu_iter_init(&priv->rvmi, mm, &priv->iter, 0);
> vma = proc_get_vma(m, &pos);
> if (unlikely(!vma))
> goto empty_set;
> @@ -1470,7 +1471,7 @@ static int show_smaps_rollup(struct seq_file *m, void *v)
> reset_lock_ctx(lock_ctx);
> /* Resume from the last position. */
> pos = last_vma_end;
> - vma_iter_init(&priv->iter, mm, pos);
> + vma_rcu_iter_init(&priv->rvmi, mm, &priv->iter, pos);
> }
> vma = proc_get_vma(m, &pos);
> }
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 11b3ffef3966..9ccbbcbd7aca 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -17,6 +17,13 @@ extern int rcuwait_wake_up(struct rcuwait *w);
> #define MMAP_LOCK_INITIALIZER(name) \
> .mmap_lock = __RWSEM_INITIALIZER((name).mmap_lock),
>
> +/* Tie a VMA iterator to mm write lock state at iterator initialisation. */
> +struct vma_rcu_iter {
> + struct mm_struct *mm;
> + struct vma_iterator *vmi;
> + unsigned int mm_wr_seq;
> +};
> +
> DECLARE_TRACEPOINT(mmap_lock_start_locking);
> DECLARE_TRACEPOINT(mmap_lock_acquire_returned);
> DECLARE_TRACEPOINT(mmap_lock_released);
> @@ -509,6 +516,16 @@ static inline void vma_mark_detached(struct vm_area_struct *vma)
> struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> unsigned long address);
>
> +static inline void vma_rcu_iter_init(struct vma_rcu_iter *rvmi,
> + struct mm_struct *mm, struct vma_iterator *vmi,
> + unsigned long addr)
> +{
> + vma_iter_init(vmi, mm, addr);
> + rvmi->mm = mm;
> + rvmi->vmi = vmi;
> + rvmi->mm_wr_seq = UINT_MAX; /* Start odd so changed by default. */
Why not initialize mm_wr_seq correctly using
mmap_lock_speculate_try_begin(mm, &rvmi->mm_wr_seq)? That way on the
first call to lock_next_vma() you won't be resetting vmi
unnecessarily.
> +}
> +
> /*
> * Locks next vma pointed by the iterator. Confirms the locked vma has not
> * been modified and will retry under mmap_lock protection if modification
> @@ -516,9 +533,8 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> * Returns either a valid locked VMA, NULL if no more VMAs or -EINTR if the
> * process was interrupted.
> */
> -struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
> - struct vma_iterator *iter,
> - unsigned long address);
> +struct vm_area_struct *lock_next_vma(struct vma_rcu_iter *rvmi,
> + unsigned long from_addr);
>
> static inline void vma_assert_can_modify(struct vm_area_struct *vma)
> {
> diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c
> index 83b7d3ded0dc..834bbf86e004 100644
> --- a/mm/mmap_lock.c
> +++ b/mm/mmap_lock.c
> @@ -13,6 +13,8 @@
> #include <linux/trace_events.h>
> #include <linux/local_lock.h>
>
> +#include "internal.h"
> +
> EXPORT_TRACEPOINT_SYMBOL(mmap_lock_start_locking);
> EXPORT_TRACEPOINT_SYMBOL(mmap_lock_acquire_returned);
> EXPORT_TRACEPOINT_SYMBOL(mmap_lock_released);
> @@ -428,21 +430,53 @@ static struct vm_area_struct *lock_next_vma_under_mmap_lock(struct mm_struct *mm
> return vma;
> }
>
> -struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
> - struct vma_iterator *vmi,
> +/* Is there no VMA in [start, last]? */
> +static bool vma_range_is_empty(struct mm_struct *mm, unsigned long start,
> + unsigned long last)
> +{
> + VMA_ITERATOR(vmi, mm, start);
> +
> + return !vma_iter_load(&vmi) && vma_iter_end(&vmi) - 1 >= last;
> +}
> +
> +/* Might the VMA tree have been modified since last mm speculation? */
> +static bool vma_rcu_iter_maybe_changed(const struct vma_rcu_iter *rvmi)
> +{
> + return (rvmi->mm_wr_seq & 1) || /* Odd implies write-lock. */
> + mmap_lock_speculate_retry(rvmi->mm, rvmi->mm_wr_seq);
> +}
> +
> +/* Could the VMA iterator have skipped a VMA due to the tree changing? */
> +static bool vma_rcu_iter_may_have_skipped(const struct vma_rcu_iter *rvmi,
> + unsigned long from, unsigned long to)
> +{
> + return vma_rcu_iter_maybe_changed(rvmi) &&
> + !vma_range_is_empty(rvmi->mm, from, to);
> +}
> +
> +struct vm_area_struct *lock_next_vma(struct vma_rcu_iter *rvmi,
> unsigned long from_addr)
> {
> + struct mm_struct *mm = rvmi->mm;
> + struct vma_iterator *vmi = rvmi->vmi;
> struct vm_area_struct *vma;
> - unsigned int mm_wr_seq;
> - bool mmap_unlocked;
>
> RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "no rcu read lock held");
> retry:
> - /* Start mmap_lock speculation in case we need to verify the vma later */
> - mmap_unlocked = mmap_lock_speculate_try_begin(mm, &mm_wr_seq);
> + /* Only trust the iterator's position if the tree cannot have changed. */
> + if (vma_rcu_iter_maybe_changed(rvmi)) {
> + vma_iter_set(vmi, from_addr);
> + mmap_lock_speculate_try_begin(mm, &rvmi->mm_wr_seq);
> + }
> vma = vma_next(vmi);
> - if (!vma)
> - return NULL;
> + if (!vma) {
> + /* No reason to fallback if nothing left in range. */
> + if (!vma_rcu_iter_may_have_skipped(rvmi, from_addr, ULONG_MAX))
> + return NULL;
> +
> + rcu_read_unlock();
> + goto fallback;
> + }
>
> vma = vma_start_read(mm, vma);
> if (IS_ERR_OR_NULL(vma)) {
> @@ -471,14 +505,9 @@ struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
> * installed ahead of it. Otherwise we might observe a gap that should
> * not be there.
> */
> - if (from_addr < vma->vm_start) {
> - /* Verify only if the address space might have changed since vma lookup. */
> - if (!mmap_unlocked || mmap_lock_speculate_retry(mm, mm_wr_seq)) {
> - vma_iter_set(vmi, from_addr);
> - if (vma != vma_next(vmi))
> - goto fallback_unlock;
> - }
> - }
> + if (from_addr < vma->vm_start &&
> + vma_rcu_iter_may_have_skipped(rvmi, from_addr, vma->vm_start - 1))
> + goto fallback_unlock;
Before this change we fell back only if the new search yielded a
different VMA (the vma != vma_next(vmi) condition) while now we
fallback unconditionally. Note that the VMA that we are rechecking is
already locked, so it can't change. Was the previous logic invalid?
>
> return vma;
>
>
> ---
> base-commit: 33dbd62d6ff310b8cce8c70101639a6eddbb1b52
> change-id: 20261027-fix-proc-pid-maps-missing-vmas-c870d14e1ca9
>
> Cheers,
> --
> Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends
2026-10-27 15:34 ` Suren Baghdasaryan
@ 2026-10-27 17:48 ` Lorenzo Stoakes (ARM)
2026-10-27 20:33 ` Suren Baghdasaryan
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-27 17:48 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
Pedro Falcato, Shakeel Butt, SJ Park, linux-kernel, linux-fsdevel,
linux-mm, stable, David Hildenbrand (Arm)
On Tue, Oct 27, 2026 at 08:34:10AM -0700, Suren Baghdasaryan wrote:
> Overall the code looks correct bit I have a couple of questions below.
Thanks!
>
> > ---
> > Note that I decided it's best to defer any VMA iterator stuff to a
> > follow-up - with the abstractions added it's clear what's going on and
> > easier for backporting.
>
> Ok. But once we get VMA iterator validation routine implemented, we
> won't need vma_rcu_iter and its helpers, I think. Are you ok with the
> fact that most of this change will have to be removed later?
No, I don't think this is correct.
The iterator can't validate itself - the failure case is invisible to it,
something changed in the tree and it has no way of knowing, it has to be
prompted to re-init etc.
I tried a version of the code with vma_iter_next_before() and it really didn't
provide much delta.
So I think any version of this has to have the rvmi stuff.
In any case, I am happy to replace all this with whatever makes most sense
going forwards.
Key here is to fix the bug + get it backported :) [ before moving on to the
N other AI-reported bugs ;) ]
> > +static inline void vma_rcu_iter_init(struct vma_rcu_iter *rvmi,
> > + struct mm_struct *mm, struct vma_iterator *vmi,
> > + unsigned long addr)
> > +{
> > + vma_iter_init(vmi, mm, addr);
> > + rvmi->mm = mm;
> > + rvmi->vmi = vmi;
> > + rvmi->mm_wr_seq = UINT_MAX; /* Start odd so changed by default. */
>
> Why not initialize mm_wr_seq correctly using
> mmap_lock_speculate_try_begin(mm, &rvmi->mm_wr_seq)? That way on the
> first call to lock_next_vma() you won't be resetting vmi
> unnecessarily.
It doesn't really save anything, the init already sets you up in start state
where you need to do a traversal anyway, and vma_next() will trigger a walk
either way.
What's useful about it is it makes it such that only lock_next_vma() does the
speculation (and thus sampling of mm_wr_seq).
> > @@ -471,14 +505,9 @@ struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
> > * installed ahead of it. Otherwise we might observe a gap that should
> > * not be there.
> > */
> > - if (from_addr < vma->vm_start) {
> > - /* Verify only if the address space might have changed since vma lookup. */
> > - if (!mmap_unlocked || mmap_lock_speculate_retry(mm, mm_wr_seq)) {
> > - vma_iter_set(vmi, from_addr);
> > - if (vma != vma_next(vmi))
> > - goto fallback_unlock;
> > - }
> > - }
> > + if (from_addr < vma->vm_start &&
> > + vma_rcu_iter_may_have_skipped(rvmi, from_addr, vma->vm_start - 1))
> > + goto fallback_unlock;
>
> Before this change we fell back only if the new search yielded a
> different VMA (the vma != vma_next(vmi) condition) while now we
> fallback unconditionally. Note that the VMA that we are rechecking is
> already locked, so it can't change. Was the previous logic invalid?
It's not unconditional - it requires that we have progressed and we may have
skipped a VMA change.
The 'we have progressed' bit was there before (from_addr < vma->vm_start), and
vma_rcu_iter_may_have_skipped() wraps the previous check plus actually walks to
check if something is there to traverse,
So this is strictly an improvement.
And yes the previous logic was invalid - the issue is with an unlocked VMA
in the range [from_addr, vma->vm_start - 1].
It's really the gap that's the problem AND a race against node replacement,
which means that 'X' is skipped by vma_next().
vma_next() is a walk to from_addr, then runs mas_next_slot() which advances
to the next entry. If a gap, then the walk assigns mas->last to the end of
the gap, and if a node is killed before that happens, due to a VMA
expanding _backwards_ like:
vma_iter_set(vmi, from_addr), then mas_walk() from mas_find():
from_addr
|
v
|---------| |-----| |---------|
| A | | X | | B |
|---------| |-----| |---------|
^ ^
| |
mas->index mas->last
The walk lands in the gap. mas_next_slot() saves mas->last as its
rewalk point.
X expands backwards, the node containing the gap is replaced:
from_addr
|
v
|---------| |---------| |---------|
| A | | X | | B |
|---------| |---------| |---------|
^
|
mas->last
mas_next_slot() finds the node dead and calls mas_rewalk(mas->last),
which now lands inside X:
from_addr
|
v
|---------| |---------| |---------|
| A | | X | | B |
|---------| |---------| |---------|
^ ^
| |
mas->index mas->last
mas_next_slot() then advances past the entry it is on, which is X:
from_addr vmi
| |
v v
|---------| |---------| |---------|
| A | | X | | B |
|---------| |---------| |---------|
So vma_next() returns B, having stepped over X. We lock B:
from_addr vmi
| |
v v
|---------| |---------| |---------|
| A | | X | | B |
|---------| |---------| |---------|
locked
Then if you get (really) unlucky, the same race happens again for the
existing code:
vma_iter_set(vmi, from_addr);
if (vma != vma_next(vmi))
goto fallback_unlock;
The walk lands in the gap before X, something else is merged into X from
below, the rewalk lands inside X and the advance steps over it again:
from_addr vmi
| |
v v
|---------| |----------| |---------|
| A | | X | | B |
|---------| |----------| |---------|
locked
And suddenly vma == vma_next(vmi), no fallback, and X is never reported.
With the new approach you simply can't miss it, as
vma_rcu_iter_maybe_changed() will see a change could have happened, and
vma_range_is_empty() tells you explicitly, with a brand new separate walk,
whether e.g. X is there.
It's finnicky, finnicky stuff, which is probably why AI found it :))
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends
2026-10-27 17:48 ` Lorenzo Stoakes (ARM)
@ 2026-10-27 20:33 ` Suren Baghdasaryan
0 siblings, 0 replies; 5+ messages in thread
From: Suren Baghdasaryan @ 2026-10-27 20:33 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
Pedro Falcato, Shakeel Butt, SJ Park, linux-kernel, linux-fsdevel,
linux-mm, stable, David Hildenbrand (Arm)
On Tue, Oct 27, 2026 at 10:49 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>
> On Tue, Oct 27, 2026 at 08:34:10AM -0700, Suren Baghdasaryan wrote:
> > Overall the code looks correct bit I have a couple of questions below.
>
> Thanks!
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
>
> >
> > > ---
> > > Note that I decided it's best to defer any VMA iterator stuff to a
> > > follow-up - with the abstractions added it's clear what's going on and
> > > easier for backporting.
> >
> > Ok. But once we get VMA iterator validation routine implemented, we
> > won't need vma_rcu_iter and its helpers, I think. Are you ok with the
> > fact that most of this change will have to be removed later?
>
> No, I don't think this is correct.
>
> The iterator can't validate itself - the failure case is invisible to it,
> something changed in the tree and it has no way of knowing, it has to be
> prompted to re-init etc.
>
> I tried a version of the code with vma_iter_next_before() and it really didn't
> provide much delta.
>
> So I think any version of this has to have the rvmi stuff.
Hmm. I though the fact that we store from_addr and can compare it with
mas->last could help us detect VMA skips but I'll have to think some
more about this.
>
> In any case, I am happy to replace all this with whatever makes most sense
> going forwards.
>
> Key here is to fix the bug + get it backported :) [ before moving on to the
> N other AI-reported bugs ;) ]
Understood. Yeah, I'm not going to nitpick here as you have your hands
full. If we find a better way we can improve it later.
>
> > > +static inline void vma_rcu_iter_init(struct vma_rcu_iter *rvmi,
> > > + struct mm_struct *mm, struct vma_iterator *vmi,
> > > + unsigned long addr)
> > > +{
> > > + vma_iter_init(vmi, mm, addr);
> > > + rvmi->mm = mm;
> > > + rvmi->vmi = vmi;
> > > + rvmi->mm_wr_seq = UINT_MAX; /* Start odd so changed by default. */
> >
> > Why not initialize mm_wr_seq correctly using
> > mmap_lock_speculate_try_begin(mm, &rvmi->mm_wr_seq)? That way on the
> > first call to lock_next_vma() you won't be resetting vmi
> > unnecessarily.
>
> It doesn't really save anything, the init already sets you up in start state
> where you need to do a traversal anyway, and vma_next() will trigger a walk
> either way.
You might end up with an extra vma_iter_set() call but it's not critical at all.
>
> What's useful about it is it makes it such that only lock_next_vma() does the
> speculation (and thus sampling of mm_wr_seq).
OTOH you won't be exposing these odd/even seqcount details. Anyway,
I'm fine with what you have.
>
> > > @@ -471,14 +505,9 @@ struct vm_area_struct *lock_next_vma(struct mm_struct *mm,
> > > * installed ahead of it. Otherwise we might observe a gap that should
> > > * not be there.
> > > */
> > > - if (from_addr < vma->vm_start) {
> > > - /* Verify only if the address space might have changed since vma lookup. */
> > > - if (!mmap_unlocked || mmap_lock_speculate_retry(mm, mm_wr_seq)) {
> > > - vma_iter_set(vmi, from_addr);
> > > - if (vma != vma_next(vmi))
> > > - goto fallback_unlock;
> > > - }
> > > - }
> > > + if (from_addr < vma->vm_start &&
> > > + vma_rcu_iter_may_have_skipped(rvmi, from_addr, vma->vm_start - 1))
> > > + goto fallback_unlock;
> >
> > Before this change we fell back only if the new search yielded a
> > different VMA (the vma != vma_next(vmi) condition) while now we
> > fallback unconditionally. Note that the VMA that we are rechecking is
> > already locked, so it can't change. Was the previous logic invalid?
>
> It's not unconditional - it requires that we have progressed and we may have
> skipped a VMA change.
>
> The 'we have progressed' bit was there before (from_addr < vma->vm_start), and
> vma_rcu_iter_may_have_skipped() wraps the previous check plus actually walks to
> check if something is there to traverse,
Ok, I see. vma_rcu_iter_may_have_skipped() effectively replaces (vma
!= vma_next(vmi)).
>
> So this is strictly an improvement.
>
> And yes the previous logic was invalid - the issue is with an unlocked VMA
> in the range [from_addr, vma->vm_start - 1].
>
> It's really the gap that's the problem AND a race against node replacement,
> which means that 'X' is skipped by vma_next().
>
> vma_next() is a walk to from_addr, then runs mas_next_slot() which advances
> to the next entry. If a gap, then the walk assigns mas->last to the end of
> the gap, and if a node is killed before that happens, due to a VMA
> expanding _backwards_ like:
>
> vma_iter_set(vmi, from_addr), then mas_walk() from mas_find():
>
> from_addr
> |
> v
> |---------| |-----| |---------|
> | A | | X | | B |
> |---------| |-----| |---------|
> ^ ^
> | |
> mas->index mas->last
>
> The walk lands in the gap. mas_next_slot() saves mas->last as its
> rewalk point.
>
> X expands backwards, the node containing the gap is replaced:
>
> from_addr
> |
> v
> |---------| |---------| |---------|
> | A | | X | | B |
> |---------| |---------| |---------|
> ^
> |
> mas->last
>
> mas_next_slot() finds the node dead and calls mas_rewalk(mas->last),
> which now lands inside X:
>
> from_addr
> |
> v
> |---------| |---------| |---------|
> | A | | X | | B |
> |---------| |---------| |---------|
> ^ ^
> | |
> mas->index mas->last
>
> mas_next_slot() then advances past the entry it is on, which is X:
>
> from_addr vmi
> | |
> v v
> |---------| |---------| |---------|
> | A | | X | | B |
> |---------| |---------| |---------|
>
> So vma_next() returns B, having stepped over X. We lock B:
>
> from_addr vmi
> | |
> v v
> |---------| |---------| |---------|
> | A | | X | | B |
> |---------| |---------| |---------|
> locked
Ok, so here we hit this "if (from_addr < vma->vm_start)" condition and
have to verify that we did not skip a VMA.
>
> Then if you get (really) unlucky, the same race happens again for the
> existing code:
>
> vma_iter_set(vmi, from_addr);
> if (vma != vma_next(vmi))
> goto fallback_unlock;
>
> The walk lands in the gap before X, something else is merged into X from
> below, the rewalk lands inside X and the advance steps over it again:
>
> from_addr vmi
> | |
> v v
> |---------| |----------| |---------|
> | A | | X | | B |
> |---------| |----------| |---------|
> locked
>
> And suddenly vma == vma_next(vmi), no fallback, and X is never reported.
Got it. The same skip must happen the first time and then during our
verification. Thanks for the detailed explanation.
>
> With the new approach you simply can't miss it, as
> vma_rcu_iter_maybe_changed() will see a change could have happened, and
> vma_range_is_empty() tells you explicitly, with a brand new separate walk,
> whether e.g. X is there.
Yep, that function name is indeed very clear.
>
> It's finnicky, finnicky stuff, which is probably why AI found it :))
>
> --
> Cheers, Lorenzo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends
2026-10-27 14:08 [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends Lorenzo Stoakes (ARM)
2026-10-27 15:34 ` Suren Baghdasaryan
@ 2026-10-27 22:10 ` Jose A. Perez de Azpillaga
1 sibling, 0 replies; 5+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-10-27 22:10 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM), Andrew Morton, Liam R. Howlett,
Vlastimil Babka, Jann Horn, Pedro Falcato, Suren Baghdasaryan,
Shakeel Butt, SJ Park
Cc: linux-kernel, linux-fsdevel, linux-mm, stable,
David Hildenbrand (Arm)
On 10/27/26 3:08 PM, Lorenzo Stoakes (ARM) wrote:
[snip]
> + if (from_addr < vma->vm_start &&
> + vma_rcu_iter_may_have_skipped(rvmi, from_addr, vma->vm_start - 1))
> + goto fallback_unlock;
the helpers make this a lot easier to follow than v1 :)
I re-tested v2 on x86_64 in qemu with KASAN, as in v1. So, this LGTM:
Reviewed-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
Tested-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
--
cheers,
jose a. p-a
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-27 22:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 14:08 [PATCH v2] fs/proc/task_mmu: fix missed VMAs when reading /proc/PID/maps and friends Lorenzo Stoakes (ARM)
2026-10-27 15:34 ` Suren Baghdasaryan
2026-10-27 17:48 ` Lorenzo Stoakes (ARM)
2026-10-27 20:33 ` Suren Baghdasaryan
2026-10-27 22:10 ` Jose A. Perez de Azpillaga
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox