[PATCH 0/4] mm: avoid losing atomic VMA flags

[PATCH 0/4] mm: avoid losing atomic VMA flags

[Lorenzo Stoakes (ARM)]

Since commit 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove
under VMA lock"), VMA_MAYBE_GUARD_BIT can be set atomically under the VMA
read lock.

However, a number of places wrongly assume the flags are stable under the
read lock, which can cause stale atomic VMA flags to overwrite existing
valid ones.

Additionally, one case in which VMA_SOFTDIRTY_BIT was overwritten is also
addressed.

This series addresses each of these, while keeping the logic inline with
existing conventions.

Each patch in this series is a hotfix.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
David Hildenbrand (Arm) (2):
      mm/vma: write lock source VMA before duplicating it in __split_vma()
      mm/madvise: preserve VMA_SOFTDIRTY_BIT when changing anonymous names

Lorenzo Stoakes (ARM) (2):
      mm: do not unintentionally overwrite atomic VMA flags
      mm/vma: avoid losing atomic VMA flags when setting sticky VMA flags

 include/linux/mm.h                | 166 +++++++++++++++++++-------------------
 mm/madvise.c                      |  10 +--
 mm/mlock.c                        |   6 +-
 mm/mprotect.c                     |   2 +-
 mm/vma.c                          |  57 +++++++------
 mm/vma_init.c                     |   3 +
 tools/testing/vma/include/dup.h   |  78 ++++++++++--------
 tools/testing/vma/include/stubs.h |   2 +-
 tools/testing/vma/shared.h        |  16 ++--
 9 files changed, 185 insertions(+), 155 deletions(-)
---
base-commit: 5f980b2d53abf3950caff5d77b5c69dc6ebb19b6
change-id: 20261022-fix-vma-atomic-8c910fd66a28

Cheers,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>

[PATCH 1/4] mm: do not unintentionally overwrite atomic VMA flags

[Lorenzo Stoakes (ARM)]

madvise, mlock and mprotect take a copy of a VMA's flags holding only the
VMA read lock, later writing it back under the write lock (in
madvise_update_vma(), mlock_fixup(), mlock_vma_pages_range() and
mprotect_fixup()).

This was done on the assumption that VMA flags cannot change with a read
lock held.

However this changed in commit 2ab7f1bbafc9 ("mm/madvise: allow guard page
install/remove under VMA lock"), which allowed VM_ATOMIC_SET_ALLOWED
flags (currently only VMA_MAYBE_GUARD_BIT) to be written concurrently under
a VMA read lock.

The actual write itself is safe as this is done under the VMA write lock
at which point no further concurrent writes can occur, however at this
point the stale flags mean that these callers can unintentionally overwrite
atomic flags incorrectly.

The consequences of this are significant - a missing VMA_MAYBE_GUARD_BIT
could result in page tables not being copied on fork (the vma_needs_copy()
check can return false) or being overwritten on file-backed THP
collapse (the file_backed_vma_is_retractable() can return true).

Resolve the issue by always taking care to retain existing atomic flags
when overwriting VMA flags, replacing the inconsistently named
vma_flags_reset_once() with vma_overwrite_flags() to do so, and always
invoking it when flags are overwritten like this.

Reads are a problem too - madvise, mlock and mprotect all compare VMA flags
without a VMA write lock held via vma_flags_same_pair().

This has more serious impact, as concurrent changes to atomic VMA flags can
cause two sets of VMA flags to be wrongly considered different.

For mprotect and madvise this causes a useless split, but mlock is impacted
more seriously as it can result in mm->locked_vm underflow.

Fix this by updating vma_flags_same_pair() to disregard
VM_ATOMIC_SET_ALLOWED VMAs - VMA equality is the same as VMA mergeability,
and atomic VMA flags are disregarded when comparing VMA flags for merge -
so this is justified semantically.

Additionally, remove the redundant vma_flags_same_mask() and the unused
vma_flags_same() as part of the change, and replace VM_ATOMIC_SET_ALLOWED
with VMA_ATOMIC_SET_ALLOWED, using the new VMA flag API.

Finally, update userland VMA tests to reflect the changes.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
Cc: <stable@vger.kernel.org>
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 include/linux/mm.h              | 166 ++++++++++++++++++++--------------------
 mm/madvise.c                    |   4 +-
 mm/mlock.c                      |   6 +-
 mm/mprotect.c                   |   2 +-
 tools/testing/vma/include/dup.h |  76 ++++++++++--------
 tools/testing/vma/shared.h      |  16 ++--
 6 files changed, 146 insertions(+), 124 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 35f5fc43861a..5f82be0fcde5 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -602,7 +602,7 @@ enum {
 #define VMA_LOCKED_MASK	mk_vma_flags(VMA_LOCKED_BIT, VMA_LOCKONFAULT_BIT)
 
 /* These flags can be updated atomically via VMA/mmap read lock. */
-#define VM_ATOMIC_SET_ALLOWED VM_MAYBE_GUARD
+#define VMA_ATOMIC_SET_ALLOWED mk_vma_flags(VMA_MAYBE_GUARD_BIT)
 
 /* Arch-specific flags to clear when updating VM flags on protection change */
 #ifndef VM_ARCH_CLEAR
@@ -987,22 +987,6 @@ static inline void vm_flags_reset(struct vm_area_struct *vma,
 	vm_flags_init(vma, flags);
 }
 
-static inline void vma_flags_reset_once(struct vm_area_struct *vma,
-					vma_flags_t *flags)
-{
-	const unsigned long word = flags->__vma_flags[0];
-
-	/* It is assumed only the first system word must be written once. */
-	vma_flags_overwrite_word_once(&vma->flags, word);
-	/* The remainder can be copied normally. */
-	if (NUM_VMA_FLAG_BITS > BITS_PER_LONG) {
-		unsigned long *dst = &vma->flags.__vma_flags[1];
-		const unsigned long *src = &flags->__vma_flags[1];
-
-		bitmap_copy(dst, src, NUM_VMA_FLAG_BITS - BITS_PER_LONG);
-	}
-}
-
 static inline void vm_flags_set(struct vm_area_struct *vma,
 				vm_flags_t flags)
 {
@@ -1039,48 +1023,6 @@ static inline void vm_flags_mod(struct vm_area_struct *vma,
 	__vm_flags_mod(vma, set, clear);
 }
 
-static __always_inline bool __vma_atomic_valid_flag(struct vm_area_struct *vma,
-		vma_flag_t bit)
-{
-	const vm_flags_t mask = BIT((__force int)bit);
-
-	/* Only specific flags are permitted */
-	if (WARN_ON_ONCE(!(mask & VM_ATOMIC_SET_ALLOWED)))
-		return false;
-
-	return true;
-}
-
-/*
- * Set VMA flag atomically. Requires only VMA/mmap read lock. Only specific
- * valid flags are allowed to do this.
- */
-static __always_inline void vma_set_atomic_flag(struct vm_area_struct *vma,
-		vma_flag_t bit)
-{
-	unsigned long *bitmap = vma->flags.__vma_flags;
-
-	vma_assert_stabilised(vma);
-	if (__vma_atomic_valid_flag(vma, bit))
-		set_bit((__force int)bit, bitmap);
-}
-
-/*
- * Test for VMA flag atomically. Requires no locks. Only specific valid flags
- * are allowed to do this.
- *
- * This is necessarily racey, so callers must ensure that serialisation is
- * achieved through some other means, or that races are permissible.
- */
-static __always_inline bool vma_test_atomic_flag(struct vm_area_struct *vma,
-		vma_flag_t bit)
-{
-	if (__vma_atomic_valid_flag(vma, bit))
-		return test_bit((__force int)bit, &vma->vm_flags);
-
-	return false;
-}
-
 /* Set an individual VMA flag in flags, non-atomically. */
 static __always_inline void vma_flags_set_flag(vma_flags_t *flags,
 		vma_flag_t bit)
@@ -1302,34 +1244,24 @@ static __always_inline vma_flags_t vma_flags_diff_pair(const vma_flags_t *flags,
 	return dst;
 }
 
-/* Determine if flags and flags_other have precisely the same flags set. */
+/*
+ * Determine if flags and flags_other have precisely the same flags set,
+ * excluding atomic VMA flags.
+ */
 static __always_inline bool vma_flags_same_pair(const vma_flags_t *flags,
-						const vma_flags_t *flags_other)
+		const vma_flags_t *flags_other)
 {
-	const unsigned long *bitmap = flags->__vma_flags;
-	const unsigned long *bitmap_other = flags_other->__vma_flags;
-
-	return bitmap_equal(bitmap, bitmap_other, NUM_VMA_FLAG_BITS);
-}
+	vma_flags_t diff = vma_flags_diff_pair(flags, flags_other);
 
-/* Determine if flags and flags_other have precisely the same flags set.  */
-static __always_inline bool vma_flags_same_mask(const vma_flags_t *flags,
-						vma_flags_t flags_other)
-{
-	const unsigned long *bitmap = flags->__vma_flags;
-	const unsigned long *bitmap_other = flags_other.__vma_flags;
+	/*
+	 * Atomic VMA flags may be set concurrently under the VMA read lock, so
+	 * are disregarded for equality testing purposes.
+	 */
+	vma_flags_clear_mask(&diff, VMA_ATOMIC_SET_ALLOWED);
 
-	return bitmap_equal(bitmap, bitmap_other, NUM_VMA_FLAG_BITS);
+	return vma_flags_empty(&diff);
 }
 
-/*
- * Helper macro to determine if only the specific flags are set, e.g.:
- *
- * if (vma_flags_same(&flags, VMA_WRITE_BIT) { ... }
- */
-#define vma_flags_same(flags, ...) \
-	vma_flags_same_mask(flags, mk_vma_flags(__VA_ARGS__))
-
 /*
  * Test whether a specific flag in the VMA is set, e.g.:
  *
@@ -1436,6 +1368,78 @@ static __always_inline void vma_clear_flags_mask(struct vm_area_struct *vma,
 #define vma_clear_flags(vma, ...) \
 	vma_clear_flags_mask(vma, mk_vma_flags(__VA_ARGS__))
 
+static __always_inline bool __vma_atomic_valid_flag(struct vm_area_struct *vma,
+		vma_flag_t bit)
+{
+	const vma_flags_t mask = VMA_ATOMIC_SET_ALLOWED;
+
+	/* Is this flag allowed to be set atomically? */
+	if (WARN_ON_ONCE(!vma_flags_test(&mask, bit)))
+		return false;
+
+	return true;
+}
+
+/*
+ * Set VMA flag atomically. Requires only VMA/mmap read lock. Only specific
+ * valid flags are allowed to do this.
+ */
+static __always_inline void vma_set_atomic_flag(struct vm_area_struct *vma,
+		vma_flag_t bit)
+{
+	unsigned long *bitmap = vma->flags.__vma_flags;
+
+	vma_assert_stabilised(vma);
+	if (__vma_atomic_valid_flag(vma, bit))
+		set_bit((__force int)bit, bitmap);
+}
+
+/*
+ * Test for VMA flag atomically. Requires no locks. Only specific valid flags
+ * are allowed to do this.
+ *
+ * This is necessarily racey, so callers must ensure that serialisation is
+ * achieved through some other means, or that races are permissible.
+ */
+static __always_inline bool vma_test_atomic_flag(struct vm_area_struct *vma,
+		vma_flag_t bit)
+{
+	if (__vma_atomic_valid_flag(vma, bit))
+		return test_bit((__force int)bit, &vma->vm_flags);
+
+	return false;
+}
+
+/*
+ * Overwrite the VMA's flags with new ones, preserving atomic VMA flags in the
+ * process. The write is done once and once only.
+ */
+static inline void vma_overwrite_flags(struct vm_area_struct *vma,
+		vma_flags_t new_flags)
+{
+	vma_flags_t atomic;
+
+	vma_assert_write_locked(vma);
+
+	/*
+	 * Preserve the VMA's atomic flags, which are kept stable by the VMA
+	 * write lock.
+	 */
+	atomic = vma_flags_and_mask(&vma->flags, VMA_ATOMIC_SET_ALLOWED);
+	vma_flags_clear_mask(&new_flags, VMA_ATOMIC_SET_ALLOWED);
+	vma_flags_set_mask(&new_flags, atomic);
+
+	/* It is assumed only the first system word must be written once. */
+	vma_flags_overwrite_word_once(&vma->flags, new_flags.__vma_flags[0]);
+	/* The remainder can be copied normally. */
+	if (NUM_VMA_FLAG_BITS > BITS_PER_LONG) {
+		unsigned long *dst = &vma->flags.__vma_flags[1];
+		const unsigned long *src = &new_flags.__vma_flags[1];
+
+		bitmap_copy(dst, src, NUM_VMA_FLAG_BITS - BITS_PER_LONG);
+	}
+}
+
 /*
  * Test whether a specific VMA flag is set in a VMA descriptor, e.g.:
  *
diff --git a/mm/madvise.c b/mm/madvise.c
index 75b409e97dbf..b2e00efcaa54 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -158,7 +158,7 @@ static int madvise_update_vma(vm_flags_t new_flags,
 	bool set_new_anon_name = madv_behavior->behavior == __MADV_SET_ANON_VMA_NAME;
 	VMA_ITERATOR(vmi, madv_behavior->mm, range->start);
 
-	if (vma_flags_same_mask(&vma->flags, new_vma_flags) &&
+	if (vma_flags_same_pair(&vma->flags, &new_vma_flags) &&
 	    (!set_new_anon_name ||
 	     anon_vma_name_eq(anon_vma_name(vma), anon_name)))
 		return 0;
@@ -177,7 +177,7 @@ static int madvise_update_vma(vm_flags_t new_flags,
 
 	/* vm_flags is protected by the mmap_lock held in write mode. */
 	vma_start_write(vma);
-	vma->flags = new_vma_flags;
+	vma_overwrite_flags(vma, new_vma_flags);
 	/*
 	 * If the vma become good for khugepaged to scan,
 	 * register it here without waiting a page fault that
diff --git a/mm/mlock.c b/mm/mlock.c
index 67ae9b18b923..1c70a7829394 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -432,14 +432,14 @@ static void mlock_vma_pages_range(struct vm_area_struct *vma,
 	}
 
 	vma_start_write(vma);
-	vma_flags_reset_once(vma, &walk_flags);
+	vma_overwrite_flags(vma, walk_flags);
 
 	lru_add_drain();
 	walk_page_range_vma(vma, start, end, &mlock_walk_ops, (void *)lock);
 	lru_add_drain();
 
 	if (lock)
-		vma_flags_reset_once(vma, new_vma_flags);
+		vma_overwrite_flags(vma, *new_vma_flags);
 }
 
 /*
@@ -494,7 +494,7 @@ static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	    vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT)) {
 		/* No work to do, and mlocking twice would be wrong */
 		vma_start_write(vma);
-		vma->flags = *new_vma_flags;
+		vma_overwrite_flags(vma, *new_vma_flags);
 	} else {
 		mlock_vma_pages_range(vma, start, end, new_vma_flags);
 	}
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 754333698c1e..e18d4e12fc34 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -833,7 +833,7 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
 	 * held in write mode.
 	 */
 	vma_start_write(vma);
-	vma_flags_reset_once(vma, &new_vma_flags);
+	vma_overwrite_flags(vma, new_vma_flags);
 	if (vma_wants_manual_pte_write_upgrade(vma))
 		mm_cp_flags |= MM_CP_TRY_CHANGE_WRITABLE;
 	vma_set_page_prot(vma);
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index f449129ea97a..815401b19c57 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -366,6 +366,9 @@ enum {
 
 #define VMA_LOCKED_MASK	mk_vma_flags(VMA_LOCKED_BIT, VMA_LOCKONFAULT_BIT)
 
+/* These flags can be updated atomically via VMA/mmap read lock. */
+#define VMA_ATOMIC_SET_ALLOWED mk_vma_flags(VMA_MAYBE_GUARD_BIT)
+
 #define RLIMIT_STACK		3	/* max stack size */
 #define RLIMIT_MEMLOCK		8	/* max locked-in-memory address space */
 
@@ -895,22 +898,6 @@ static inline void vm_flags_reset(struct vm_area_struct *vma,
 	vm_flags_init(vma, flags);
 }
 
-static inline void vma_flags_reset_once(struct vm_area_struct *vma,
-					vma_flags_t *flags)
-{
-	const unsigned long word = flags->__vma_flags[0];
-
-	/* It is assumed only the first system word must be written once. */
-	vma_flags_overwrite_word_once(&vma->flags, word);
-	/* The remainder can be copied normally. */
-	if (NUM_VMA_FLAG_BITS > BITS_PER_LONG) {
-		unsigned long *dst = &vma->flags.__vma_flags[1];
-		const unsigned long *src = &flags->__vma_flags[1];
-
-		bitmap_copy(dst, src, NUM_VMA_FLAG_BITS - BITS_PER_LONG);
-	}
-}
-
 static inline void vm_flags_set(struct vm_area_struct *vma,
 				vm_flags_t flags)
 {
@@ -1037,27 +1024,24 @@ static __always_inline vma_flags_t vma_flags_diff_pair(const vma_flags_t *flags,
 	return dst;
 }
 
+/*
+ * Determine if flags and flags_other have precisely the same flags set,
+ * excluding atomic VMA flags.
+ */
 static __always_inline bool vma_flags_same_pair(const vma_flags_t *flags,
-						const vma_flags_t *flags_other)
+		const vma_flags_t *flags_other)
 {
-	const unsigned long *bitmap = flags->__vma_flags;
-	const unsigned long *bitmap_other = flags_other->__vma_flags;
-
-	return bitmap_equal(bitmap, bitmap_other, NUM_VMA_FLAG_BITS);
-}
+	vma_flags_t diff = vma_flags_diff_pair(flags, flags_other);
 
-static __always_inline bool vma_flags_same_mask(const vma_flags_t *flags,
-						vma_flags_t flags_other)
-{
-	const unsigned long *bitmap = flags->__vma_flags;
-	const unsigned long *bitmap_other = flags_other.__vma_flags;
+	/*
+	 * Atomic VMA flags may be set concurrently under the VMA read lock, so
+	 * are disregarded for equality testing purposes.
+	 */
+	vma_flags_clear_mask(&diff, VMA_ATOMIC_SET_ALLOWED);
 
-	return bitmap_equal(bitmap, bitmap_other, NUM_VMA_FLAG_BITS);
+	return vma_flags_empty(&diff);
 }
 
-#define vma_flags_same(flags, ...) \
-	vma_flags_same_mask(flags, mk_vma_flags(__VA_ARGS__))
-
 static __always_inline bool vma_test(const struct vm_area_struct *vma,
 		vma_flag_t bit)
 {
@@ -1106,6 +1090,36 @@ static __always_inline void vma_clear_flags_mask(struct vm_area_struct *vma,
 #define vma_clear_flags(vma, ...) \
 	vma_clear_flags_mask(vma, mk_vma_flags(__VA_ARGS__))
 
+/*
+ * Overwrite the VMA's flags with new ones, preserving atomic VMA flags in the
+ * process. The write is done once and once only.
+ */
+static inline void vma_overwrite_flags(struct vm_area_struct *vma,
+				       vma_flags_t new_flags)
+{
+	vma_flags_t atomic;
+
+	vma_assert_write_locked(vma);
+
+	/*
+	 * Preserve the VMA's atomic flags, which are kept stable by the VMA
+	 * write lock.
+	 */
+	atomic = vma_flags_and_mask(&vma->flags, VMA_ATOMIC_SET_ALLOWED);
+	vma_flags_clear_mask(&new_flags, VMA_ATOMIC_SET_ALLOWED);
+	vma_flags_set_mask(&new_flags, atomic);
+
+	/* It is assumed only the first system word must be written once. */
+	vma_flags_overwrite_word_once(&vma->flags, new_flags.__vma_flags[0]);
+	/* The remainder can be copied normally. */
+	if (NUM_VMA_FLAG_BITS > BITS_PER_LONG) {
+		unsigned long *dst = &vma->flags.__vma_flags[1];
+		const unsigned long *src = &new_flags.__vma_flags[1];
+
+		bitmap_copy(dst, src, NUM_VMA_FLAG_BITS - BITS_PER_LONG);
+	}
+}
+
 static __always_inline bool vma_desc_test(const struct vm_area_desc *desc,
 		vma_flag_t bit)
 {
diff --git a/tools/testing/vma/shared.h b/tools/testing/vma/shared.h
index 97cd7a679dc1..646edf367d7c 100644
--- a/tools/testing/vma/shared.h
+++ b/tools/testing/vma/shared.h
@@ -51,17 +51,21 @@
 		      __TO_SCALAR(__val1), __TO_SCALAR(__val2)); \
 	} while (0)
 
-#define ASSERT_FLAGS_SAME_MASK(_flags, _flags_other) \
-	ASSERT_TRUE(vma_flags_same_mask((_flags), (_flags_other)))
+#define ASSERT_FLAGS_SAME_MASK(_flags, _flags_other) do {		\
+	const vma_flags_t __flags_other = (_flags_other);		\
+	ASSERT_TRUE(vma_flags_same_pair((_flags), &__flags_other));	\
+	} while (0)
 
-#define ASSERT_FLAGS_NOT_SAME_MASK(_flags, _flags_other) \
-	ASSERT_FALSE(vma_flags_same_mask((_flags), (_flags_other)))
+#define ASSERT_FLAGS_NOT_SAME_MASK(_flags, _flags_other) do {		\
+	const vma_flags_t __flags_other = (_flags_other);		\
+	ASSERT_FALSE(vma_flags_same_pair((_flags), &__flags_other));	\
+	} while (0)
 
 #define ASSERT_FLAGS_SAME(_flags, ...) \
-	ASSERT_TRUE(vma_flags_same(_flags, __VA_ARGS__))
+	ASSERT_FLAGS_SAME_MASK(_flags, mk_vma_flags(__VA_ARGS__))
 
 #define ASSERT_FLAGS_NOT_SAME(_flags, ...) \
-	ASSERT_FALSE(vma_flags_same(_flags, __VA_ARGS__))
+	ASSERT_FLAGS_NOT_SAME_MASK(_flags, mk_vma_flags(__VA_ARGS__))
 
 #define ASSERT_FLAGS_EMPTY(_flags) \
 	ASSERT_TRUE(vma_flags_empty(_flags))

-- 
2.56.0

[PATCH 2/4] mm/vma: write lock source VMA before duplicating it in __split_vma()

[Lorenzo Stoakes (ARM)]

From: "David Hildenbrand (Arm)" <david@kernel.org>

__split_vma() duplicates the source VMA before taking its VMA write lock,
and until the VMA is write-locked, a concurrent thread might set
VMA_MAYBE_GUARD_BIT, resulting in the duplication missing the flag.

If this happens, then fork may fail to duplicate guard markers and, if a
file-backed mapping, THP collapse may also discard the markers.

Resolve this by taking the VMA write lock before vm_area_dup() and, since
all callers of vm_area_dup() now hold the VMA write lock, assert that this
is the case there, and add a comment to explain why it's needed.

Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
Cc: <stable@vger.kernel.org>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
[ ljs - add VMA write assert in vm_area_dup(), commit message touchup ]
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/vma.c      | 2 +-
 mm/vma_init.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/vma.c b/mm/vma.c
index 1be613044b4a..aa59005e2f42 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -538,6 +538,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
 			return err;
 	}
 
+	vma_start_write(vma);
 	new = vm_area_dup(vma);
 	if (!new)
 		return -ENOMEM;
@@ -568,7 +569,6 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	if (new->vm_ops && new->vm_ops->open)
 		new->vm_ops->open(new);
 
-	vma_start_write(vma);
 	vma_start_write(new);
 
 	init_vma_prep(&vp, vma);
diff --git a/mm/vma_init.c b/mm/vma_init.c
index baa7e82f47e3..d661f229a734 100644
--- a/mm/vma_init.c
+++ b/mm/vma_init.c
@@ -127,6 +127,9 @@ struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig)
 {
 	struct vm_area_struct *new = kmem_cache_alloc(vm_area_cachep, GFP_KERNEL);
 
+	/* Must hold a write lock to stabilise atomic VMA flags. */
+	vma_assert_write_locked(orig);
+
 	if (!new)
 		return NULL;
 

-- 
2.56.0

[PATCH 3/4] mm/madvise: preserve VMA_SOFTDIRTY_BIT when changing anonymous names

[Lorenzo Stoakes (ARM)]

From: "David Hildenbrand (Arm)" <david@kernel.org>

When changing the anonymous VMA name, madvise_update_vma() calls
vma_modify_name() to possibly merge with adjacent VMAs. We don't call
vma_modify_flags() and consequently don't obtain the new VMA flags.

Later in the function we update the VMA flags to the stale new_vma_flags
we obtained before. This can result in us losing the VMA_SOFTDIRTY_BIT
flag set on a VMA we just merged with.

There is no need to update the VMA flags at all, so just exit early. This
skips calling khugepaged_enter_vma(), but since the flags didn't change
this is fine.

Fixes: 9a10064f5625 ("mm: add a field to store names for private anonymous memory")
Cc: <stable@vger.kernel.org>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
[ ljs - Minor edit of commit message, rebased ]
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/madvise.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index b2e00efcaa54..e9fa4f71efea 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -177,6 +177,9 @@ static int madvise_update_vma(vm_flags_t new_flags,
 
 	/* vm_flags is protected by the mmap_lock held in write mode. */
 	vma_start_write(vma);
+	if (set_new_anon_name)
+		return replace_anon_vma_name(vma, anon_name);
+
 	vma_overwrite_flags(vma, new_vma_flags);
 	/*
 	 * If the vma become good for khugepaged to scan,
@@ -186,9 +189,6 @@ static int madvise_update_vma(vm_flags_t new_flags,
 	if (vma_flags_test(&new_vma_flags, VMA_HUGEPAGE_BIT))
 		khugepaged_enter_vma(vma, vma_flags_to_legacy(new_vma_flags));
 
-	if (set_new_anon_name)
-		return replace_anon_vma_name(vma, anon_name);
-
 	return 0;
 }
 

-- 
2.56.0

[PATCH 4/4] mm/vma: avoid losing atomic VMA flags when setting sticky VMA flags

[Lorenzo Stoakes (ARM)]

vma_merge_existing_range() merges an existing VMA (middle) when its
attributes have changed to match those of one or both adjacent VMAs.

Sticky flags are propagated if they are set in any of the merged VMAs. They
are initialised from vmg->vma_flags, which is assumed to contain middle's
sticky flags when the function is invoked.

However, the VMA write lock is not held for middle when they are first
populated, so concurrent updates to atomic VMA flags may occur, which will
be lost upon merge.

This can result in fork failing to propagate guard region markers and (if
file-backed) THP collapse discarding them.

Resolve this by propagating the sticky flags after middle is locked.

Make life easier by factoring this into propagate_sticky_flags(), which
treats vmg->vma_flags as its base (in case any sticky flags are set there),
updating it as sticky flags are accumulated from merged VMAs.

Also update vma_expand() to use propagate_sticky_flags() so all merge paths
utilise it.

propagate_sticky_flags() also asserts that the VMA write lock is held for
the VMA whose sticky flags are being read, with a comment explaining why.

Another means by which atomic VMA flags can fail to be observed is when the
merge is finalised in commit_merge().

rmap locks are dropped in vma_complete() before the sticky flags are
written, meaning an rmap walker can observe the VMA without sticky flags
set, which can also result in retract_page_tables() discarding guard
markers.

Resolve this by assigning the sticky flags before dropping rmap locks,
factored into commit_sticky_flags() with a write assert on the VMA being
written to.

Also remove the vma_set_flags_mask() invocation at the end of
vma_merge_existing_range() as this was introduced by commit
64212ba02e66 ("mm: implement sticky VMA flags") to propagate sticky flags,
which is now redundant with the introduction of commit_sticky_flags().

The vma_set_flags_mask() invocation at the end of vma_expand() setting
sticky flags only is also removed for the same reason.

Also update vma_merge_existing_range()'s comment to reflect the fact that
vmg->vma_flags gets updated.

Finally, update the userland VMA tests to reflect a previously missed
const-ification in vma_assert_write_locked() which would otherwise cause a
warning when building them.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
Fixes: 49e14dabed7a ("mm: set the VM_MAYBE_GUARD flag on guard region install")
Cc: <stable@vger.kernel.org>
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/vma.c                          | 55 +++++++++++++++++++++------------------
 tools/testing/vma/include/dup.h   |  2 +-
 tools/testing/vma/include/stubs.h |  2 +-
 3 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/mm/vma.c b/mm/vma.c
index aa59005e2f42..a65e397d6f62 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -783,6 +783,27 @@ static void vmg_adjust_set_range(struct vma_merge_struct *vmg)
 	}
 }
 
+static void propagate_sticky_flags(struct vma_merge_struct *vmg,
+		const struct vm_area_struct *vma)
+{
+	const vma_flags_t sticky =
+		vma_flags_and_mask(&vma->flags, VMA_STICKY_FLAGS);
+
+	/* Write lock prevents concurrent atomic VMA flag updates. */
+	vma_assert_write_locked(vma);
+	vma_flags_set_mask(&vmg->vma_flags, sticky);
+}
+
+static void commit_sticky_flags(struct vma_merge_struct *vmg)
+{
+	const vma_flags_t sticky =
+		vma_flags_and_mask(&vmg->vma_flags, VMA_STICKY_FLAGS);
+	struct vm_area_struct *target = vmg->target;
+
+	vma_assert_write_locked(target);
+	vma_set_flags_mask(target, sticky);
+}
+
 /*
  * Actually perform the VMA merge operation.
  *
@@ -818,6 +839,8 @@ static int commit_merge(struct vma_merge_struct *vmg)
 		return -ENOMEM;
 
 	vma_prepare(&vp);
+	/* Must be made visible before vma_complete() for rmap walkers. */
+	commit_sticky_flags(vmg);
 	/*
 	 * THP pages may need to do additional splits if we increase
 	 * middle->vm_start.
@@ -857,8 +880,8 @@ static bool can_merge_remove_vma(struct vm_area_struct *vma)
  * merge.
  *
  * As part of this operation, if a merge occurs, the @vmg object will have its
- * vma, start, end, and pgoff fields modified to execute the merge. Subsequent
- * calls to this function should reset these fields.
+ * vma, start, end, pgoff and vma_flags fields modified to execute the merge.
+ * Subsequent calls to this function should reset these fields.
  *
  * Returns: The merged VMA if merge succeeds, or NULL otherwise.
  *
@@ -872,8 +895,6 @@ static bool can_merge_remove_vma(struct vm_area_struct *vma)
 static __must_check struct vm_area_struct *vma_merge_existing_range(
 		struct vma_merge_struct *vmg)
 {
-	vma_flags_t sticky_flags = vma_flags_and_mask(&vmg->vma_flags,
-						      VMA_STICKY_FLAGS);
 	struct vm_area_struct *middle = vmg->middle;
 	struct vm_area_struct *prev = vmg->prev;
 	struct vm_area_struct *next;
@@ -966,24 +987,18 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
 
 	/* No matter what happens, we will be adjusting middle. */
 	vma_start_write(middle);
+	propagate_sticky_flags(vmg, middle);
 
 	if (merge_right) {
-		vma_flags_t next_sticky;
-
 		vma_start_write(next);
 		vmg->target = next;
-		next_sticky = vma_flags_and_mask(&next->flags, VMA_STICKY_FLAGS);
-		vma_flags_set_mask(&sticky_flags, next_sticky);
+		propagate_sticky_flags(vmg, next);
 	}
 
 	if (merge_left) {
-		vma_flags_t prev_sticky;
-
 		vma_start_write(prev);
 		vmg->target = prev;
-
-		prev_sticky = vma_flags_and_mask(&prev->flags, VMA_STICKY_FLAGS);
-		vma_flags_set_mask(&sticky_flags, prev_sticky);
+		propagate_sticky_flags(vmg, prev);
 	}
 
 	if (merge_both) {
@@ -1054,7 +1069,6 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
 	if (err || commit_merge(vmg))
 		goto abort;
 
-	vma_set_flags_mask(vmg->target, sticky_flags);
 	khugepaged_enter_vma(vmg->target, vmg->vm_flags);
 	vmg->state = VMA_MERGE_SUCCESS;
 	return vmg->target;
@@ -1241,16 +1255,11 @@ int vma_expand(struct vma_merge_struct *vmg)
 	struct vm_area_struct *target = vmg->target;
 	struct vm_area_struct *next = vmg->next;
 	bool remove_next = false;
-	vma_flags_t sticky_flags =
-		vma_flags_and_mask(&vmg->vma_flags, VMA_STICKY_FLAGS);
-	vma_flags_t target_sticky;
 	int ret = 0;
 
 	mmap_assert_write_locked(vmg->mm);
 	vma_start_write(target);
 
-	target_sticky = vma_flags_and_mask(&target->flags, VMA_STICKY_FLAGS);
-
 	if (next && target != next && vmg->end == next->vm_end)
 		remove_next = true;
 
@@ -1265,7 +1274,7 @@ int vma_expand(struct vma_merge_struct *vmg)
 	VM_WARN_ON_VMG(target->vm_start < vmg->start ||
 		       target->vm_end > vmg->end, vmg);
 
-	vma_flags_set_mask(&sticky_flags, target_sticky);
+	propagate_sticky_flags(vmg, target);
 
 	/*
 	 * If we are removing the next VMA or copying from a VMA
@@ -1282,18 +1291,14 @@ int vma_expand(struct vma_merge_struct *vmg)
 		return ret;
 
 	if (remove_next) {
-		vma_flags_t next_sticky;
-
 		vma_start_write(next);
 		vmg->__remove_next = true;
 
-		next_sticky = vma_flags_and_mask(&next->flags, VMA_STICKY_FLAGS);
-		vma_flags_set_mask(&sticky_flags, next_sticky);
+		propagate_sticky_flags(vmg, next);
 	}
 	if (commit_merge(vmg))
 		goto nomem;
 
-	vma_set_flags_mask(target, sticky_flags);
 	return 0;
 
 nomem:
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 815401b19c57..c778fe437993 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1212,7 +1212,7 @@ static inline void vma_assert_detached(struct vm_area_struct *vma)
 	WARN_ON_ONCE(vma_is_attached(vma));
 }
 
-static inline void vma_assert_write_locked(struct vm_area_struct *);
+static inline void vma_assert_write_locked(const struct vm_area_struct *);
 static inline void vma_mark_attached(struct vm_area_struct *vma)
 {
 	vma_assert_write_locked(vma);
diff --git a/tools/testing/vma/include/stubs.h b/tools/testing/vma/include/stubs.h
index f786a585caf0..8d030a3ace84 100644
--- a/tools/testing/vma/include/stubs.h
+++ b/tools/testing/vma/include/stubs.h
@@ -320,7 +320,7 @@ static inline void anon_vma_lock_write(struct anon_vma *anon_vma)
 {
 }
 
-static inline void vma_assert_write_locked(struct vm_area_struct *vma)
+static inline void vma_assert_write_locked(const struct vm_area_struct *vma)
 {
 }
 

-- 
2.56.0

Re: [PATCH 0/4] mm: avoid losing atomic VMA flags

[Andrew Morton]

On Fri, 23 Oct 2026 14:47:26 +0100 "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:

> Since commit 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove
> under VMA lock"), VMA_MAYBE_GUARD_BIT can be set atomically under the VMA
> read lock.
> 
> However, a number of places wrongly assume the flags are stable under the
> read lock, which can cause stale atomic VMA flags to overwrite existing
> valid ones.
> 
> Additionally, one case in which VMA_SOFTDIRTY_BIT was overwritten is also
> addressed.
> 
> This series addresses each of these, while keeping the logic inline with
> existing conventions.

Added to mm-unstable thanks.

> Each patch in this series is a hotfix.

The fixes continue to pour in.  Right now I'm adding everything to the
tail of mm-unstable while patiently for Linus to merge our stuff.

Once that happens I'll assemble the second-week merge-window material
into mm-stable and I'll spill the recently-arriving cc:stable material
into mm-hotfixes.

I don't think it matters whether cc:stable material is upstreamed in
7.4-rc1 or in 7.4-rc3 - rather than rushing it in I'd prefer to give it
a week or two baking time.

Re: [PATCH 2/4] mm/vma: write lock source VMA before duplicating it in __split_vma()

[Davidlohr Bueso]

On Fri, 23 Oct 2026, Lorenzo Stoakes (ARM) wrote:

>From: "David Hildenbrand (Arm)" <david@kernel.org>
>
>__split_vma() duplicates the source VMA before taking its VMA write lock,
>and until the VMA is write-locked, a concurrent thread might set
>VMA_MAYBE_GUARD_BIT, resulting in the duplication missing the flag.
>
>If this happens, then fork may fail to duplicate guard markers and, if a
>file-backed mapping, THP collapse may also discard the markers.
>
>Resolve this by taking the VMA write lock before vm_area_dup() and, since
>all callers of vm_area_dup() now hold the VMA write lock, assert that this
>is the case there, and add a comment to explain why it's needed.
>
>Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
>Cc: <stable@vger.kernel.org>
>Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>[ ljs - add VMA write assert in vm_area_dup(), commit message touchup ]
>Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

... with one nit below.

>---
> mm/vma.c      | 2 +-
> mm/vma_init.c | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/mm/vma.c b/mm/vma.c
>index 1be613044b4a..aa59005e2f42 100644
>--- a/mm/vma.c
>+++ b/mm/vma.c
>@@ -538,6 +538,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> 			return err;
> 	}
>
>+	vma_start_write(vma);
> 	new = vm_area_dup(vma);
> 	if (!new)
> 		return -ENOMEM;
>@@ -568,7 +569,6 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> 	if (new->vm_ops && new->vm_ops->open)
> 		new->vm_ops->open(new);
>
>-	vma_start_write(vma);
> 	vma_start_write(new);
>
> 	init_vma_prep(&vp, vma);
>diff --git a/mm/vma_init.c b/mm/vma_init.c
>index baa7e82f47e3..d661f229a734 100644
>--- a/mm/vma_init.c
>+++ b/mm/vma_init.c
>@@ -127,6 +127,9 @@ struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig)
> {
> 	struct vm_area_struct *new = kmem_cache_alloc(vm_area_cachep, GFP_KERNEL);
>
>+	/* Must hold a write lock to stabilise atomic VMA flags. */

should just say "stabilize atomic VMA flags".

>+	vma_assert_write_locked(orig);
>+
> 	if (!new)
> 		return NULL;
>
>
>-- 
>2.56.0
>

Re: [PATCH 2/4] mm/vma: write lock source VMA before duplicating it in __split_vma()

[David Hildenbrand (Arm)]

On 10/23/26 21:26, Davidlohr Bueso wrote:
> On Fri, 23 Oct 2026, Lorenzo Stoakes (ARM) wrote:
> 
>> From: "David Hildenbrand (Arm)" <david@kernel.org>
>>
>> __split_vma() duplicates the source VMA before taking its VMA write lock,
>> and until the VMA is write-locked, a concurrent thread might set
>> VMA_MAYBE_GUARD_BIT, resulting in the duplication missing the flag.
>>
>> If this happens, then fork may fail to duplicate guard markers and, if a
>> file-backed mapping, THP collapse may also discard the markers.
>>
>> Resolve this by taking the VMA write lock before vm_area_dup() and, since
>> all callers of vm_area_dup() now hold the VMA write lock, assert that this
>> is the case there, and add a comment to explain why it's needed.
>>
>> Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA
>> lock")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>> [ ljs - add VMA write assert in vm_area_dup(), commit message touchup ]
>> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> 
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> 

Thanks.

> ... with one nit below.
> 
>> ---
>> mm/vma.c      | 2 +-
>> mm/vma_init.c | 3 +++
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/vma.c b/mm/vma.c
>> index 1be613044b4a..aa59005e2f42 100644
>> --- a/mm/vma.c
>> +++ b/mm/vma.c
>> @@ -538,6 +538,7 @@ __split_vma(struct vma_iterator *vmi, struct
>> vm_area_struct *vma,
>>             return err;
>>     }
>>
>> +    vma_start_write(vma);
>>     new = vm_area_dup(vma);
>>     if (!new)
>>         return -ENOMEM;
>> @@ -568,7 +569,6 @@ __split_vma(struct vma_iterator *vmi, struct
>> vm_area_struct *vma,
>>     if (new->vm_ops && new->vm_ops->open)
>>         new->vm_ops->open(new);
>>
>> -    vma_start_write(vma);
>>     vma_start_write(new);
>>
>>     init_vma_prep(&vp, vma);
>> diff --git a/mm/vma_init.c b/mm/vma_init.c
>> index baa7e82f47e3..d661f229a734 100644
>> --- a/mm/vma_init.c
>> +++ b/mm/vma_init.c
>> @@ -127,6 +127,9 @@ struct vm_area_struct *vm_area_dup(struct vm_area_struct
>> *orig)
>> {
>>     struct vm_area_struct *new = kmem_cache_alloc(vm_area_cachep, GFP_KERNEL);
>>
>> +    /* Must hold a write lock to stabilise atomic VMA flags. */
> 
> should just say "stabilize atomic VMA flags".

You mean the s/stabilise/stabilize/ ?

I guess that's just the (uncommon in the kernel) UK english variant Lorenzo
tends do use.

-- 
Cheers,

David

Re: [PATCH 2/4] mm/vma: write lock source VMA before duplicating it in __split_vma()

[Lorenzo Stoakes (ARM)]

On Mon, Oct 26, 2026 at 11:42:40AM +0100, David Hildenbrand (Arm) wrote:
> > should just say "stabilize atomic VMA flags".
>
> You mean the s/stabilise/stabilize/ ?
>
> I guess that's just the (uncommon in the kernel) UK english variant Lorenzo
> tends do use.

King Charles dictates that this form be used ;)

--
Cheers, Lorenzo

Re: [PATCH 1/4] mm: do not unintentionally overwrite atomic VMA flags

[David Hildenbrand (Arm)]

On 10/23/26 15:47, Lorenzo Stoakes (ARM) wrote:
> madvise, mlock and mprotect take a copy of a VMA's flags holding only the
> VMA read lock, later writing it back under the write lock (in
> madvise_update_vma(), mlock_fixup(), mlock_vma_pages_range() and
> mprotect_fixup()).
> 
> This was done on the assumption that VMA flags cannot change with a read
> lock held.
> 
> However this changed in commit 2ab7f1bbafc9 ("mm/madvise: allow guard page
> install/remove under VMA lock"), which allowed VM_ATOMIC_SET_ALLOWED
> flags (currently only VMA_MAYBE_GUARD_BIT) to be written concurrently under
> a VMA read lock.
> 
> The actual write itself is safe as this is done under the VMA write lock
> at which point no further concurrent writes can occur, however at this
> point the stale flags mean that these callers can unintentionally overwrite
> atomic flags incorrectly.
> 
> The consequences of this are significant - a missing VMA_MAYBE_GUARD_BIT
> could result in page tables not being copied on fork (the vma_needs_copy()
> check can return false) or being overwritten on file-backed THP
> collapse (the file_backed_vma_is_retractable() can return true).
> 
> Resolve the issue by always taking care to retain existing atomic flags
> when overwriting VMA flags, replacing the inconsistently named
> vma_flags_reset_once() with vma_overwrite_flags() to do so, and always
> invoking it when flags are overwritten like this.
> 
> Reads are a problem too - madvise, mlock and mprotect all compare VMA flags
> without a VMA write lock held via vma_flags_same_pair().
> 
> This has more serious impact, as concurrent changes to atomic VMA flags can
> cause two sets of VMA flags to be wrongly considered different.
> 
> For mprotect and madvise this causes a useless split, but mlock is impacted
> more seriously as it can result in mm->locked_vm underflow.
> 
> Fix this by updating vma_flags_same_pair() to disregard
> VM_ATOMIC_SET_ALLOWED VMAs - VMA equality is the same as VMA mergeability,
> and atomic VMA flags are disregarded when comparing VMA flags for merge -
> so this is justified semantically.
> 
> Additionally, remove the redundant vma_flags_same_mask() and the unused
> vma_flags_same() as part of the change, and replace VM_ATOMIC_SET_ALLOWED
> with VMA_ATOMIC_SET_ALLOWED, using the new VMA flag API.
> 
> Finally, update userland VMA tests to reflect the changes.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---

Nothing jumped at me

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

-- 
Cheers,

David

Re: [PATCH 4/4] mm/vma: avoid losing atomic VMA flags when setting sticky VMA flags

[David Hildenbrand (Arm)]

On 10/23/26 15:47, Lorenzo Stoakes (ARM) wrote:
> vma_merge_existing_range() merges an existing VMA (middle) when its
> attributes have changed to match those of one or both adjacent VMAs.
> 
> Sticky flags are propagated if they are set in any of the merged VMAs. They
> are initialised from vmg->vma_flags, which is assumed to contain middle's
> sticky flags when the function is invoked.
> 
> However, the VMA write lock is not held for middle when they are first
> populated, so concurrent updates to atomic VMA flags may occur, which will
> be lost upon merge.
> 
> This can result in fork failing to propagate guard region markers and (if
> file-backed) THP collapse discarding them.
> 
> Resolve this by propagating the sticky flags after middle is locked.
> 
> Make life easier by factoring this into propagate_sticky_flags(), which
> treats vmg->vma_flags as its base (in case any sticky flags are set there),
> updating it as sticky flags are accumulated from merged VMAs.
> 
> Also update vma_expand() to use propagate_sticky_flags() so all merge paths
> utilise it.
> 
> propagate_sticky_flags() also asserts that the VMA write lock is held for
> the VMA whose sticky flags are being read, with a comment explaining why.
> 
> Another means by which atomic VMA flags can fail to be observed is when the
> merge is finalised in commit_merge().
> 
> rmap locks are dropped in vma_complete() before the sticky flags are
> written, meaning an rmap walker can observe the VMA without sticky flags
> set, which can also result in retract_page_tables() discarding guard
> markers.
> 
> Resolve this by assigning the sticky flags before dropping rmap locks,
> factored into commit_sticky_flags() with a write assert on the VMA being
> written to.
> 
> Also remove the vma_set_flags_mask() invocation at the end of
> vma_merge_existing_range() as this was introduced by commit
> 64212ba02e66 ("mm: implement sticky VMA flags") to propagate sticky flags,
> which is now redundant with the introduction of commit_sticky_flags().
> 
> The vma_set_flags_mask() invocation at the end of vma_expand() setting
> sticky flags only is also removed for the same reason.
> 
> Also update vma_merge_existing_range()'s comment to reflect the fact that
> vmg->vma_flags gets updated.
> 
> Finally, update the userland VMA tests to reflect a previously missed
> const-ification in vma_assert_write_locked() which would otherwise cause a
> warning when building them.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
> Fixes: 49e14dabed7a ("mm: set the VM_MAYBE_GUARD flag on guard region install")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---

Nothing jumped at me

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

-- 
Cheers,

David

Re: [PATCH 1/4] mm: do not unintentionally overwrite atomic VMA flags

[Pedro Falcato]

On Fri, Oct 23, 2026 at 02:47:27PM +0100, Lorenzo Stoakes (ARM) wrote:
> madvise, mlock and mprotect take a copy of a VMA's flags holding only the
> VMA read lock, later writing it back under the write lock (in
> madvise_update_vma(), mlock_fixup(), mlock_vma_pages_range() and
> mprotect_fixup()).
> 
> This was done on the assumption that VMA flags cannot change with a read
> lock held.
> 
> However this changed in commit 2ab7f1bbafc9 ("mm/madvise: allow guard page
> install/remove under VMA lock"), which allowed VM_ATOMIC_SET_ALLOWED
> flags (currently only VMA_MAYBE_GUARD_BIT) to be written concurrently under
> a VMA read lock.
> 
> The actual write itself is safe as this is done under the VMA write lock
> at which point no further concurrent writes can occur, however at this
> point the stale flags mean that these callers can unintentionally overwrite
> atomic flags incorrectly.
> 
> The consequences of this are significant - a missing VMA_MAYBE_GUARD_BIT
> could result in page tables not being copied on fork (the vma_needs_copy()
> check can return false) or being overwritten on file-backed THP
> collapse (the file_backed_vma_is_retractable() can return true).
> 
> Resolve the issue by always taking care to retain existing atomic flags
> when overwriting VMA flags, replacing the inconsistently named
> vma_flags_reset_once() with vma_overwrite_flags() to do so, and always
> invoking it when flags are overwritten like this.
> 
> Reads are a problem too - madvise, mlock and mprotect all compare VMA flags
> without a VMA write lock held via vma_flags_same_pair().
> 
> This has more serious impact, as concurrent changes to atomic VMA flags can
> cause two sets of VMA flags to be wrongly considered different.

I really wish we had a new field for atomic flags :/

> 
> For mprotect and madvise this causes a useless split, but mlock is impacted
> more seriously as it can result in mm->locked_vm underflow.
> 
> Fix this by updating vma_flags_same_pair() to disregard
> VM_ATOMIC_SET_ALLOWED VMAs - VMA equality is the same as VMA mergeability,
> and atomic VMA flags are disregarded when comparing VMA flags for merge -
> so this is justified semantically.
> 
> Additionally, remove the redundant vma_flags_same_mask() and the unused
> vma_flags_same() as part of the change, and replace VM_ATOMIC_SET_ALLOWED
> with VMA_ATOMIC_SET_ALLOWED, using the new VMA flag API.
> 
> Finally, update userland VMA tests to reflect the changes.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Anyway, LGTM

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

-- 
Pedro

Re: [PATCH 2/4] mm/vma: write lock source VMA before duplicating it in __split_vma()

[Pedro Falcato]

On Fri, Oct 23, 2026 at 02:47:28PM +0100, Lorenzo Stoakes (ARM) wrote:
> From: "David Hildenbrand (Arm)" <david@kernel.org>
> 
> __split_vma() duplicates the source VMA before taking its VMA write lock,
> and until the VMA is write-locked, a concurrent thread might set
> VMA_MAYBE_GUARD_BIT, resulting in the duplication missing the flag.
> 
> If this happens, then fork may fail to duplicate guard markers and, if a
> file-backed mapping, THP collapse may also discard the markers.
> 
> Resolve this by taking the VMA write lock before vm_area_dup() and, since
> all callers of vm_area_dup() now hold the VMA write lock, assert that this
> is the case there, and add a comment to explain why it's needed.
> 
> Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> [ ljs - add VMA write assert in vm_area_dup(), commit message touchup ]
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Reviewed-by: Pedro Falcato <pfalcato@suse.de> 

-- 
Pedro

Re: [PATCH 3/4] mm/madvise: preserve VMA_SOFTDIRTY_BIT when changing anonymous names

[Pedro Falcato]

On Fri, Oct 23, 2026 at 02:47:29PM +0100, Lorenzo Stoakes (ARM) wrote:
> From: "David Hildenbrand (Arm)" <david@kernel.org>
> 
> When changing the anonymous VMA name, madvise_update_vma() calls
> vma_modify_name() to possibly merge with adjacent VMAs. We don't call
> vma_modify_flags() and consequently don't obtain the new VMA flags.
> 
> Later in the function we update the VMA flags to the stale new_vma_flags
> we obtained before. This can result in us losing the VMA_SOFTDIRTY_BIT
> flag set on a VMA we just merged with.
> 
> There is no need to update the VMA flags at all, so just exit early. This
> skips calling khugepaged_enter_vma(), but since the flags didn't change
> this is fine.
> 
> Fixes: 9a10064f5625 ("mm: add a field to store names for private anonymous memory")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> [ ljs - Minor edit of commit message, rebased ]
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

> ---
>  mm/madvise.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index b2e00efcaa54..e9fa4f71efea 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -177,6 +177,9 @@ static int madvise_update_vma(vm_flags_t new_flags,
>  
>  	/* vm_flags is protected by the mmap_lock held in write mode. */
>  	vma_start_write(vma);
> +	if (set_new_anon_name)
> +		return replace_anon_vma_name(vma, anon_name);

This looks non-obvious and brittle, maybe have a comment here?
Or some more clever solution.

-- 
Pedro

Re: [PATCH 4/4] mm/vma: avoid losing atomic VMA flags when setting sticky VMA flags

[Pedro Falcato]

On Fri, Oct 23, 2026 at 02:47:30PM +0100, Lorenzo Stoakes (ARM) wrote:
> vma_merge_existing_range() merges an existing VMA (middle) when its
> attributes have changed to match those of one or both adjacent VMAs.
> 
> Sticky flags are propagated if they are set in any of the merged VMAs. They
> are initialised from vmg->vma_flags, which is assumed to contain middle's
> sticky flags when the function is invoked.
> 
> However, the VMA write lock is not held for middle when they are first
> populated, so concurrent updates to atomic VMA flags may occur, which will
> be lost upon merge.
> 
> This can result in fork failing to propagate guard region markers and (if
> file-backed) THP collapse discarding them.
> 
> Resolve this by propagating the sticky flags after middle is locked.
> 
> Make life easier by factoring this into propagate_sticky_flags(), which
> treats vmg->vma_flags as its base (in case any sticky flags are set there),
> updating it as sticky flags are accumulated from merged VMAs.
> 
> Also update vma_expand() to use propagate_sticky_flags() so all merge paths
> utilise it.
> 
> propagate_sticky_flags() also asserts that the VMA write lock is held for
> the VMA whose sticky flags are being read, with a comment explaining why.
> 
> Another means by which atomic VMA flags can fail to be observed is when the
> merge is finalised in commit_merge().
> 
> rmap locks are dropped in vma_complete() before the sticky flags are
> written, meaning an rmap walker can observe the VMA without sticky flags
> set, which can also result in retract_page_tables() discarding guard
> markers.
> 
> Resolve this by assigning the sticky flags before dropping rmap locks,
> factored into commit_sticky_flags() with a write assert on the VMA being
> written to.
> 
> Also remove the vma_set_flags_mask() invocation at the end of
> vma_merge_existing_range() as this was introduced by commit
> 64212ba02e66 ("mm: implement sticky VMA flags") to propagate sticky flags,
> which is now redundant with the introduction of commit_sticky_flags().
> 
> The vma_set_flags_mask() invocation at the end of vma_expand() setting
> sticky flags only is also removed for the same reason.
> 
> Also update vma_merge_existing_range()'s comment to reflect the fact that
> vmg->vma_flags gets updated.
> 
> Finally, update the userland VMA tests to reflect a previously missed
> const-ification in vma_assert_write_locked() which would otherwise cause a
> warning when building them.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Fixes: 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove under VMA lock")
> Fixes: 49e14dabed7a ("mm: set the VM_MAYBE_GUARD flag on guard region install")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Reviewed-by: Pedro Falcato <pfalcato@suse.de> 

-- 
Pedro

Re: [PATCH 0/4] mm: avoid losing atomic VMA flags

[Gregory Price (Meta)]

On 2026-10-23 14:47 +0100, Lorenzo Stoakes (ARM) wrote:
> Since commit 2ab7f1bbafc9 ("mm/madvise: allow guard page install/remove
> under VMA lock"), VMA_MAYBE_GUARD_BIT can be set atomically under the VMA
> read lock.
> 
> However, a number of places wrongly assume the flags are stable under the
> read lock, which can cause stale atomic VMA flags to overwrite existing
> valid ones.
> 
> Additionally, one case in which VMA_SOFTDIRTY_BIT was overwritten is also
> addressed.
> 
> This series addresses each of these, while keeping the logic inline with
> existing conventions.
> 
> Each patch in this series is a hotfix.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

You too get to be the unwilling participant in my schlopptesting
experiment - but the good news is there's no issues and you win
tags and free self/unit tests.

Tested-by: Gregory Price (Meta) <gourry@gourry.net>

Might be worth taking the VMA unit test below.
Selftests... maybe the non-race uffd one.

Summary

- **Reproduced and validated.**
  Every bug the series describes reproduces on the base.
  After applying the patches, the reproducers fail to repro.

-- >8 --
From: Gregory Price (Meta) <gourry@gourry.net>
Subject: [PATCH] selftests/mm: guard-regions: test VM_MAYBE_GUARD is not lost under races

MADV_GUARD_INSTALL sets VM_MAYBE_GUARD atomically while holding only the
VMA read lock. mprotect(), madvise(), mlock(), VMA split and VMA merge
used to snapshot VMA flags under the mmap write lock before write-locking
the VMA and write them back afterwards, dropping a concurrently set
VM_MAYBE_GUARD. For a file-backed mapping that means fork() no longer
copies the page tables, so the child silently loses its guard markers.

Add a flag_race test which, for each of those operations, repeatedly
maps a fresh region and races a guard installation against the operation
from two threads, then asserts via smaps that the VMA covering the
guarded page still has the "gu" flag whenever the installation
succeeded.

Without "mm: avoid losing atomic VMA flags" this fails within a few
rounds for mprotect() (all three backings); with only patch 1 applied it
fails on the split case; with patches 1-3 it fails on the merge case;
with the whole series it passes 2000 rounds of each operation.

Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 tools/testing/selftests/mm/guard-regions.c | 158 +++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/tools/testing/selftests/mm/guard-regions.c b/tools/testing/selftests/mm/guard-regions.c
index f7d53ea3c252..fe03354a1dca 100644
--- a/tools/testing/selftests/mm/guard-regions.c
+++ b/tools/testing/selftests/mm/guard-regions.c
@@ -9,8 +9,10 @@
 #include <linux/limits.h>
 #include <linux/userfaultfd.h>
 #include <linux/fs.h>
+#include <pthread.h>
 #include <setjmp.h>
 #include <signal.h>
+#include <stdatomic.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -2330,4 +2332,160 @@ TEST_F(guard_regions, smaps)
 	ASSERT_TRUE(check_vmflag_guard(ptr));
 }
 
+/*
+ * Operations which snapshot VMA flags under the mmap write lock but before
+ * write-locking the VMA, racing against MADV_GUARD_INSTALL which sets
+ * VM_MAYBE_GUARD atomically under the VMA read lock.
+ */
+enum guard_flag_race_op {
+	RACE_MPROTECT,	/* mprotect() whole VMA. */
+	RACE_MADVISE,	/* madvise(MADV_RANDOM) whole VMA. */
+	RACE_MLOCK,	/* mlock() whole VMA. */
+	RACE_SPLIT,	/* mprotect() 2nd page, guard 1st (split duplicates). */
+	RACE_MERGE,	/* mprotect() 2nd page so it merges with the 1st. */
+	NR_RACE_OPS,
+};
+
+struct guard_flag_race {
+	unsigned long page_size;
+	enum guard_flag_race_op op;
+	char *ptr;
+	char *guard_ptr;
+	int guard_ret;
+	unsigned int jitter_guard, jitter_op;
+	atomic_int round, guard_done, op_done, stop;
+};
+
+static void race_spin(unsigned int n)
+{
+	for (volatile unsigned int i = 0; i < n; i++)
+		;
+}
+
+static int race_wait(struct guard_flag_race *race, int seen)
+{
+	int curr;
+
+	while ((curr = atomic_load(&race->round)) == seen)
+		if (atomic_load(&race->stop))
+			return -1;
+	return curr;
+}
+
+static void *race_guard_thread(void *arg)
+{
+	struct guard_flag_race *race = arg;
+	int seen = 0;
+
+	while ((seen = race_wait(race, seen)) >= 0) {
+		race_spin(race->jitter_guard);
+		race->guard_ret = madvise(race->guard_ptr, race->page_size,
+					  MADV_GUARD_INSTALL);
+		atomic_store(&race->guard_done, seen);
+	}
+	return NULL;
+}
+
+static void *race_op_thread(void *arg)
+{
+	struct guard_flag_race *race = arg;
+	const unsigned long page_size = race->page_size;
+	int seen = 0;
+
+	while ((seen = race_wait(race, seen)) >= 0) {
+		race_spin(race->jitter_op);
+		switch (race->op) {
+		case RACE_MPROTECT:
+			mprotect(race->ptr, 2 * page_size, PROT_READ);
+			break;
+		case RACE_MADVISE:
+			madvise(race->ptr, 2 * page_size, MADV_RANDOM);
+			break;
+		case RACE_MLOCK:
+			mlock(race->ptr, 2 * page_size);
+			break;
+		case RACE_SPLIT:
+		case RACE_MERGE:
+			mprotect(&race->ptr[page_size], page_size, PROT_READ);
+			break;
+		default:
+			break;
+		}
+		atomic_store(&race->op_done, seen);
+	}
+	return NULL;
+}
+
+/*
+ * Assert that VM_MAYBE_GUARD is never lost when guard regions are installed
+ * concurrently with operations that update VMA flags, split or merge VMAs.
+ *
+ * Losing it means fork() may not copy the page tables of a file-backed
+ * mapping, silently dropping the guard markers in the child.
+ */
+TEST_F(guard_regions, flag_race)
+{
+	const unsigned long page_size = self->page_size;
+	struct guard_flag_race race = { .page_size = page_size };
+	pthread_t guard_thread, op_thread;
+	struct procmap_fd procmap;
+	int round = 0;
+
+	srand(getpid());
+	ASSERT_EQ(open_self_procmap(&procmap), 0);
+	ASSERT_EQ(pthread_create(&guard_thread, NULL, race_guard_thread, &race), 0);
+	ASSERT_EQ(pthread_create(&op_thread, NULL, race_op_thread, &race), 0);
+
+	for (race.op = 0; race.op < NR_RACE_OPS; race.op++) {
+		int i;
+
+		for (i = 0; i < 2000; i++) {
+			char *ptr;
+
+			ptr = mmap_(self, variant, NULL, 2 * page_size,
+				    PROT_READ | PROT_WRITE, 0, 0);
+			ASSERT_NE(ptr, MAP_FAILED);
+			/*
+			 * Populate so anonymous mappings have an anon_vma and
+			 * can be guarded under the VMA lock.
+			 */
+			ptr[0] = 'x';
+			ptr[page_size] = 'x';
+
+			race.ptr = ptr;
+			race.guard_ptr = &ptr[page_size];
+			if (race.op == RACE_SPLIT) {
+				race.guard_ptr = ptr;
+			} else if (race.op == RACE_MERGE) {
+				/* Make 1st page RO so 2nd merges into it. */
+				ASSERT_EQ(mprotect(ptr, page_size, PROT_READ), 0);
+			}
+			race.jitter_guard = rand() % 2000;
+			race.jitter_op = rand() % 2000;
+
+			atomic_store(&race.round, ++round);
+			while (atomic_load(&race.guard_done) != round ||
+			       atomic_load(&race.op_done) != round)
+				;
+
+			/* smaps is keyed by VMA start, so look it up. */
+			ASSERT_TRUE(find_vma_procmap(&procmap, race.guard_ptr));
+			if (race.guard_ret == 0 &&
+			    !check_vmflag_guard((void *)procmap.query.vma_start)) {
+				TH_LOG("VM_MAYBE_GUARD lost, op %d round %d",
+				       race.op, i);
+				atomic_store(&race.stop, 1);
+				ASSERT_TRUE(false);
+			}
+
+			ASSERT_EQ(munmap(ptr, 2 * page_size), 0);
+		}
+	}
+
+	atomic_store(&race.stop, 1);
+	pthread_join(guard_thread, NULL);
+	pthread_join(op_thread, NULL);
+	ASSERT_EQ(close_procmap(&procmap), 0);
+}
+
 TEST_HARNESS_MAIN

-- >8 --
From: Gregory Price (Meta) <gourry@gourry.net>
Subject: [PATCH] selftests/mm: soft-dirty: check VM_SOFTDIRTY survives an anon name merge

test_merge() checks that VM_SOFTDIRTY is propagated when a soft-dirty
clean VMA is merged into a soft-dirty one via mremap() and mprotect(),
but not via a change of anonymous VMA name.

That path was broken: madvise_update_vma() overwrote the merged VMA's
flags with the stale flags of the renamed VMA, dropping VM_SOFTDIRTY from
the VMA it merged with. This is fixed by "mm/madvise: preserve
VMA_SOFTDIRTY_BIT when changing anonymous names".

Add a case which names VMA 1, clears soft-dirty, places a differently
named (and therefore soft-dirty, unmerged) VMA 2 adjacent to it, then
renames VMA 1 so that it merges into VMA 2, and asserts both pages are
still reported soft-dirty. Skip it if PR_SET_VMA_ANON_NAME is not
supported.

Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 tools/testing/selftests/mm/soft-dirty.c | 76 ++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/soft-dirty.c b/tools/testing/selftests/mm/soft-dirty.c
index 7f649b673355..9f4660b19e1a 100644
--- a/tools/testing/selftests/mm/soft-dirty.c
+++ b/tools/testing/selftests/mm/soft-dirty.c
@@ -6,6 +6,7 @@
 #include <stdint.h>
 #include <malloc.h>
 #include <sys/mman.h>
+#include <sys/prctl.h>
 
 #include "kselftest.h"
 #include "vm_util.h"
@@ -13,6 +14,17 @@
 #define PAGEMAP_FILE_PATH "/proc/self/pagemap"
 #define TEST_ITERATIONS 10000
 
+#ifndef PR_SET_VMA
+#define PR_SET_VMA		0x53564d41
+#define PR_SET_VMA_ANON_NAME	0
+#endif
+
+static int set_anon_name(char *addr, size_t len, const char *name)
+{
+	return prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, (unsigned long)addr,
+		     len, (unsigned long)name);
+}
+
 static void test_simple(int pagemap_fd, int pagesize)
 {
 	int i;
@@ -308,6 +320,68 @@ static void test_merge(int pagemap_fd, int pagesize)
 			 __func__);
 
 	munmap(map, 2 * pagesize);
+
+	/*
+	 * Finally, establish a named VMA and clear VM_SOFTDIRTY:
+	 *
+	 *      "a"
+	 *       -
+	 * |---|---|---|---|---|
+	 * |   | 1 |   |   |   |
+	 * |---|---|---|---|---|
+	 */
+	map = mmap(&reserved[pagesize], pagesize, PROT_READ | PROT_WRITE,
+		   MAP_ANON | MAP_PRIVATE | MAP_FIXED, -1, 0);
+	if (map == MAP_FAILED)
+		ksft_exit_fail_msg("mmap failed\n");
+	if (set_anon_name(map, pagesize, "a")) {
+		ksft_test_result_skip("Test %s-anon soft-dirty after anon name merge 1st pg\n",
+				      __func__);
+		ksft_test_result_skip("Test %s-anon soft-dirty after anon name merge 2nd pg\n",
+				      __func__);
+		munmap(map, pagesize);
+		return;
+	}
+	clear_softdirty();
+
+	/*
+	 * ...and place a differently named VMA immediately adjacent:
+	 *
+	 *      "a" "b"
+	 *       -  S/D
+	 * |---|---|---|---|---|
+	 * |   | 1 | 2 |   |   |
+	 * |---|---|---|---|---|
+	 */
+	map2 = mmap(&reserved[2 * pagesize], pagesize, PROT_READ | PROT_WRITE,
+		    MAP_ANON | MAP_PRIVATE | MAP_FIXED, -1, 0);
+	if (map2 == MAP_FAILED)
+		ksft_exit_fail_msg("mmap failed\n");
+	if (set_anon_name(map2, pagesize, "b"))
+		ksft_exit_fail_msg("prctl(PR_SET_VMA_ANON_NAME) failed\n");
+
+	/*
+	 * Now rename VMA 1 so it's compatible with 2 and therefore merges. The
+	 * anon name path must not overwrite the merged VMA's flags with VMA 1's
+	 * stale ones, which lack VM_SOFTDIRTY:
+	 *
+	 *       merge
+	 *        S/D
+	 * |---|-------|---|---|
+	 * |   |   1   |   |   |
+	 * |---|-------|---|---|
+	 */
+	if (set_anon_name(map, pagesize, "b"))
+		ksft_exit_fail_msg("prctl(PR_SET_VMA_ANON_NAME) failed\n");
+
+	ksft_test_result(pagemap_is_softdirty(pagemap_fd, map) == 1,
+			 "Test %s-anon soft-dirty after anon name merge 1st pg\n",
+			 __func__);
+	ksft_test_result(pagemap_is_softdirty(pagemap_fd, map2) == 1,
+			 "Test %s-anon soft-dirty after anon name merge 2nd pg\n",
+			 __func__);
+
+	munmap(map, 2 * pagesize);
 }
 
 static void test_mprotect_anon(int pagemap_fd, int pagesize)
@@ -330,7 +404,7 @@ int main(int argc, char **argv)
 	if (!softdirty_supported())
 		ksft_exit_skip("soft-dirty is not support\n");
 
-	ksft_set_plan(19);
+	ksft_set_plan(21);
 	pagemap_fd = open(PAGEMAP_FILE_PATH, O_RDONLY);
 	if (pagemap_fd < 0)
 		ksft_exit_fail_msg("Failed to open %s\n", PAGEMAP_FILE_PATH);

-- >8 --
From: Gregory Price (Meta) <gourry@gourry.net>
Subject: [PATCH] tools/testing/vma: test merge retains sticky flags set after vmg snapshot

Callers of vma_merge_existing_range() populate vmg->vma_flags from the
middle VMA before it is write-locked. VMA_MAYBE_GUARD_BIT can be set
atomically on middle after that point (MADV_GUARD_INSTALL under the VMA
read lock), so a merge which takes middle's sticky flags from
vmg->vma_flags rather than from middle itself loses it. That is the case
fixed by "mm/vma: avoid losing atomic VMA flags when setting sticky VMA
flags".

The existing merge_existing test always uses the same flags for middle
and for vmg, so it cannot observe this. Add a test which sets
VMA_MAYBE_GUARD_BIT on middle only, leaving it clear in vmg->vma_flags,
prev and next, and asserts that the merged VMA retains it for each merge
shape: merge right (partial and full span), merge left (partial and full
span) and merge both.

Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 tools/testing/vma/tests/merge.c | 92 +++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/tools/testing/vma/tests/merge.c b/tools/testing/vma/tests/merge.c
index b26f1a66a170..3ebd4ca50904 100644
--- a/tools/testing/vma/tests/merge.c
+++ b/tools/testing/vma/tests/merge.c
@@ -1096,6 +1096,97 @@ static bool test_merge_existing(void)
 	return true;
 }
 
+struct stale_sticky_shape {
+	const char *name;
+	bool has_prev, has_next;
+	unsigned long start, end;
+	bool expect_prev;
+};
+
+/*
+ * Callers of vma_merge_existing_range() (e.g. mprotect, madvise, mlock) take a
+ * snapshot of middle's flags into vmg->vma_flags before middle is write-locked.
+ * Atomic VMA flags (currently VMA_MAYBE_GUARD_BIT) can still be set on middle
+ * after that snapshot, under the VMA read lock (MADV_GUARD_INSTALL), so the
+ * merge must take middle's sticky flags from middle once it is write-locked,
+ * not from the stale vmg->vma_flags, or the merged VMA loses them.
+ *
+ * Simulate that race by setting VMA_MAYBE_GUARD_BIT on middle only, so that
+ * neither vmg->vma_flags nor prev/next have it, and assert that the merged
+ * VMA retains it for each merge shape.
+ */
+static bool test_merge_existing_stale_sticky(void)
+{
+	const vma_flags_t vma_flags = mk_vma_flags(VMA_READ_BIT, VMA_WRITE_BIT,
+			VMA_MAYREAD_BIT, VMA_MAYWRITE_BIT);
+	static const struct stale_sticky_shape shapes[] = {
+		/*
+		 * 0123456789
+		 *    VVVVNNN
+		 */
+		{ "merge right partial", false, true, 0x4000, 0x7000, false },
+		{ "merge right full",    false, true, 0x3000, 0x7000, false },
+		/*
+		 * 0123456789
+		 * PPPVVVV
+		 */
+		{ "merge left partial",  true, false, 0x3000, 0x6000, true },
+		{ "merge left full",     true, false, 0x3000, 0x7000, true },
+		/*
+		 * 0123456789
+		 * PPPVVVVNNN
+		 */
+		{ "merge both",          true, true,  0x3000, 0x7000, true },
+	};
+	struct mm_struct mm = {};
+	VMA_ITERATOR(vmi, &mm, 0);
+	struct vma_merge_struct vmg = {
+		.mm = &mm,
+		.vmi = &vmi,
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(shapes); i++) {
+		struct vm_area_struct *vma_prev = NULL, *vma_next = NULL;
+		struct vm_area_struct *vma, *merged, *expected;
+		vma_flags_t middle_flags = vma_flags;
+
+		if (shapes[i].has_prev)
+			vma_prev = alloc_and_link_vma(&mm, 0, 0x3000, 0,
+						      vma_flags);
+		/* Set after the caller's snapshot, as if by MADV_GUARD_INSTALL. */
+		vma_flags_set(&middle_flags, VMA_MAYBE_GUARD_BIT);
+		vma = alloc_and_link_vma(&mm, 0x3000, 0x7000, 3, middle_flags);
+		if (shapes[i].has_next)
+			vma_next = alloc_and_link_vma(&mm, 0x7000, 0xa000, 7,
+						      vma_flags);
+
+		/* The stale snapshot: no VMA_MAYBE_GUARD_BIT. */
+		vmg_set_range(&vmg, shapes[i].start, shapes[i].end,
+			      shapes[i].start >> PAGE_SHIFT, vma_flags);
+		vmg.middle = vma;
+		/* Offset into middle means prev is middle. */
+		vmg.prev = shapes[i].start > vma->vm_start ? vma : vma_prev;
+
+		expected = shapes[i].expect_prev ? vma_prev : vma_next;
+		merged = merge_existing(&vmg);
+		if (merged != expected || vmg.state != VMA_MERGE_SUCCESS) {
+			fprintf(stderr, "%s: merge did not occur as expected\n",
+				shapes[i].name);
+			return false;
+		}
+		if (!vma_test(merged, VMA_MAYBE_GUARD_BIT)) {
+			fprintf(stderr, "%s: VMA_MAYBE_GUARD_BIT lost on merge\n",
+				shapes[i].name);
+			return false;
+		}
+
+		cleanup_mm(&mm, &vmi);
+	}
+
+	return true;
+}
+
 static bool test_anon_vma_non_mergeable(void)
 {
 	vma_flags_t vma_flags = mk_vma_flags(VMA_READ_BIT, VMA_WRITE_BIT,
@@ -1501,6 +1592,7 @@ static void run_merge_tests(int *num_tests, int *num_fail)
 	TEST(vma_merge_with_close);
 	TEST(vma_merge_new_with_close);
 	TEST(merge_existing);
+	TEST(merge_existing_stale_sticky);
 	TEST(anon_vma_non_mergeable);
 	TEST(dup_anon_vma);
 	TEST(vmi_prealloc_fail);

-- 
~Gregory