[Devel] [PATCH RFC hci-8.1 0/4] mm: keep DMA-pinned MADV_DOEXEC memory working across exec

[Devel] [PATCH RFC hci-8.1 0/4] mm: keep DMA-pinned MADV_DOEXEC memory working across exec

[Andrey Drobyshev]

QEMU live update (qemu-update, cpr-exec) keeps guest RAM across exec with
MADV_DOEXEC.  vma_dup() duplicates such VMAs the same way fork does: the
pages get shared and write-protected in both mms.

That's fine for ordinary guest RAM, but not for a VM with a VFIO device.
VFIO holds long-term pins on all guest pages, so after exec the first
guest write to a pinned page can't reuse it and copies it instead.  The
guest and the new QEMU use the copy, while the IOMMU keeps pointing the
device at the old pinned page, and device DMA silently diverges.  After
qemu-update a passed-through GPU fails.

This series moves preserved anonymous VMAs into the new mm instead of
copying them.  Page table entries, physical pages and anon-exclusive state
move as is, so nothing gets copied on write, and the device and the guest
keep seeing the same memory.  Exec is refused while other tasks still share
the old mm, since moving its memory away would break them.

Tested with qemu-update with a whole GPU passed through (RTX 6000 Ada):
the GPU keeps working after qemu-update.

https://virtuozzo.atlassian.net/browse/VSTOR-103860

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
Andrey Drobyshev (4):
      mm: support moving anonymous page tables between address spaces
      exec: protect preserved memory against surviving shared-mm tasks
      mm: move MADV_DOEXEC memory instead of copying it
      selftests/mm: test memory preservation across exec

 fs/exec.c                                 | 102 +++-
 include/linux/binfmts.h                   |   2 +
 include/linux/huge_mm.h                   |   3 +-
 include/linux/mm.h                        |   4 -
 include/linux/mm_types.h                  |   5 +-
 include/linux/rmap.h                      |   7 +-
 kernel/fork.c                             |  34 ++
 mm/huge_memory.c                          |  28 +-
 mm/internal.h                             |   4 +
 mm/madvise.c                              |   2 +-
 mm/memory.c                               |  81 +---
 mm/mlock.c                                |   2 +-
 mm/mmap.c                                 |  64 ++-
 mm/mremap.c                               | 140 +++++-
 tools/testing/selftests/mm/Makefile       |   1 +
 tools/testing/selftests/mm/doexec.c       | 774 ++++++++++++++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh |   1 +
 17 files changed, 1112 insertions(+), 142 deletions(-)
---
base-commit: f5d1fdcfffe101fe2bafa7df09d95d2e811c5c5e
change-id: 20261007-vfio_v1-c812d56db752

Best regards,
--  
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH RFC PATCH hci-8.1 1/4] mm: support moving anonymous page tables between address spaces

[Andrey Drobyshev]

Teach move_page_tables and move_huge_pmd to use the destination mm for
page-table operations and transfer RSS, swap-list and huge-page table
accounting. Restrict cross-mm moves to same-address anonymous DOEXEC VMAs.

Add the parallel exec move wrapper, joining all workers before returning
on failure. Existing DOEXEC callers still use the copy path in this commit.

https://virtuozzo.atlassian.net/browse/VSTOR-103860

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 include/linux/huge_mm.h |   3 +-
 mm/huge_memory.c        |  28 +++++++---
 mm/internal.h           |   2 +
 mm/mremap.c             | 140 +++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 152 insertions(+), 21 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index b94c2e8ee918..e2ec0348bebd 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -31,7 +31,8 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, pmd_t *pmd,
 		 unsigned long addr);
 int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma, pud_t *pud,
 		 unsigned long addr);
-bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
+bool move_huge_pmd(struct vm_area_struct *vma, struct vm_area_struct *new_vma,
+		   unsigned long old_addr,
 		   unsigned long new_addr, pmd_t *old_pmd, pmd_t *new_pmd);
 int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 		    pmd_t *pmd, unsigned long addr, pgprot_t newprot,
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 90868f69ee33..9954dac5e53e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2219,12 +2219,15 @@ static pmd_t clear_uffd_wp_pmd(pmd_t pmd)
 	return pmd;
 }
 
-bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
+bool move_huge_pmd(struct vm_area_struct *vma, struct vm_area_struct *new_vma,
+		  unsigned long old_addr,
 		  unsigned long new_addr, pmd_t *old_pmd, pmd_t *new_pmd)
 {
 	spinlock_t *old_ptl, *new_ptl;
 	pmd_t pmd;
 	struct mm_struct *mm = vma->vm_mm;
+	struct mm_struct *new_mm = new_vma->vm_mm;
+	bool cross_mm = mm != new_mm;
 	bool force_flush = false;
 
 	/*
@@ -2243,7 +2246,7 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
 	 */
 	old_ptl = __pmd_trans_huge_lock(old_pmd, vma);
 	if (old_ptl) {
-		new_ptl = pmd_lockptr(mm, new_pmd);
+		new_ptl = pmd_lockptr(new_mm, new_pmd);
 		if (new_ptl != old_ptl)
 			spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
 		pmd = pmdp_huge_get_and_clear(mm, old_addr, old_pmd);
@@ -2251,15 +2254,28 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
 			force_flush = true;
 		VM_BUG_ON(!pmd_none(*new_pmd));
 
-		if (pmd_move_must_withdraw(new_ptl, old_ptl, vma)) {
+		if (cross_mm || pmd_move_must_withdraw(new_ptl, old_ptl, vma)) {
 			pgtable_t pgtable;
 			pgtable = pgtable_trans_huge_withdraw(mm, old_pmd);
-			pgtable_trans_huge_deposit(mm, new_pmd, pgtable);
+			pgtable_trans_huge_deposit(new_mm, new_pmd, pgtable);
+			if (cross_mm) {
+				mm_dec_nr_ptes(mm);
+				mm_inc_nr_ptes(new_mm);
+			}
+		}
+		if (cross_mm) {
+			if (is_huge_zero_pmd(pmd)) {
+				/* The source retains its per-mm zero-page reference. */
+				mm_get_huge_zero_folio(new_mm);
+			} else {
+				add_mm_counter(mm, MM_ANONPAGES, -HPAGE_PMD_NR);
+				add_mm_counter(new_mm, MM_ANONPAGES, HPAGE_PMD_NR);
+			}
 		}
 		pmd = move_soft_dirty_pmd(pmd);
-		if (vma_has_uffd_without_event_remap(vma))
+		if (cross_mm || vma_has_uffd_without_event_remap(vma))
 			pmd = clear_uffd_wp_pmd(pmd);
-		set_pmd_at(mm, new_addr, new_pmd, pmd);
+		set_pmd_at(new_mm, new_addr, new_pmd, pmd);
 		if (force_flush)
 			flush_pmd_tlb_range(vma, old_addr, old_addr + PMD_SIZE);
 		if (new_ptl != old_ptl)
diff --git a/mm/internal.h b/mm/internal.h
index d57e67a83d70..b0fed4393315 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1514,6 +1514,8 @@ extern struct list_lru shadow_nodes;
 } while (0)
 
 /* mremap.c */
+int move_page_range_exec(struct vm_area_struct *new_vma,
+			 struct vm_area_struct *old_vma);
 unsigned long move_page_tables(struct vm_area_struct *vma,
 	unsigned long old_addr, struct vm_area_struct *new_vma,
 	unsigned long new_addr, unsigned long len,
diff --git a/mm/mremap.c b/mm/mremap.c
index 1e6d40a240c9..6f64ff7a8aaa 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -25,6 +25,7 @@
 #include <linux/uaccess.h>
 #include <linux/userfaultfd_k.h>
 #include <linux/mempolicy.h>
+#include <linux/padata.h>
 
 #include <asm/cacheflush.h>
 #include <asm/tlb.h>
@@ -133,13 +134,59 @@ static pte_t move_soft_dirty_pte(pte_t pte)
 	return pte;
 }
 
+/* Transfer RSS without duplicating page references, rmaps or swap entries. */
+static void move_pte_account(struct vm_area_struct *vma, unsigned long addr,
+			     struct mm_struct *new_mm, pte_t pte)
+{
+	struct mm_struct *mm = vma->vm_mm;
+	struct folio *folio;
+	swp_entry_t entry;
+	int counter;
+
+	if (pte_present(pte)) {
+		folio = vm_normal_folio(vma, addr, pte);
+		if (!folio) {
+#ifdef CONFIG_KSM
+			if (is_ksm_zero_pte(pte)) {
+				atomic_long_dec(&mm->ksm_zero_pages);
+				atomic_long_inc(&new_mm->ksm_zero_pages);
+			}
+#endif
+			return;
+		}
+		counter = mm_counter(folio);
+	} else {
+		entry = pte_to_swp_entry(pte);
+		if (!non_swap_entry(entry)) {
+			/* swapoff must also visit the destination mm. */
+			if (list_empty(&new_mm->mmlist)) {
+				spin_lock(&mmlist_lock);
+				if (list_empty(&new_mm->mmlist))
+					list_add(&new_mm->mmlist, &mm->mmlist);
+				spin_unlock(&mmlist_lock);
+			}
+			counter = MM_SWAPENTS;
+		} else if (is_migration_entry(entry) ||
+			   is_device_private_entry(entry) ||
+			   is_device_exclusive_entry(entry)) {
+			counter = mm_counter(pfn_swap_entry_folio(entry));
+		} else {
+			return;
+		}
+	}
+	dec_mm_counter(mm, counter);
+	inc_mm_counter(new_mm, counter);
+}
+
 static int move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
 		unsigned long old_addr, unsigned long old_end,
 		struct vm_area_struct *new_vma, pmd_t *new_pmd,
 		unsigned long new_addr, bool need_rmap_locks)
 {
-	bool need_clear_uffd_wp = vma_has_uffd_without_event_remap(vma);
 	struct mm_struct *mm = vma->vm_mm;
+	struct mm_struct *new_mm = new_vma->vm_mm;
+	bool cross_mm = mm != new_mm;
+	bool need_clear_uffd_wp = cross_mm || vma_has_uffd_without_event_remap(vma);
 	pte_t *old_pte, *new_pte, pte;
 	pmd_t dummy_pmdval;
 	spinlock_t *old_ptl, *new_ptl;
@@ -184,7 +231,7 @@ static int move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
 	 * mmap_lock, so this new_pte page is stable, so there is no need to get
 	 * pmdval and do pmd_same() check.
 	 */
-	new_pte = pte_offset_map_rw_nolock(mm, new_pmd, new_addr, &dummy_pmdval,
+	new_pte = pte_offset_map_rw_nolock(new_mm, new_pmd, new_addr, &dummy_pmdval,
 					   &new_ptl);
 	if (!new_pte) {
 		pte_unmap_unlock(old_pte, old_ptl);
@@ -202,6 +249,8 @@ static int move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
 			continue;
 
 		pte = ptep_get_and_clear(mm, old_addr, old_pte);
+		if (cross_mm)
+			move_pte_account(vma, old_addr, new_mm, pte);
 		/*
 		 * If we are remapping a valid PTE, make sure
 		 * to flush TLB before we drop the PTL for the
@@ -219,7 +268,7 @@ static int move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
 		pte = move_soft_dirty_pte(pte);
 
 		if (need_clear_uffd_wp && pte_marker_uffd_wp(pte))
-			pte_clear(mm, new_addr, new_pte);
+			pte_clear(new_mm, new_addr, new_pte);
 		else {
 			if (need_clear_uffd_wp) {
 				if (pte_present(pte))
@@ -227,7 +276,7 @@ static int move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
 				else if (is_swap_pte(pte))
 					pte = pte_swp_clear_uffd_wp(pte);
 			}
-			set_pte_at(mm, new_addr, new_pte, pte);
+			set_pte_at(new_mm, new_addr, new_pte, pte);
 		}
 	}
 
@@ -495,6 +544,7 @@ static __always_inline unsigned long get_extent(enum pgt_entry entry,
  * pgt_entry. Returns true if the move was successful, else false.
  */
 static bool move_pgt_entry(enum pgt_entry entry, struct vm_area_struct *vma,
+			struct vm_area_struct *new_vma,
 			unsigned long old_addr, unsigned long new_addr,
 			void *old_entry, void *new_entry, bool need_rmap_locks)
 {
@@ -515,7 +565,7 @@ static bool move_pgt_entry(enum pgt_entry entry, struct vm_area_struct *vma,
 		break;
 	case HPAGE_PMD:
 		moved = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
-			move_huge_pmd(vma, old_addr, new_addr, old_entry,
+			move_huge_pmd(vma, new_vma, old_addr, new_addr, old_entry,
 				      new_entry);
 		break;
 	case HPAGE_PUD:
@@ -596,10 +646,19 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 	struct mmu_notifier_range range;
 	pmd_t *old_pmd, *new_pmd;
 	pud_t *old_pud, *new_pud;
+	bool cross_mm = vma->vm_mm != new_vma->vm_mm;
 
 	if (!len)
 		return 0;
 
+	/* Cross-mm moves are only used for same-address anonymous DOEXEC VMAs. */
+	if (cross_mm) {
+		if (WARN_ON_ONCE(!vma_is_anonymous(vma) ||
+				 !(vma->vm_flags & VM_EXEC_KEEP) ||
+				 old_addr != new_addr || !need_rmap_locks))
+			return 0;
+	}
+
 	old_end = old_addr + len;
 
 	if (is_vm_hugetlb_page(vma))
@@ -610,7 +669,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 	 * If possible, realign addresses to PMD boundary for faster copy.
 	 * Only realign if the mremap copying hits a PMD boundary.
 	 */
-	if (len >= PMD_SIZE - (old_addr & ~PMD_MASK))
+	if (!cross_mm && len >= PMD_SIZE - (old_addr & ~PMD_MASK))
 		try_realign_addr(&old_addr, vma, &new_addr, new_vma, PMD_MASK,
 				 for_stack);
 
@@ -630,19 +689,23 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 		old_pud = get_old_pud(vma->vm_mm, old_addr);
 		if (!old_pud)
 			continue;
-		new_pud = alloc_new_pud(vma->vm_mm, vma, new_addr);
+		new_pud = alloc_new_pud(new_vma->vm_mm, new_vma, new_addr);
 		if (!new_pud)
 			break;
 		if (pud_trans_huge(*old_pud) || pud_devmap(*old_pud)) {
+			/* Anonymous PUD-sized THPs are not supported. */
+			if (cross_mm)
+				break;
 			if (extent == HPAGE_PUD_SIZE) {
-				move_pgt_entry(HPAGE_PUD, vma, old_addr, new_addr,
+				move_pgt_entry(HPAGE_PUD, vma, new_vma, old_addr, new_addr,
 					       old_pud, new_pud, need_rmap_locks);
 				/* We ignore and continue on error? */
 				continue;
 			}
-		} else if (IS_ENABLED(CONFIG_HAVE_MOVE_PUD) && extent == PUD_SIZE) {
+		} else if (!cross_mm && IS_ENABLED(CONFIG_HAVE_MOVE_PUD) &&
+			   extent == PUD_SIZE) {
 
-			if (move_pgt_entry(NORMAL_PUD, vma, old_addr, new_addr,
+			if (move_pgt_entry(NORMAL_PUD, vma, new_vma, old_addr, new_addr,
 					   old_pud, new_pud, true))
 				continue;
 		}
@@ -651,24 +714,24 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 		old_pmd = get_old_pmd(vma->vm_mm, old_addr);
 		if (!old_pmd)
 			continue;
-		new_pmd = alloc_new_pmd(vma->vm_mm, vma, new_addr);
+		new_pmd = alloc_new_pmd(new_vma->vm_mm, new_vma, new_addr);
 		if (!new_pmd)
 			break;
 again:
 		if (is_swap_pmd(*old_pmd) || pmd_trans_huge(*old_pmd) ||
 		    pmd_devmap(*old_pmd)) {
 			if (extent == HPAGE_PMD_SIZE &&
-			    move_pgt_entry(HPAGE_PMD, vma, old_addr, new_addr,
+			    move_pgt_entry(HPAGE_PMD, vma, new_vma, old_addr, new_addr,
 					   old_pmd, new_pmd, need_rmap_locks))
 				continue;
 			split_huge_pmd(vma, old_pmd, old_addr);
-		} else if (IS_ENABLED(CONFIG_HAVE_MOVE_PMD) &&
+		} else if (!cross_mm && IS_ENABLED(CONFIG_HAVE_MOVE_PMD) &&
 			   extent == PMD_SIZE) {
 			/*
 			 * If the extent is PMD-sized, try to speed the move by
 			 * moving at the PMD level if possible.
 			 */
-			if (move_pgt_entry(NORMAL_PMD, vma, old_addr, new_addr,
+			if (move_pgt_entry(NORMAL_PMD, vma, new_vma, old_addr, new_addr,
 					   old_pmd, new_pmd, true))
 				continue;
 		}
@@ -693,6 +756,55 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 	return len + old_addr - old_end;	/* how much done */
 }
 
+struct move_page_range_args {
+	struct vm_area_struct *old_vma;
+	struct vm_area_struct *new_vma;
+};
+
+static int move_page_range_chunk(unsigned long addr, unsigned long end, void *arg)
+{
+	struct move_page_range_args *args = arg;
+	unsigned long len = end - addr;
+
+	return move_page_tables(args->old_vma, addr, args->new_vma, addr,
+				len, true, false) == len ? 0 : -ENOMEM;
+}
+
+/*
+ * The caller holds both mmap locks while workers transfer disjoint PMDs.
+ * A failure can leave mappings in both mms; there is no rollback. This is
+ * only usable after exec's point of no return, where errors are fatal.
+ * padata_do_multithreaded() joins all workers, including on error, before
+ * either mm may be torn down.
+ */
+int move_page_range_exec(struct vm_area_struct *new_vma,
+			 struct vm_area_struct *old_vma)
+{
+	struct move_page_range_args args = { old_vma, new_vma };
+#ifdef CONFIG_PADATA
+	struct padata_mt_job job = {
+		.thread_fn = move_page_range_chunk,
+		.fn_arg = &args,
+		.start = old_vma->vm_start,
+		.size = old_vma->vm_end - old_vma->vm_start,
+		.align = PMD_SIZE,
+		.min_chunk = max(1ul << 27, PMD_SIZE),
+		.max_threads = 16,
+	};
+#endif
+
+	mmap_assert_write_locked(old_vma->vm_mm);
+	mmap_assert_write_locked(new_vma->vm_mm);
+	if (WARN_ON_ONCE(!(old_vma->vm_flags & VM_EXEC_KEEP)))
+		return -EINVAL;
+
+#ifdef CONFIG_PADATA
+	return padata_do_multithreaded(&job);
+#else
+	return move_page_range_chunk(old_vma->vm_start, old_vma->vm_end, &args);
+#endif
+}
+
 static unsigned long move_vma(struct vm_area_struct *vma,
 		unsigned long old_addr, unsigned long old_len,
 		unsigned long new_len, unsigned long new_addr,

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH RFC PATCH hci-8.1 2/4] exec: protect preserved memory against surviving shared-mm tasks

[Andrey Drobyshev]

Reserve the marked mm before exec becomes irreversible. Reject tasks
outside the execing thread group while allowing inspection references
and sibling threads that de_thread will remove.

Track CLONE_VM creations from copy_mm until publication or failure so
admission cannot miss an unpublished task. Reject new shared-mm clones
while the reservation is held, and release it on mm switch or exec failure.
Use kABI reserve slots and a spare binprm bit to preserve existing layouts.

https://virtuozzo.atlassian.net/browse/VSTOR-103860

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 fs/exec.c                | 88 +++++++++++++++++++++++++++++++++++++++++++++---
 include/linux/binfmts.h  |  2 ++
 include/linux/mm_types.h |  5 +--
 kernel/fork.c            | 34 +++++++++++++++++++
 4 files changed, 122 insertions(+), 7 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index e861a54b491f..5b2f9c3b0d06 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -978,6 +978,79 @@ ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
 EXPORT_SYMBOL(read_code);
 #endif
 
+/* The reservation owns no mm reference; release it before dropping old_mm. */
+static void release_preserved_mem(struct linux_binprm *bprm, struct mm_struct *mm)
+{
+	if (!bprm->preserving_mem)
+		return;
+	spin_lock(&mm->page_table_lock);
+	mm->exec_keep_active = false;
+	spin_unlock(&mm->page_table_lock);
+	bprm->preserving_mem = false;
+}
+
+/*
+ * Reserve the marked address space before exec becomes irreversible.
+ * Inspection references do not constitute shared execution: mmap_lock
+ * serializes those readers with the mapping transfer. Only tasks outside
+ * our group survive de_thread() and must be rejected here.
+ */
+static int reserve_preserved_mem(struct linux_binprm *bprm)
+{
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+	struct task_struct *p, *t;
+	bool preserve = false;
+	int ret = 0;
+
+	if (!bprm->accepts_preserved_mem || !mm)
+		return 0;
+
+	if (mmap_read_lock_killable(mm))
+		return -EINTR;
+	{
+		VMA_ITERATOR(vmi, mm, 0);
+
+		for_each_vma(vmi, vma) {
+			if (vma->vm_flags & VM_EXEC_KEEP) {
+				preserve = true;
+				break;
+			}
+		}
+	}
+	mmap_read_unlock(mm);
+	if (!preserve)
+		return 0;
+
+	/* A clone is either counted here or already visible in the task list. */
+	spin_lock(&mm->page_table_lock);
+	if (mm->exec_keep_active || mm->exec_keep_clones)
+		ret = -EBUSY;
+	else
+		mm->exec_keep_active = true;
+	spin_unlock(&mm->page_table_lock);
+	if (ret)
+		return ret;
+	bprm->preserving_mem = true;
+
+	read_lock(&tasklist_lock);
+	for_each_process_thread(p, t) {
+		if (same_thread_group(current, t))
+			continue;
+		task_lock(t);
+		if (t->mm == mm)
+			ret = -EBUSY;
+		task_unlock(t);
+		if (ret)
+			goto out;
+	}
+out:
+	read_unlock(&tasklist_lock);
+	if (ret)
+		release_preserved_mem(bprm, mm);
+	return ret;
+}
+
 static int vma_dup_some(struct mm_struct *old_mm, struct mm_struct *new_mm)
 {
 	struct vm_area_struct *vma;
@@ -1028,7 +1101,7 @@ static int exec_mmap(struct linux_binprm *bprm)
 			up_write(&tsk->signal->exec_update_lock);
 			return ret;
 		}
-		if (bprm->accepts_preserved_mem) {
+		if (bprm->preserving_mem) {
 			ret = vma_dup_some(old_mm, mm);
 			if (ret) {
 				mmap_read_unlock(old_mm);
@@ -1067,6 +1140,7 @@ static int exec_mmap(struct linux_binprm *bprm)
 		BUG_ON(active_mm != old_mm);
 		setmax_mm_hiwater_rss(&tsk->signal->maxrss, old_mm);
 		mm_update_next_owner(old_mm);
+		release_preserved_mem(bprm, old_mm);
 		mmput(old_mm);
 		return 0;
 	}
@@ -1260,10 +1334,9 @@ void __set_task_comm(struct task_struct *tsk, const char *buf, bool exec)
 }
 
 /*
- * Calling this is the point of no return. None of the failures will be
- * seen by userspace since either the process is already taking a fatal
- * signal (via de_thread() or coredump), or will have SEGV raised
- * (after exec_mmap()) by search_binary_handler (see below).
+ * Complete the recoverable checks before setting point_of_no_return.
+ * Subsequent failures cannot return to the old image: either a fatal
+ * signal is already pending, or bprm_execve() raises SIGSEGV.
  */
 int begin_new_exec(struct linux_binprm * bprm)
 {
@@ -1275,6 +1348,10 @@ int begin_new_exec(struct linux_binprm * bprm)
 	if (retval)
 		return retval;
 
+	retval = reserve_preserved_mem(bprm);
+	if (retval)
+		return retval;
+
 	/*
 	 * This tracepoint marks the point before flushing the old exec where
 	 * the current task is still unchanged, but errors are fatal (point of
@@ -1527,6 +1604,7 @@ static void do_close_execat(struct file *file)
 
 static void free_bprm(struct linux_binprm *bprm)
 {
+	release_preserved_mem(bprm, current->mm);
 	if (bprm->mm) {
 		acct_arg_size(bprm, 0);
 		mmput(bprm->mm);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 9f03c39498e9..7c4defdc184f 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -51,6 +51,8 @@ struct linux_binprm {
 #ifdef __alpha__
 	unsigned int taso:1;
 #endif
+	/* Uses spare bits without changing the existing binprm layout. */
+	RH_KABI_EXCLUDE(unsigned int preserving_mem:1)
 	struct file *executable; /* Executable to pass to the interpreter */
 	struct file *interpreter;
 	struct file *file;
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index cf50c74516b0..0a74c377f64c 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1091,8 +1091,9 @@ struct mm_struct {
 		} lru_gen;
 #endif /* CONFIG_LRU_GEN_WALKS_MMU */
 	} __randomize_layout;
-	RH_KABI_RESERVE(1)
-	RH_KABI_RESERVE(2)
+	/* Shared-mm clone publication, serialized by page_table_lock. */
+	RH_KABI_USE(1, unsigned int exec_keep_clones)
+	RH_KABI_USE(2, bool exec_keep_active)
 	RH_KABI_RESERVE(3)
 	RH_KABI_RESERVE(4)
 	/*
diff --git a/kernel/fork.c b/kernel/fork.c
index 00d9b4febd85..bd8020af2037 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1270,6 +1270,8 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 	mt_init_flags(&mm->mm_mt, MM_MT_FLAGS);
 	mt_set_external_lock(&mm->mm_mt, &mm->mmap_lock);
 	atomic_set(&mm->mm_users, 1);
+	mm->exec_keep_clones = 0;
+	mm->exec_keep_active = false;
 	atomic_set(&mm->mm_count, 1);
 	seqcount_init(&mm->write_protect_seq);
 	mmap_init_lock(mm);
@@ -1710,9 +1712,32 @@ static struct mm_struct *dup_mm(struct task_struct *tsk,
 	return NULL;
 }
 
+/* Keep unpublished CLONE_VM tasks visible to preserving-exec admission. */
+static int exec_keep_clone_begin(struct mm_struct *mm)
+{
+	int ret = 0;
+
+	spin_lock(&mm->page_table_lock);
+	if (mm->exec_keep_active)
+		ret = -EBUSY;
+	else
+		mm->exec_keep_clones++;
+	spin_unlock(&mm->page_table_lock);
+	return ret;
+}
+
+static void exec_keep_clone_end(struct mm_struct *mm)
+{
+	spin_lock(&mm->page_table_lock);
+	if (!WARN_ON_ONCE(!mm->exec_keep_clones))
+		mm->exec_keep_clones--;
+	spin_unlock(&mm->page_table_lock);
+}
+
 static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
 {
 	struct mm_struct *mm, *oldmm;
+	int ret;
 
 	tsk->min_flt = tsk->maj_flt = 0;
 	tsk->nvcsw = tsk->nivcsw = 0;
@@ -1734,6 +1759,9 @@ static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
 		return 0;
 
 	if (clone_flags & CLONE_VM) {
+		ret = exec_keep_clone_begin(oldmm);
+		if (ret)
+			return ret;
 		mmget(oldmm);
 		mm = oldmm;
 	} else {
@@ -2638,6 +2666,10 @@ __latent_entropy struct task_struct *copy_process(
 	syscall_tracepoint_update(p);
 	write_unlock_irq(&tasklist_lock);
 
+	/* Admission can now find this shared-mm task in the task list. */
+	if ((clone_flags & CLONE_VM) && p->mm)
+		exec_keep_clone_end(p->mm);
+
 	if (pidfile)
 		fd_install(pidfd, pidfile);
 
@@ -2679,6 +2711,8 @@ __latent_entropy struct task_struct *copy_process(
 	exit_ve_namespace(p);
 bad_fork_cleanup_mm:
 	if (p->mm) {
+		if (clone_flags & CLONE_VM)
+			exec_keep_clone_end(p->mm);
 		mm_clear_owner(p->mm, p);
 		mmput(p->mm);
 	}

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH RFC PATCH hci-8.1 3/4] mm: move MADV_DOEXEC memory instead of copying it

[Andrey Drobyshev]

Fork-style duplication write-protects preserved guest RAM. With long-term
DMA pins, subsequent writes can COW to new frames while the device keeps
accessing the old frames. Move mappings to the new mm instead, preserving
physical pages, permissions and anonymous-page exclusivity.

Use the exec reservation and old-mm write lock, clone the anonymous VMA
lineage, unmerge KSM and drop mlock state. Transfer the commit reservation
and pin-aware fork state, and remove the obsolete parallel copying and
exec-specific pinned-page sharing exception. Partial-move failures remain
fatal and tear down both mms after workers finish.

https://virtuozzo.atlassian.net/browse/VSTOR-103860

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 fs/exec.c            | 16 ++++++++---
 include/linux/mm.h   |  4 ---
 include/linux/rmap.h |  7 ++---
 mm/internal.h        |  2 ++
 mm/madvise.c         |  2 +-
 mm/memory.c          | 81 ++++------------------------------------------------
 mm/mlock.c           |  2 +-
 mm/mmap.c            | 64 +++++++++++++++++++++++++----------------
 8 files changed, 63 insertions(+), 115 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 5b2f9c3b0d06..32532111244f 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1057,6 +1057,7 @@ static int vma_dup_some(struct mm_struct *old_mm, struct mm_struct *new_mm)
 	VMA_ITERATOR(vmi, old_mm, 0);
 	int ret = 0;
 
+	update_hiwater_rss(old_mm);
 	mmap_write_lock_nested(new_mm, SINGLE_DEPTH_NESTING);
 	for_each_vma(vmi, vma)
 		if (vma->vm_flags & VM_EXEC_KEEP) {
@@ -1079,6 +1080,7 @@ static int exec_mmap(struct linux_binprm *bprm)
 	struct task_struct *tsk;
 	struct mm_struct *old_mm, *active_mm;
 	struct mm_struct *mm = bprm->mm;
+	const bool preserve = bprm->preserving_mem;
 	int ret;
 
 	/* Notify parent that we're no longer interested in the old VM */
@@ -1096,15 +1098,18 @@ static int exec_mmap(struct linux_binprm *bprm)
 		 * whose default action is to create a coredump get
 		 * out and die instead of going through with the exec.
 		 */
-		ret = mmap_read_lock_killable(old_mm);
+		if (preserve)
+			ret = mmap_write_lock_killable(old_mm);
+		else
+			ret = mmap_read_lock_killable(old_mm);
 		if (ret) {
 			up_write(&tsk->signal->exec_update_lock);
 			return ret;
 		}
-		if (bprm->preserving_mem) {
+		if (preserve) {
 			ret = vma_dup_some(old_mm, mm);
 			if (ret) {
-				mmap_read_unlock(old_mm);
+				mmap_write_unlock(old_mm);
 				up_write(&tsk->signal->exec_update_lock);
 				return ret;
 			}
@@ -1136,7 +1141,10 @@ static int exec_mmap(struct linux_binprm *bprm)
 	task_unlock(tsk);
 	lru_gen_use_mm(mm);
 	if (old_mm) {
-		mmap_read_unlock(old_mm);
+		if (preserve)
+			mmap_write_unlock(old_mm);
+		else
+			mmap_read_unlock(old_mm);
 		BUG_ON(active_mm != old_mm);
 		setmax_mm_hiwater_rss(&tsk->signal->maxrss, old_mm);
 		mm_update_next_owner(old_mm);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 75b8bcd9f442..a4df541f8591 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2479,10 +2479,6 @@ void free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
 		unsigned long end, unsigned long floor, unsigned long ceiling);
 int
 copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma);
-#ifdef CONFIG_PADATA
-int copy_page_range_mt(struct vm_area_struct *dst_vma,
-			struct vm_area_struct *src_vma);
-#endif
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 			void *buf, int len, int write);
 
diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 5dc4cd6e4e2c..3645abd38d68 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -400,7 +400,6 @@ static __always_inline int __folio_try_dup_anon_rmap(struct folio *folio,
 	const int orig_nr_pages = nr_pages;
 	bool maybe_pinned;
 	int i;
-	bool is_exec_keep;
 
 	VM_WARN_ON_FOLIO(!folio_test_anon(folio), folio);
 	__folio_rmap_sanity_checks(folio, page, nr_pages, level);
@@ -415,8 +414,6 @@ static __always_inline int __folio_try_dup_anon_rmap(struct folio *folio,
 	maybe_pinned = likely(!folio_is_device_private(folio)) &&
 		       unlikely(folio_needs_cow_for_dma(src_vma, folio));
 
-	is_exec_keep = dst_vma->vm_flags & VM_EXEC_KEEP ? true : false;
-
 	/*
 	 * No need to check+clear for already shared PTEs/PMDs of the
 	 * folio. But if any page is PageAnonExclusive, we must fallback to
@@ -424,7 +421,7 @@ static __always_inline int __folio_try_dup_anon_rmap(struct folio *folio,
 	 */
 	switch (level) {
 	case RMAP_LEVEL_PTE:
-		if (unlikely(maybe_pinned) && likely(!is_exec_keep)) {
+		if (unlikely(maybe_pinned)) {
 			for (i = 0; i < nr_pages; i++)
 				if (PageAnonExclusive(page + i))
 					return -EBUSY;
@@ -446,7 +443,7 @@ static __always_inline int __folio_try_dup_anon_rmap(struct folio *folio,
 		break;
 	case RMAP_LEVEL_PMD:
 		if (PageAnonExclusive(page)) {
-			if (unlikely(maybe_pinned) && likely(!is_exec_keep))
+			if (unlikely(maybe_pinned))
 				return -EBUSY;
 			ClearPageAnonExclusive(page);
 		}
diff --git a/mm/internal.h b/mm/internal.h
index b0fed4393315..caf6eabd22b0 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -928,6 +928,8 @@ static inline void mlock_vma_folio(struct folio *folio,
 }
 
 void munlock_folio(struct folio *folio);
+void mlock_vma_pages_range(struct vm_area_struct *vma, unsigned long start,
+			 unsigned long end, vm_flags_t newflags);
 static inline void munlock_vma_folio(struct folio *folio,
 					struct vm_area_struct *vma)
 {
diff --git a/mm/madvise.c b/mm/madvise.c
index b89d891e122c..109b5222f48e 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1655,7 +1655,7 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start,
  *		triggering read faults if required
  *  MADV_POPULATE_WRITE - populate (prefault) page tables writable by
  *		triggering write faults if required
- *  MADV_DOEXEC - On exec, preserve and duplicate this area in the new process
+ *  MADV_DOEXEC - On exec, transfer this area's mappings to the new process
  *		if the new process allows it.
  *  MADV_DONTEXEC - Undo the effect of MADV_DOEXEC.
  *
diff --git a/mm/memory.c b/mm/memory.c
index 5e414c68f3a9..73060460496c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1436,66 +1436,6 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
 	return ret;
 }
 
-#ifdef CONFIG_PADATA
-
-struct copy_page_range_args {
-	struct vm_area_struct *dst_vma;
-	struct vm_area_struct *src_vma;
-};
-
-static int copy_page_range_chunk(unsigned long addr,
-				 unsigned long end, void *arg)
-{
-	struct copy_page_range_args *args = arg;
-	struct vm_area_struct *dst_vma = args->dst_vma;
-	struct vm_area_struct *src_vma = args->src_vma;
-	struct mm_struct *dst_mm = dst_vma->vm_mm;
-	struct mm_struct *src_mm = src_vma->vm_mm;
-	pgd_t *src_pgd, *dst_pgd;
-	unsigned long next;
-	int ret = 0;
-
-	dst_pgd = pgd_offset(dst_mm, addr);
-	src_pgd = pgd_offset(src_mm, addr);
-
-	do {
-		next = pgd_addr_end(addr, end);
-		if (pgd_none_or_clear_bad(src_pgd))
-			continue;
-		if (unlikely(copy_p4d_range(dst_vma, src_vma, dst_pgd, src_pgd,
-					    addr, next))) {
-			ret = -ENOMEM;
-			break;
-		}
-	} while (dst_pgd++, src_pgd++, addr = next, addr != end);
-
-	return ret;
-}
-
-/*
- * A stripped down version of copy_page_range() used to copy a VMA as part
- * of preserving it across exec. Multithreading via padata is used to speed
- * up the copying of very large VMAs.
- */
-int copy_page_range_mt(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
-{
-	struct copy_page_range_args args = { dst_vma, src_vma };
-	struct padata_mt_job job = {
-		.thread_fn   = copy_page_range_chunk,
-		.fn_arg      = &args,
-		.start       = src_vma->vm_start,
-		.size        = src_vma->vm_end - src_vma->vm_start,
-		.align       = PMD_SIZE,
-		.min_chunk   = max(1ul << 27, PMD_SIZE),
-		.max_threads = 16,
-	};
-
-	BUG_ON(!(src_vma->vm_flags & VM_EXEC_KEEP));
-
-	return padata_do_multithreaded(&job);
-}
-#endif /* CONFIG_PADATA */
-
 /* Whether we should zap all COWed (private) pages too */
 static inline bool should_zap_cows(struct zap_details *details)
 {
@@ -2033,8 +1973,7 @@ static void unmap_page_range_mt(struct mmu_gather *tlb,
 	BUG_ON(addr >= end);
 
 	/*
-	 * Assumptions made below about VM_EXEC_KEEP VMAs only hold true
-	 * if the entire address space is being unmapped.
+	 * Parallel unmapping is only safe when tearing down the full mm.
 	 */
 	if (!tlb->fullmm) {
 		unmap_page_range(tlb, vma, addr, end, details);
@@ -2042,20 +1981,12 @@ static void unmap_page_range_mt(struct mmu_gather *tlb,
 	}
 
 	/*
-	 * Pages in VMAs being preserved will have an additional reference
-	 * due to the mappings having been copied prior to unmapping the full
-	 * address space. Since lock contention due to freeing pages will not
-	 * be an issue, use additional threads to unmap them.
-	 * Otherwise for sufficiently large, private anon VMAs, using
-	 * multiple threads can result in severe lock contention the swap
-	 * subsystem if a signficant portion is swapped out. Since the number
-	 * of swapped pages is tracked per MM and not per VMA, err on the
-	 * side of caution and assume the number applies to the VMA and revert
-	 * to unmapping single threaded if necessary.
+	 * For sufficiently large, private anon VMAs, parallel unmapping can
+	 * cause severe swap lock contention. The swap count is per-mm, so
+	 * conservatively assume it applies to this VMA. Preserved mappings
+	 * have already been moved out and need no special treatment here.
 	 */
-	if (vma->vm_flags & VM_EXEC_KEEP) {
-		job.max_threads = 16;
-	} else if (!(vma->vm_flags & VM_SHARED) && total_sz >= min_chunk_sz * 3) {
+	if (!(vma->vm_flags & VM_SHARED) && total_sz >= min_chunk_sz * 3) {
 		unsigned long swapped_sz;
 
 		swapped_sz = get_mm_counter(tlb->mm, MM_SWAPENTS) << PAGE_SHIFT;
diff --git a/mm/mlock.c b/mm/mlock.c
index 3fc174451561..0d362033c299 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -422,7 +422,7 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,
  * Called for mlock(), mlock2() and mlockall(), to set @vma VM_LOCKED;
  * called for munlock() and munlockall(), to clear VM_LOCKED from @vma.
  */
-static void mlock_vma_pages_range(struct vm_area_struct *vma,
+void mlock_vma_pages_range(struct vm_area_struct *vma,
 	unsigned long start, unsigned long end, vm_flags_t newflags)
 {
 	static const struct mm_walk_ops mlock_walk_ops = {
diff --git a/mm/mmap.c b/mm/mmap.c
index 320f397816df..3db7c8316382 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1751,21 +1751,40 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
 }
 
 /*
- * Duplicate an anonymous VMA into a new mm as part of preserving
- * it across exec.
+ * Transfer an anonymous VMA's mappings to a new mm across exec. Both mmap
+ * locks must be held for writing; other tasks using the old mm must be quiesced.
  */
 int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 {
 	struct vm_area_struct *vma;
 	unsigned long npages;
+	vm_flags_t flags = old_vma->vm_flags;
 	int ret = -ENOMEM;
 
+	mmap_assert_write_locked(old_vma->vm_mm);
+	mmap_assert_write_locked(mm);
+	vma_start_write(old_vma);
+
 	if (WARN_ON(!vma_is_anonymous(old_vma)))
 		return -EINVAL;
 
 	if (find_vma_intersection(mm, old_vma->vm_start, old_vma->vm_end))
 		return -EEXIST;
 
+	/* KSM tracks mappings by mm and address; unmerge before moving them. */
+	ret = ksm_madvise(old_vma, old_vma->vm_start, old_vma->vm_end,
+			  MADV_UNMERGEABLE, &flags);
+	if (ret)
+		return ret;
+
+	/* The new image does not inherit mlock, including the folio state. */
+	if (old_vma->vm_flags & VM_LOCKED) {
+		old_vma->vm_mm->locked_vm -= vma_pages(old_vma);
+		mlock_vma_pages_range(old_vma, old_vma->vm_start,
+				      old_vma->vm_end,
+				      old_vma->vm_flags & ~VM_LOCKED_MASK);
+	}
+
 	npages = vma_pages(old_vma);
 	vm_stat_account(mm, old_vma->vm_flags, npages);
 
@@ -1778,17 +1797,17 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 		goto fail_nomem_policy;
 
 	vma->vm_mm = mm;
-	ret = anon_vma_fork(vma, old_vma);
+	/* Keep the same anon_vma: this is a move, not a fork. */
+	ret = anon_vma_clone(vma, old_vma);
 	if (ret)
-		goto fail_nomem_anon_vma_fork;
+		goto fail_nomem_anon_vma_clone;
 
 	/*
 	 * Clear functionality that should not carry over to the new
-	 * process. Note that VM_EXEC_KEEP is cleared later to allow
-	 * code called by copy_page_range to infer that the copying is
-	 * for preserving over exec and not for process forking.
+	 * process. VM_EXEC_KEEP is consumed by this exec.
 	 */
-	vm_flags_clear(vma, VM_LOCKED|VM_LOCKONFAULT|VM_UFFD_MISSING|VM_UFFD_WP|VM_UFFD_MINOR);
+	vm_flags_clear(vma, VM_LOCKED_MASK | VM_UFFD_MISSING | VM_UFFD_WP |
+		       VM_UFFD_MINOR | VM_EXEC_KEEP);
 	vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
 
 	ret = vma_link(mm, vma);
@@ -1796,29 +1815,24 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 		goto fail_nomem_vma_link;
 
 	/*
-	 * Now that the dup vma is inserted into the mm, clear VM_ACCOUNT
-	 * from old_vma.  Since vma_dup() is only called during exec to
-	 * duplicate a vma from the outgoing mm into the mm of the new
-	 * process, this effectively transfers the accounting from the old
-	 * vma to new one.
-	 * exec holds the mmap read lock on the outgoing mm at this point
-	 * so __vm_flags_mod() is used here to avoid an assert that the
-	 * mmap write lock is held which is not needed in this instance..
+	 * Transfer the reservation to the linked destination VMA. If moving
+	 * page tables fails partway through, exec cannot resume the old image:
+	 * it must terminate and tear down both mms. Each mm then frees its
+	 * remaining mappings, and the destination releases the reservation.
 	 */
-	__vm_flags_mod(old_vma, 0, VM_ACCOUNT);
+	vm_flags_clear(old_vma, VM_ACCOUNT);
 
-#ifdef CONFIG_PADATA
-	ret = copy_page_range_mt(vma, old_vma);
-#else
-	ret = copy_page_range(vma, old_vma);
-#endif
-	vm_flags_clear(vma, VM_EXEC_KEEP);
+	/* Preserve the pin-aware fork checks in the new mm. */
+	if (test_bit(MMF_HAS_PINNED, &old_vma->vm_mm->flags))
+		set_bit(MMF_HAS_PINNED, &mm->flags);
 
-	return ret;
+	if (!test_bit(MMF_VM_MERGEABLE, &mm->flags))
+		ksm_fork(mm, old_vma->vm_mm);
+	return move_page_range_exec(vma, old_vma);
 
 fail_nomem_vma_link:
 	unlink_anon_vmas(vma);
-fail_nomem_anon_vma_fork:
+fail_nomem_anon_vma_clone:
 	mpol_put(vma_policy(vma));
 fail_nomem_policy:
 	vm_area_free(vma);

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH RFC PATCH hci-8.1 4/4] selftests/mm: test memory preservation across exec

[Andrey Drobyshev]

Add an ELF-opted-in MADV_DOEXEC test and wire it into the mm test runner.
Compare CPU writes against retained long-term GUP pins across repeated
exec and a later fork, including base pages, THP, mlock, protected mappings
and fast GUP. Check existing COW mappings separately.

Exercise recoverable rejection and retry with an external CLONE_VM task,
own and non-leader threads, concurrent shared-mm clone creation, proc
inspection during repeated exec, and vhost owner reset before exec. Add
an optional task-local allocation-failure sweep.

https://virtuozzo.atlassian.net/browse/VSTOR-103860

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 tools/testing/selftests/mm/Makefile       |   1 +
 tools/testing/selftests/mm/doexec.c       | 774 ++++++++++++++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh |   1 +
 3 files changed, 776 insertions(+)

diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index c837b0549c89..7f9efb0e0a38 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -74,6 +74,7 @@ endif
 
 TEST_GEN_FILES = cow
 TEST_GEN_FILES += compaction_test
+TEST_GEN_FILES += doexec
 TEST_GEN_FILES += gup_longterm
 TEST_GEN_FILES += gup_test
 TEST_GEN_FILES += hmm-tests
diff --git a/tools/testing/selftests/mm/doexec.c b/tools/testing/selftests/mm/doexec.c
new file mode 100644
index 000000000000..6ec09e868679
--- /dev/null
+++ b/tools/testing/selftests/mm/doexec.c
@@ -0,0 +1,774 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Verify that exec preserves the memory backing long-term DMA pins. */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/vhost.h>
+#include <poll.h>
+#include <pthread.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdatomic.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/resource.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "../../../../mm/gup_test.h"
+#include "../kselftest.h"
+
+#ifndef MADV_DOEXEC
+#define MADV_DOEXEC 201
+#endif
+#ifndef MADV_COLLAPSE
+#define MADV_COLLAPSE 25
+#endif
+
+/* The downstream ELF loader requires this opt-in in a PT_NOTE segment. */
+asm(".pushsection .note.doexec,\"a\",@note\n"
+	".balign 4\n"
+	".long 17, 0, 0x07c1feed\n"
+	".asciz \"preserved-mem-ok\"\n"
+	".balign 4\n"
+	".popsection\n");
+
+enum test_kind {
+	BASE,
+	HUGE,
+	LOCKED,
+	PROTECTED,
+	READ_PIN,
+	FAST_PIN,
+	NR_TESTS,
+};
+
+static const char * const names[] = {
+	"base pages",
+	"PMD huge pages",
+	"mlocked pages",
+	"read-only mapping",
+	"read-only pins",
+	"fast GUP pins",
+};
+
+static int wait_child(pid_t pid)
+{
+	int status;
+	pid_t ret;
+
+	do {
+		ret = waitpid(pid, &status, 0);
+	} while (ret < 0 && errno == EINTR);
+	if (ret < 0 || !WIFEXITED(status))
+		return KSFT_FAIL;
+	return WEXITSTATUS(status);
+}
+
+static int check_pin(int fd, char *ram, size_t size, unsigned char value)
+{
+	char *copy = malloc(size);
+	uint64_t addr = (uintptr_t)copy;
+	size_t i;
+	int ret = KSFT_FAIL;
+
+	if (!copy)
+		return ret;
+	if (ioctl(fd, PIN_LONGTERM_TEST_READ, &addr)) {
+		ksft_print_msg("read pin: %s\n", strerror(errno));
+		goto out;
+	}
+	for (i = 0; i < size; i++) {
+		if ((unsigned char)ram[i] != value ||
+		    (unsigned char)copy[i] != value) {
+			ksft_print_msg("CPU/pin mismatch at byte %zu\n", i);
+			goto out;
+		}
+	}
+	ret = KSFT_PASS;
+out:
+	free(copy);
+	return ret;
+}
+
+static int check_bytes(char *ram, size_t size, unsigned char value)
+{
+	size_t i;
+
+	for (i = 0; i < size; i++)
+		if ((unsigned char)ram[i] != value)
+			return KSFT_FAIL;
+	return KSFT_PASS;
+}
+
+static int after_cow_exec(char *ram, size_t size, pid_t sibling, int pipe_fd)
+{
+	char ready = 1;
+	int ret = check_bytes(ram, size, 0x31);
+
+	memset(ram, 0x42, size);
+	if (write(pipe_fd, &ready, 1) != 1)
+		ret = KSFT_FAIL;
+	close(pipe_fd);
+	if (wait_child(sibling))
+		ret = KSFT_FAIL;
+	return ret;
+}
+
+/* Pages shared with a separate mm must retain their pre-existing COW state. */
+static int before_cow_exec(void)
+{
+	size_t size = 16 * sysconf(_SC_PAGESIZE);
+	char addr_arg[32], size_arg[32], pid_arg[32], fd_arg[32], ready;
+	int pipe_fd[2];
+	char *ram;
+	pid_t pid;
+
+	ram = mmap(NULL, size, PROT_READ | PROT_WRITE,
+		   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (ram == MAP_FAILED)
+		return KSFT_FAIL;
+	memset(ram, 0x31, size);
+	if (madvise(ram, size, MADV_DOEXEC))
+		return errno == EINVAL ? KSFT_SKIP : KSFT_FAIL;
+	if (pipe(pipe_fd))
+		return KSFT_FAIL;
+	pid = fork();
+	if (pid < 0)
+		return KSFT_FAIL;
+	if (!pid) {
+		close(pipe_fd[1]);
+		if (read(pipe_fd[0], &ready, 1) != 1)
+			_exit(KSFT_FAIL);
+		_exit(check_bytes(ram, size, 0x31));
+	}
+	close(pipe_fd[0]);
+	snprintf(addr_arg, sizeof(addr_arg), "%llu",
+		 (unsigned long long)(uintptr_t)ram);
+	snprintf(size_arg, sizeof(size_arg), "%zu", size);
+	snprintf(pid_arg, sizeof(pid_arg), "%d", pid);
+	snprintf(fd_arg, sizeof(fd_arg), "%d", pipe_fd[1]);
+	execl("/proc/self/exe", "doexec", "--cow", addr_arg, size_arg,
+	      pid_arg, fd_arg, NULL);
+	close(pipe_fd[1]);
+	wait_child(pid);
+	return KSFT_FAIL;
+}
+
+static void exec_stage(int fd, char *ram, size_t size, int kind, int stage)
+{
+	char fd_arg[32], addr_arg[32], size_arg[32], kind_arg[32], stage_arg[32];
+
+	snprintf(fd_arg, sizeof(fd_arg), "%d", fd);
+	snprintf(addr_arg, sizeof(addr_arg), "%llu",
+		 (unsigned long long)(uintptr_t)ram);
+	snprintf(size_arg, sizeof(size_arg), "%zu", size);
+	snprintf(kind_arg, sizeof(kind_arg), "%d", kind);
+	snprintf(stage_arg, sizeof(stage_arg), "%d", stage);
+	execl("/proc/self/exe", "doexec", "--exec", fd_arg, addr_arg, size_arg,
+	      kind_arg, stage_arg, NULL);
+	ksft_print_msg("exec: %s\n", strerror(errno));
+	_exit(KSFT_FAIL);
+}
+
+static int after_exec(int fd, char *ram, size_t size, int kind, int stage)
+{
+	unsigned char before = stage == 1 ? 0x31 : 0x42;
+	unsigned char after = stage == 1 ? 0x42 : 0x53;
+	pid_t pid;
+
+	if (check_pin(fd, ram, size, before))
+		return KSFT_FAIL;
+	if (kind == PROTECTED && mprotect(ram, size, PROT_READ | PROT_WRITE))
+		return KSFT_FAIL;
+	memset(ram, after, size);
+	if (check_pin(fd, ram, size, after))
+		return KSFT_FAIL;
+	if (stage == 1) {
+		/* VM_EXEC_KEEP is consumed, so mark again for the second exec. */
+		if (madvise(ram, size, MADV_DOEXEC))
+			return KSFT_FAIL;
+		if (kind == PROTECTED && mprotect(ram, size, PROT_READ))
+			return KSFT_FAIL;
+		exec_stage(fd, ram, size, kind, 2);
+	}
+
+	/* Fork must copy into the child and retain the parent's pinned pages. */
+	pid = fork();
+	if (pid < 0)
+		return KSFT_FAIL;
+	if (!pid) {
+		memset(ram, 0x64, size);
+		_exit(KSFT_PASS);
+	}
+	if (wait_child(pid) || check_pin(fd, ram, size, after))
+		return KSFT_FAIL;
+	memset(ram, 0x75, size);
+	if (check_pin(fd, ram, size, 0x75))
+		return KSFT_FAIL;
+	return ioctl(fd, PIN_LONGTERM_TEST_STOP) ? KSFT_FAIL : KSFT_PASS;
+}
+
+static size_t huge_page_size(void)
+{
+	FILE *file = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r");
+	size_t size = 0;
+
+	if (file) {
+		if (fscanf(file, "%zu", &size) != 1)
+			size = 0;
+		fclose(file);
+	}
+	return size;
+}
+
+static int before_exec(int kind)
+{
+	size_t size = 16 * sysconf(_SC_PAGESIZE);
+	struct pin_longterm_test pin;
+	char *ram, *mapping;
+	uintptr_t aligned;
+	int fd;
+
+	if (kind == HUGE) {
+		size = huge_page_size();
+		if (!size)
+			return KSFT_SKIP;
+	}
+	fd = open("/sys/kernel/debug/gup_test", O_RDWR);
+	if (fd < 0)
+		return KSFT_SKIP;
+	mapping = mmap(NULL, 2 * size, PROT_READ | PROT_WRITE,
+		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (mapping == MAP_FAILED)
+		return KSFT_FAIL;
+	aligned = ((uintptr_t)mapping + size - 1) & ~(size - 1);
+	ram = (char *)aligned;
+	if (ram != mapping)
+		munmap(mapping, ram - mapping);
+	if (ram + size != mapping + 2 * size)
+		munmap(ram + size, mapping + 2 * size - (ram + size));
+	if (madvise(ram, size, kind == HUGE ? MADV_HUGEPAGE : MADV_NOHUGEPAGE))
+		return KSFT_FAIL;
+	memset(ram, 0x31, size);
+	if (kind == HUGE && madvise(ram, size, MADV_COLLAPSE))
+		return KSFT_SKIP;
+	if (kind == LOCKED && mlock(ram, size))
+		return KSFT_SKIP;
+	if (madvise(ram, size, MADV_DOEXEC)) {
+		ksft_print_msg("MADV_DOEXEC: %s\n", strerror(errno));
+		return errno == EINVAL ? KSFT_SKIP : KSFT_FAIL;
+	}
+	pin = (struct pin_longterm_test) {
+		.addr = (uintptr_t)ram,
+		.size = size,
+		.flags = kind == READ_PIN ? 0 : PIN_LONGTERM_TEST_FLAG_USE_WRITE,
+	};
+	if (kind == FAST_PIN)
+		pin.flags |= PIN_LONGTERM_TEST_FLAG_USE_FAST;
+	if (ioctl(fd, PIN_LONGTERM_TEST_START, &pin)) {
+		ksft_print_msg("pin: %s\n", strerror(errno));
+		return errno == ENOTTY ? KSFT_SKIP : KSFT_FAIL;
+	}
+	if (kind == PROTECTED && mprotect(ram, size, PROT_READ))
+		return KSFT_FAIL;
+	exec_stage(fd, ram, size, kind, 1);
+	return KSFT_FAIL;
+}
+
+static int check_exec(char *ram, size_t size, size_t stride, bool reject)
+{
+	char addr_arg[32], size_arg[32], stride_arg[32];
+
+	snprintf(addr_arg, sizeof(addr_arg), "%llu",
+		 (unsigned long long)(uintptr_t)ram);
+	snprintf(size_arg, sizeof(size_arg), "%zu", size);
+	snprintf(stride_arg, sizeof(stride_arg), "%zu", stride);
+	execl("/proc/self/exe", "doexec", reject ? "--reject" : "--check", addr_arg, size_arg,
+	      stride_arg, NULL);
+	return errno;
+}
+
+static char *preserved_ram(size_t size)
+{
+	char *ram = mmap(NULL, size, PROT_READ | PROT_WRITE,
+			 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+	if (ram == MAP_FAILED)
+		return NULL;
+	memset(ram, 0x31, size);
+	if (madvise(ram, size, MADV_DOEXEC)) {
+		munmap(ram, size);
+		return NULL;
+	}
+	return ram;
+}
+
+static int blocked_task(void *arg)
+{
+	int fd = *(int *)arg;
+	char ready;
+
+	return read(fd, &ready, 1) == 1 ? KSFT_PASS : KSFT_FAIL;
+}
+
+static void *blocked_thread(void *arg)
+{
+	blocked_task(arg);
+	return NULL;
+}
+
+/* Own threads may be killed by exec; a separate CLONE_VM task may not. */
+static int shared_mm_exec(bool own_thread)
+{
+	size_t size = sysconf(_SC_PAGESIZE);
+	char *ram = preserved_ram(size), *stack;
+	int pipe_fd[2], ret;
+	pthread_t thread;
+	pid_t pid;
+
+	if (!ram)
+		return errno == EINVAL ? KSFT_SKIP : KSFT_FAIL;
+	if (pipe(pipe_fd))
+		return KSFT_FAIL;
+	if (own_thread) {
+		if (pthread_create(&thread, NULL, blocked_thread, &pipe_fd[0]))
+			return KSFT_FAIL;
+		check_exec(ram, size, 1, false);
+		write(pipe_fd[1], "x", 1);
+		pthread_join(thread, NULL);
+		return KSFT_FAIL;
+	}
+	stack = mmap(NULL, 65536, PROT_READ | PROT_WRITE,
+		     MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+	if (stack == MAP_FAILED)
+		return KSFT_FAIL;
+	pid = clone(blocked_task, stack + 65536, CLONE_VM | SIGCHLD, &pipe_fd[0]);
+	if (pid < 0)
+		return KSFT_FAIL;
+	ret = check_exec(ram, size, 1, true);
+	/* Exec must return EBUSY with the old mapping intact. */
+	if (ret != EBUSY || check_bytes(ram, size, 0x31))
+		ret = KSFT_FAIL;
+	else
+		ret = KSFT_PASS;
+	write(pipe_fd[1], "x", 1);
+	if (wait_child(pid))
+		ret = KSFT_FAIL;
+	if (ret)
+		return ret;
+	/* Releasing the outside user must allow a retry. */
+	check_exec(ram, size, 1, false);
+	return KSFT_FAIL;
+}
+
+/* The caller must release its backend owner before preserving exec. */
+static int vhost_owner_exec(void)
+{
+	size_t size = sysconf(_SC_PAGESIZE);
+	char *ram;
+	int fd = open("/dev/vhost-net", O_RDWR);
+
+	if (fd < 0)
+		return KSFT_SKIP;
+	ram = preserved_ram(size);
+	if (!ram)
+		return errno == EINVAL ? KSFT_SKIP : KSFT_FAIL;
+	if (ioctl(fd, VHOST_SET_OWNER))
+		return KSFT_SKIP;
+	if (ioctl(fd, VHOST_RESET_OWNER))
+		return KSFT_FAIL;
+	close(fd);
+	check_exec(ram, size, 1, false);
+	return KSFT_FAIL;
+}
+
+static void *exec_thread(void *arg)
+{
+	check_exec(arg, sysconf(_SC_PAGESIZE), 1, false);
+	return NULL;
+}
+
+static int nonleader_exec(void)
+{
+	char *ram = preserved_ram(sysconf(_SC_PAGESIZE));
+	pthread_t thread;
+
+	if (!ram)
+		return errno == EINVAL ? KSFT_SKIP : KSFT_FAIL;
+	if (pthread_create(&thread, NULL, exec_thread, ram))
+		return KSFT_FAIL;
+	pthread_join(thread, NULL);
+	return KSFT_FAIL;
+}
+
+static int proc_exec(char *ram, size_t size, int stage)
+{
+	char addr_arg[32], size_arg[32], stage_arg[32];
+
+	if (check_bytes(ram, size, 0x31))
+		return KSFT_FAIL;
+	if (stage == 32)
+		return KSFT_PASS;
+	if (madvise(ram, size, MADV_DOEXEC))
+		return KSFT_FAIL;
+	snprintf(addr_arg, sizeof(addr_arg), "%llu",
+		 (unsigned long long)(uintptr_t)ram);
+	snprintf(size_arg, sizeof(size_arg), "%zu", size);
+	snprintf(stage_arg, sizeof(stage_arg), "%d", stage + 1);
+	execl("/proc/self/exe", "doexec", "--proc", addr_arg, size_arg,
+	      stage_arg, NULL);
+	return KSFT_FAIL;
+}
+
+/* Read both pre-opened and freshly opened proc files during repeated exec. */
+static int proc_readers_exec(void)
+{
+	static const char * const files[] = { "maps", "smaps_rollup", "pagemap" };
+	int ready[2], go[2], fds[3] = { -1, -1, -1 }, status, ret = KSFT_FAIL;
+	char paths[3][80], buffer[4096];
+	unsigned int round = 0;
+	pid_t pid, waited;
+	int i;
+
+	if (pipe(ready) || pipe(go))
+		return KSFT_FAIL;
+	pid = fork();
+	if (pid < 0)
+		return KSFT_FAIL;
+	if (!pid) {
+		size_t size = 16 * sysconf(_SC_PAGESIZE);
+		char *ram = preserved_ram(size);
+
+		close(ready[0]);
+		close(go[1]);
+		if (!ram)
+			_exit(errno == EINVAL ? KSFT_SKIP : KSFT_FAIL);
+		if (write(ready[1], "x", 1) != 1 || read(go[0], buffer, 1) != 1)
+			_exit(KSFT_FAIL);
+		close(ready[1]);
+		close(go[0]);
+		_exit(proc_exec(ram, size, 0));
+	}
+	close(ready[1]);
+	close(go[0]);
+	if (read(ready[0], buffer, 1) != 1) {
+		close(ready[0]);
+		close(go[1]);
+		return wait_child(pid);
+	}
+	close(ready[0]);
+	for (i = 0; i < 3; i++) {
+		snprintf(paths[i], sizeof(paths[i]), "/proc/%d/%s", pid, files[i]);
+		fds[i] = open(paths[i], O_RDONLY);
+		if (fds[i] < 0) {
+			ret = KSFT_SKIP;
+			goto stop;
+		}
+	}
+	if (write(go[1], "x", 1) != 1)
+		goto stop;
+	close(go[1]);
+	go[1] = -1;
+	for (;;) {
+		waited = waitpid(pid, &status, WNOHANG);
+		if (waited == pid) {
+			ret = WIFEXITED(status) ? WEXITSTATUS(status) : KSFT_FAIL;
+			break;
+		}
+		if (waited < 0 && errno != EINTR)
+			break;
+		for (i = 0; i < 3; i++) {
+			lseek(fds[i], 0, SEEK_SET);
+			if (read(fds[i], buffer, sizeof(buffer)) < 0 &&
+			    errno != EINTR && errno != ESRCH)
+				goto stop;
+		}
+		/* Keep two old handles while refreshing the third. */
+		i = round++ % 3;
+		close(fds[i]);
+		fds[i] = open(paths[i], O_RDONLY);
+		if (fds[i] < 0) {
+			ret = wait_child(pid);
+			break;
+		}
+	}
+	for (i = 0; i < 3; i++)
+		if (fds[i] >= 0)
+			close(fds[i]);
+	return ret;
+stop:
+	if (go[1] >= 0)
+		close(go[1]);
+	kill(pid, SIGKILL);
+	wait_child(pid);
+	for (i = 0; i < 3; i++)
+		if (fds[i] >= 0)
+			close(fds[i]);
+	return ret;
+}
+
+struct clone_race {
+	char *ram;
+	int report;
+	atomic_bool stop;
+	atomic_uint rounds;
+};
+
+static int clone_race_child(void *arg)
+{
+	struct clone_race *race = arg;
+	unsigned int i;
+
+	for (i = 0; i < 8; i++) {
+		if (__atomic_load_n(race->ram, __ATOMIC_RELAXED) != 0x31) {
+			write(race->report, "x", 1);
+			return KSFT_FAIL;
+		}
+		usleep(1000);
+	}
+	return KSFT_PASS;
+}
+
+static void *clone_race_thread(void *arg)
+{
+	struct clone_race *race = arg;
+	char *stack = mmap(NULL, 65536, PROT_READ | PROT_WRITE,
+			   MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+	pid_t pid;
+
+	if (stack == MAP_FAILED) {
+		write(race->report, "x", 1);
+		atomic_store(&race->rounds, 4);
+		return NULL;
+	}
+	while (!atomic_load(&race->stop)) {
+		pid = clone(clone_race_child, stack + 65536,
+			    CLONE_VM | SIGCHLD, race);
+		if (pid >= 0) {
+			if (wait_child(pid))
+				write(race->report, "x", 1);
+		} else if (errno != EBUSY && errno != EINTR) {
+			write(race->report, "x", 1);
+		}
+		atomic_fetch_add(&race->rounds, 1);
+		usleep(1000);
+	}
+	munmap(stack, 65536);
+	return NULL;
+}
+
+/* A supervisor collects errors from any shared-mm child surviving exec. */
+static int clone_exec_race(void)
+{
+	int report[2], ret;
+	struct pollfd pfd;
+	char failed;
+	pid_t pid;
+
+	if (pipe(report))
+		return KSFT_FAIL;
+	pid = fork();
+	if (pid < 0)
+		return KSFT_FAIL;
+	if (!pid) {
+		size_t size = sysconf(_SC_PAGESIZE);
+		struct clone_race race = { .report = report[1] };
+		pthread_t thread;
+		unsigned int attempt;
+
+		close(report[0]);
+		race.ram = preserved_ram(size);
+		if (!race.ram)
+			_exit(errno == EINVAL ? KSFT_SKIP : KSFT_FAIL);
+		if (pthread_create(&thread, NULL, clone_race_thread, &race))
+			_exit(KSFT_FAIL);
+		while (atomic_load(&race.rounds) < 4)
+			usleep(1000);
+		for (attempt = 0; attempt < 500; attempt++) {
+			if (check_exec(race.ram, size, 1, false) != EBUSY)
+				break;
+			if (check_bytes(race.ram, size, 0x31))
+				break;
+			usleep(1000);
+		}
+		atomic_store(&race.stop, true);
+		pthread_join(thread, NULL);
+		_exit(KSFT_FAIL);
+	}
+	close(report[1]);
+	ret = wait_child(pid);
+	pfd = (struct pollfd) { .fd = report[0], .events = POLLIN | POLLHUP };
+	if (poll(&pfd, 1, 3000) <= 0 || read(report[0], &failed, 1) != 0)
+		ret = KSFT_FAIL;
+	close(report[0]);
+	return ret;
+}
+
+/* Opt-in sweep of allocation failures, using only the child's fail-nth. */
+static int allocation_failures(void)
+{
+	const size_t size = 256ul << 20, stride = 2ul << 20;
+	struct rlimit no_core = { 0, 0 };
+	unsigned int early = 0, fatal = 0, success = 0, loader = 0;
+	unsigned int nth;
+	pid_t pid;
+	int status;
+	pid_t waited;
+
+	if (!getenv("DOEXEC_FAULT_INJECTION") || access("/proc/self/fail-nth", W_OK))
+		return KSFT_SKIP;
+	for (nth = 1; nth <= 256; nth++) {
+		pid = fork();
+		if (pid < 0)
+			return KSFT_FAIL;
+		if (!pid) {
+			char *ram, number[32];
+			size_t i;
+			int fd, len, err;
+
+			setrlimit(RLIMIT_CORE, &no_core);
+			ram = mmap(NULL, size, PROT_READ | PROT_WRITE,
+				   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+			if (ram == MAP_FAILED || madvise(ram, size, MADV_NOHUGEPAGE))
+				_exit(KSFT_FAIL);
+			for (i = 0; i < size; i += stride)
+				ram[i] = 0x31;
+			if (madvise(ram, size, MADV_DOEXEC))
+				_exit(errno == EINVAL ? KSFT_SKIP : KSFT_FAIL);
+			fd = open("/proc/self/fail-nth", O_WRONLY | O_CLOEXEC);
+			len = snprintf(number, sizeof(number), "%u", nth);
+			if (fd < 0 || write(fd, number, len) != len)
+				_exit(KSFT_FAIL);
+			err = check_exec(ram, size, stride, false);
+			/* Disarm before checking mappings or exiting. */
+			pwrite(fd, "0", 1, 0);
+			for (i = 0; i < size; i += stride)
+				if (ram[i] != 0x31)
+					_exit(KSFT_FAIL);
+			_exit(err == ENOMEM || err == EFAULT || err == EAGAIN ? 10 : KSFT_FAIL);
+		}
+		do {
+			waited = waitpid(pid, &status, 0);
+		} while (waited < 0 && errno == EINTR);
+		if (waited < 0)
+			return KSFT_FAIL;
+		if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
+			fatal++;
+		else if (WIFEXITED(status) && WEXITSTATUS(status) == 10)
+			early++;
+		else if (WIFEXITED(status) && WEXITSTATUS(status) == KSFT_PASS)
+			success++;
+		/* A dynamic loader may exit when a post-exec allocation fails. */
+		else if (WIFEXITED(status) && WEXITSTATUS(status) == 127)
+			loader++;
+		else if (WIFEXITED(status) && WEXITSTATUS(status) == KSFT_SKIP)
+			return KSFT_SKIP;
+		else
+			return KSFT_FAIL;
+	}
+	ksft_print_msg("faults: %u recoverable, %u SIGSEGV, %u success, %u loader exits\n",
+		       early, fatal, success, loader);
+	/* A sweep that injected no recoverable/fatal errors provides no coverage. */
+	return early && fatal ? KSFT_PASS : KSFT_SKIP;
+}
+
+int main(int argc, char **argv)
+{
+	pid_t pid;
+	int kind, ret;
+
+	if (argc == 7 && !strcmp(argv[1], "--exec"))
+		return after_exec(atoi(argv[2]),
+			(char *)(uintptr_t)strtoull(argv[3], NULL, 10),
+			strtoull(argv[4], NULL, 10), atoi(argv[5]), atoi(argv[6]));
+	if (argc == 6 && !strcmp(argv[1], "--cow"))
+		return after_cow_exec((char *)(uintptr_t)strtoull(argv[2], NULL, 10),
+			strtoull(argv[3], NULL, 10), atoi(argv[4]), atoi(argv[5]));
+	if (argc == 5 && !strcmp(argv[1], "--reject"))
+		return KSFT_FAIL;
+	if (argc == 5 && !strcmp(argv[1], "--proc"))
+		return proc_exec((char *)(uintptr_t)strtoull(argv[2], NULL, 10),
+				 strtoull(argv[3], NULL, 10), atoi(argv[4]));
+	if (argc == 5 && !strcmp(argv[1], "--check")) {
+		char *ram = (char *)(uintptr_t)strtoull(argv[2], NULL, 10);
+		size_t size = strtoull(argv[3], NULL, 10);
+		size_t stride = strtoull(argv[4], NULL, 10), i;
+		int fd = open("/proc/self/fail-nth", O_WRONLY);
+
+		if (fd >= 0) {
+			write(fd, "0", 1);
+			close(fd);
+		}
+		for (i = 0; i < size; i += stride)
+			if (ram[i] != 0x31)
+				return KSFT_FAIL;
+		return KSFT_PASS;
+	}
+	ksft_print_header();
+	ksft_set_plan(NR_TESTS + 8);
+	for (kind = 0; kind < NR_TESTS; kind++) {
+		fflush(stdout);
+		pid = fork();
+		if (!pid)
+			_exit(before_exec(kind));
+		ret = pid < 0 ? KSFT_FAIL : wait_child(pid);
+		if (ret == KSFT_SKIP)
+			ksft_test_result_skip("%s: DOEXEC, GUP test or mapping unavailable\n",
+					      names[kind]);
+		else
+			ksft_test_result(ret == KSFT_PASS, "%s: repeated exec and fork\n",
+					 names[kind]);
+	}
+	fflush(stdout);
+	pid = fork();
+	if (!pid)
+		_exit(before_cow_exec());
+	ret = pid < 0 ? KSFT_FAIL : wait_child(pid);
+	if (ret == KSFT_SKIP)
+		ksft_test_result_skip("pre-existing COW: DOEXEC unavailable\n");
+	else
+		ksft_test_result(ret == KSFT_PASS, "pre-existing COW preserved\n");
+	for (kind = 0; kind < 7; kind++) {
+		static const char * const extra_names[] = {
+			"CLONE_VM: recoverable EBUSY and retry",
+			"own threads: preserving exec allowed",
+			"vhost owner: reset before preserving exec",
+			"allocation failure sweep (opt-in)",
+			"non-leader thread: preserving exec allowed",
+			"proc readers: repeated preserving exec allowed",
+			"concurrent CLONE_VM: shared memory is not stolen",
+		};
+
+		fflush(stdout);
+		pid = fork();
+		if (!pid) {
+			switch (kind) {
+			case 0:
+			case 1:
+				_exit(shared_mm_exec(kind));
+			case 2:
+				_exit(vhost_owner_exec());
+			case 3:
+				_exit(allocation_failures());
+			case 4:
+				_exit(nonleader_exec());
+			case 5:
+				_exit(proc_readers_exec());
+			case 6:
+				_exit(clone_exec_race());
+			}
+		}
+		ret = pid < 0 ? KSFT_FAIL : wait_child(pid);
+		if (ret == KSFT_SKIP)
+			ksft_test_result_skip("%s: unavailable or disabled\n", extra_names[kind]);
+		else
+			ksft_test_result(ret == KSFT_PASS, "%s\n", extra_names[kind]);
+	}
+	ksft_finished();
+}
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index 6ac8ba7e2447..9643b39762b8 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -296,6 +296,7 @@ fi
 # Dump pages 0, 19, and 4096, using pin_user_pages:
 CATEGORY="gup_test" run_test ./gup_test -ct -F 0x1 0 19 0x1000
 CATEGORY="gup_test" run_test ./gup_longterm
+CATEGORY="gup_test" run_test ./doexec
 
 CATEGORY="userfaultfd" run_test ./uffd-unit-tests
 uffd_stress_bin=./uffd-stress

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH RFC PATCH hci-8.1 2/4] exec: protect preserved memory against surviving shared-mm tasks

[no-reply]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] exec: reserve_preserved_mem() fails execve() with -EBUSY for vfork/CLONE_VM parents
- [Medium] fork: CLONE_VM clones, including sibling pthread_create(), fail with -EBUSY during reservation
- [Medium] exec: stale VM_EXEC_KEEP snapshot in reserve_preserved_mem() drops later-marked VMAs
- [Medium] exec: preserved-mm reservation misses non-task mm users like kthread_use_mm()
--

commit 0203d4ae0ee568e8d416b284444f0842edfb59ae
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

exec: protect preserved memory against surviving shared-mm tasks

This patch reserves the mm holding VM_EXEC_KEEP VMAs before exec becomes
irreversible. It rejects tasks outside the execing thread group that share
the mm, and tracks in-flight CLONE_VM clones through a counter in
mm_struct. New shared-mm clones are refused while the reservation is held.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-103860

> diff --git a/fs/exec.c b/fs/exec.c
> --- a/fs/exec.c
> +++ b/fs/exec.c

[ ... ]

> +static int reserve_preserved_mem(struct linux_binprm *bprm)
> +{

[ ... ]

> +	mmap_read_unlock(mm);
> +	if (!preserve)
> +		return 0;

[Severity: Medium]
Can a VMA marked after this scan be silently dropped?

Sibling threads of the execing group are exempt from admission and keep
running until de_thread().  If one of them calls madvise(MADV_DOEXEC) after
this scan returned 0 with preserving_mem unset, exec_mmap() no longer
re-evaluates the VMAs, so vma_dup_some() is skipped and the newly marked
VMA is not preserved.

Before this patch, vma_dup_some() ran after de_thread() under
mmap_read_lock, so the set of marked VMAs was stable at transfer time.
Should the decision be re-checked after de_thread()?

> +	/* A clone is either counted here or already visible in the task list. */
> +	spin_lock(&mm->page_table_lock);
> +	if (mm->exec_keep_active || mm->exec_keep_clones)
> +		ret = -EBUSY;

[Severity: Medium]
Can an in-flight pthread_create() from a sibling thread make the whole
execve() fail here?

If another thread of the execing group is inside copy_mm() between
exec_keep_clone_begin() and exec_keep_clone_end(), exec_keep_clones is
non-zero and reserve_preserved_mem() returns -EBUSY with no retry.  Such
siblings are about to be removed by de_thread() anyway.

> +	else
> +		mm->exec_keep_active = true;
> +	spin_unlock(&mm->page_table_lock);
> +	if (ret)
> +		return ret;
> +	bprm->preserving_mem = true;
> +
> +	read_lock(&tasklist_lock);
> +	for_each_process_thread(p, t) {
> +		if (same_thread_group(current, t))
> +			continue;
> +		task_lock(t);
> +		if (t->mm == mm)
> +			ret = -EBUSY;

[Severity: Medium]
Does this turn a valid vfork() or posix_spawn() exec into a failure?

If a process has a VM_EXEC_KEEP VMA and calls vfork() or
clone(CLONE_VM|CLONE_VFORK), the child shares the parent's mm.  When the
child calls execve() on a binary with accepts_preserved_mem set, the
suspended parent is in a different thread group and has t->mm == mm, so
reserve_preserved_mem() returns -EBUSY.  Before this patch that exec
succeeded.

Is failing the entire execve() with an -EBUSY that userspace does not expect
the intended behavior here, rather than exec without preservation?

Separately, this check is a point-in-time scan of t->mm.  Can it miss other
users of the mm that are not in a task's ->mm field, such as a kthread
holding the mm via kthread_use_mm() on a stored mm, or any holder of an
mmget() reference?  exec_keep_active only gates CLONE_VM in copy_mm(), and
mm_users is not examined.

> +		task_unlock(t);
> +		if (ret)
> +			goto out;
> +	}
> +out:
> +	read_unlock(&tasklist_lock);
> +	if (ret)
> +		release_preserved_mem(bprm, mm);
> +	return ret;
> +}

[ ... ]

> @@ -1067,6 +1140,7 @@ static int exec_mmap(struct linux_binprm *bprm)
>  		BUG_ON(active_mm != old_mm);
>  		setmax_mm_hiwater_rss(&tsk->signal->maxrss, old_mm);
>  		mm_update_next_owner(old_mm);
> +		release_preserved_mem(bprm, old_mm);
>  		mmput(old_mm);

[Severity: Medium]
Can an mm user that the reservation never saw keep old_mm alive after this
point?

By the time the reservation is released here, the VM_EXEC_KEEP VMAs have
already been moved to the new mm by vma_dup_some().  A holder such as one
using kthread_use_mm() on a stored mm would then see empty, newly faulted
pages in the old VMAs instead of the preserved data.

The commit message says other tasks using the old mm are quiesced, but this
appears to be enforced only for tasks on the task list.  Is there a check
that mm_users is no longer greater than one after de_thread()?

[ ... ]

> diff --git a/kernel/fork.c b/kernel/fork.c
> --- a/kernel/fork.c
> +++ b/kernel/fork.c

[ ... ]

> +static int exec_keep_clone_begin(struct mm_struct *mm)
> +{
> +	int ret = 0;
> +
> +	spin_lock(&mm->page_table_lock);
> +	if (mm->exec_keep_active)
> +		ret = -EBUSY;

[Severity: Medium]
Is it intended that every CLONE_VM clone is rejected while exec_keep_active
is set, including CLONE_THREAD clones from sibling threads of the execing
group?

The reservation window runs from reserve_preserved_mem() until exec_mmap()
or free_bprm(), and de_thread() can wait for siblings to exit inside that
window.  During that time a sibling's pthread_create() fails with -EBUSY
even though that sibling is about to be killed.

Also, exec_keep_active is checked only in copy_mm(), so other paths that
attach an existing mm to a task or kthread are not gated.

> +	else
> +		mm->exec_keep_clones++;
> +	spin_unlock(&mm->page_table_lock);
> +	return ret;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261009-vfio_v1-v1-0-1901cd6c3c74@virtuozzo.com?part=2

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH RFC PATCH hci-8.1 1/4] mm: support moving anonymous page tables between address spaces

[no-reply]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mm: cross-mm move forces rmap locks, serializing padata workers on one anon_vma
- [Low] mm: padata workers in move_page_range_exec() allocate page tables outside exec task memcg
--

commit 8261a13046a4e58b21e6c687a520d3921507b391
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

mm: support moving anonymous page tables between address spaces

This teaches move_page_tables() and move_huge_pmd() to use the destination
mm for page-table operations and to transfer RSS, swap-list and huge-page
table accounting. Cross-mm moves are restricted to same-address anonymous
DOEXEC VMAs. It also adds a parallel exec move wrapper,
move_page_range_exec(), built on padata.

Reference in the commit message:
https://virtuozzo.atlassian.net/browse/VSTOR-103860

> diff --git a/mm/mremap.c b/mm/mremap.c
> --- a/mm/mremap.c
> +++ b/mm/mremap.c

[ ... ]

> @@ -610,10 +646,19 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
>  	struct mmu_notifier_range range;
>  	pmd_t *old_pmd, *new_pmd;
>  	pud_t *old_pud, *new_pud;
> +	bool cross_mm = vma->vm_mm != new_vma->vm_mm;
>  
>  	if (!len)
>  		return 0;
>  
> +	/* Cross-mm moves are only used for same-address anonymous DOEXEC VMAs. */
> +	if (cross_mm) {
> +		if (WARN_ON_ONCE(!vma_is_anonymous(vma) ||
> +				 !(vma->vm_flags & VM_EXEC_KEEP) ||
> +				 old_addr != new_addr || !need_rmap_locks))
> +			return 0;
> +	}

[Severity: Medium]
Does requiring need_rmap_locks for every cross-mm move serialize the padata
workers added below?

move_page_range_chunk() passes need_rmap_locks=true for every chunk:

    return move_page_tables(args->old_vma, addr, args->new_vma, addr,
                            len, true, false) == len ? 0 : -ENOMEM;

For each PMD-sized extent, move_ptes() and move_pgt_entry() then call
take_rmap_locks(vma), which does:

    if (vma->anon_vma)
            anon_vma_lock_write(vma->anon_vma);

All workers operate on the same old VMA, so they all contend on the same
anon_vma write lock, and the PTE-moving loop runs inside that critical
section. Only page-table allocation and cond_resched() can overlap.

Since passing false is rejected by the WARN_ON_ONCE() above, the parallel
path cannot avoid the lock. Does that largely cancel the speedup that
move_page_range_exec() is meant to provide with up to 16 threads?

The commit message does not say why rmap locks are required for cross-mm
moves, or that the workers end up serialized on this lock. Could that be
documented, or could the requirement be relaxed for this case?

[ ... ]

> +static int move_page_range_chunk(unsigned long addr, unsigned long end, void *arg)
> +{
> +	struct move_page_range_args *args = arg;
> +	unsigned long len = end - addr;
> +
> +	return move_page_tables(args->old_vma, addr, args->new_vma, addr,
> +				len, true, false) == len ? 0 : -ENOMEM;
> +}

[ ... ]

> +int move_page_range_exec(struct vm_area_struct *new_vma,
> +			 struct vm_area_struct *old_vma)
> +{

[ ... ]

> +#ifdef CONFIG_PADATA
> +	return padata_do_multithreaded(&job);
> +#else
> +	return move_page_range_chunk(old_vma->vm_start, old_vma->vm_end, &args);
> +#endif
> +}

[Severity: Low]
Are the destination page tables allocated by the padata helpers charged to
the right memcg?

When the job spans more than one chunk, padata_do_multithreaded() queues the
helpers on system_unbound_wq. Each helper then runs:

    padata_mt_helper()
      move_page_range_chunk()
        move_page_tables()
          alloc_new_pud(new_vma->vm_mm, ...)
          alloc_new_pmd(new_vma->vm_mm, ...)
          pte_alloc(new_vma->vm_mm, new_pmd)

These use GFP_PGTABLE_USER, which includes __GFP_ACCOUNT. The charge is
resolved from active_memcg() or current, and here that is the kworker rather
than the exec'ing task.

Neither padata nor this patch calls set_active_memcg() or otherwise attaches
the workers to the submitter's cgroup. Can part of the exec'ing task's
page-table memory therefore escape its memcg limit?

copy_page_range_mt() in mm/memory.c appears to have the same behavior, but
this adds a new padata consumer that relies on it. Should the workers run
under the submitter's memcg, for example by wrapping the chunk function with
set_active_memcg()?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261009-vfio_v1-v1-0-1901cd6c3c74@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH RFC PATCH hci-8.1 4/4] selftests/mm: test memory preservation across exec

[no-reply]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] selftests/mm: vhost_owner_exec() closes the fd, so VHOST_RESET_OWNER is not tested
- [Low] selftests/mm: doexec binary missing from .gitignore
- [Low] selftests/mm: doexec.c ELF note asm uses @note, breaks on ARM
- [Low] selftests/mm: doexec.c ignores write()/pwrite() return values
- [Low] selftests/mm: doexec THP and mlock cases never verify THP or VM_LOCKED after exec
--

commit 7d4cb550ffa3b6f0d72d5c986f22c04da82182ae
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

selftests/mm: test memory preservation across exec

This adds a selftest, doexec, that checks MADV_DOEXEC mappings keep their
backing pages across repeated exec and a later fork. It covers long-term
GUP pins, CLONE_VM rejection and retry, proc readers, vhost owner reset,
and an optional allocation-failure sweep. The test is built through
TEST_GEN_FILES and added to run_vmtests.sh.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-103860

> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -74,6 +74,7 @@ endif
>
>  TEST_GEN_FILES = cow
>  TEST_GEN_FILES += compaction_test
> +TEST_GEN_FILES += doexec

[Severity: Low]
This isn't a bug, but the new generated binary has no entry in
tools/testing/selftests/mm/.gitignore, where siblings such as
gup_longterm are listed.

Should doexec be added there so it does not show up as untracked in git
status after an in-tree build?

>  TEST_GEN_FILES += gup_longterm
>  TEST_GEN_FILES += gup_test
>  TEST_GEN_FILES += hmm-tests

[ ... ]

> diff --git a/tools/testing/selftests/mm/doexec.c b/tools/testing/selftests/mm/doexec.c
> --- /dev/null
> +++ b/tools/testing/selftests/mm/doexec.c

[ ... ]

> +/* The downstream ELF loader requires this opt-in in a PT_NOTE segment. */
> +asm(".pushsection .note.doexec,\"a\",@note\n"
> +	".balign 4\n"

[Severity: Low]
Will this assemble on 32-bit ARM?

GNU as on ARM treats '@' as a comment character, so the section type
there has to be spelled %note. Since the Makefile adds doexec to
TEST_GEN_FILES with no architecture guard, an ARM build of the mm selftests
could either fail or produce a section that is not SHT_NOTE, so the loader
opt-in would not be recognised.

Would a portable spelling, or an architecture guard, be more suitable?

[ ... ]

> +	if (own_thread) {
> +		if (pthread_create(&thread, NULL, blocked_thread, &pipe_fd[0]))
> +			return KSFT_FAIL;
> +		check_exec(ram, size, 1, false);
> +		write(pipe_fd[1], "x", 1);
> +		pthread_join(thread, NULL);
> +		return KSFT_FAIL;
> +	}

[ ... ]

> +	write(pipe_fd[1], "x", 1);
> +	if (wait_child(pid))
> +		ret = KSFT_FAIL;

[Severity: Low]
Several write() and pwrite() return values are ignored: here in
shared_mm_exec(), and also in clone_race_child(), clone_race_thread(), the
--check handler in main(), and allocation_failures(). These will trigger
glibc warn_unused_result warnings.

In shared_mm_exec(), the write() above is what wakes the CLONE_VM task
blocked in read() inside blocked_task(). If it failed, wouldn't the
following wait_child() block forever?

Could the return values be checked?

[ ... ]

> +/* The caller must release its backend owner before preserving exec. */
> +static int vhost_owner_exec(void)
> +{
> +	size_t size = sysconf(_SC_PAGESIZE);
> +	char *ram;
> +	int fd = open("/dev/vhost-net", O_RDWR);
> +
> +	if (fd < 0)
> +		return KSFT_SKIP;
> +	ram = preserved_ram(size);
> +	if (!ram)
> +		return errno == EINVAL ? KSFT_SKIP : KSFT_FAIL;
> +	if (ioctl(fd, VHOST_SET_OWNER))
> +		return KSFT_SKIP;
> +	if (ioctl(fd, VHOST_RESET_OWNER))
> +		return KSFT_FAIL;
> +	close(fd);
> +	check_exec(ram, size, 1, false);
> +	return KSFT_FAIL;
> +}

[Severity: Medium]
Does this test actually exercise VHOST_RESET_OWNER? The fd is closed before
the exec, and close() releases the owner's mm through the same cleanup
path:

vhost_net_release()
  vhost_dev_cleanup(&n->dev)
    vhost_detach_mm()

VHOST_RESET_OWNER reaches the same function:

vhost_net_reset_owner()
  vhost_dev_reset_owner()
    vhost_dev_cleanup()

So the exec sees the same state whether or not the reset ioctl worked, and
a broken reset-owner would go unnoticed, despite the subtest name "vhost
owner: reset before preserving exec".

Could the exec be done with the fd still open after the reset? It would
also be useful to check that the exec is refused while the owner is still
set, so the rejection side is covered too.

[ ... ]

> +static int after_exec(int fd, char *ram, size_t size, int kind, int stage)
> +{
> +	unsigned char before = stage == 1 ? 0x31 : 0x42;
> +	unsigned char after = stage == 1 ? 0x42 : 0x53;
> +	pid_t pid;
> +
> +	if (check_pin(fd, ram, size, before))
> +		return KSFT_FAIL;
> +	if (kind == PROTECTED && mprotect(ram, size, PROT_READ | PROT_WRITE))
> +		return KSFT_FAIL;
> +	memset(ram, after, size);
> +	if (check_pin(fd, ram, size, after))
> +		return KSFT_FAIL;

[ ... ]

> +	if (kind == HUGE && madvise(ram, size, MADV_COLLAPSE))
> +		return KSFT_SKIP;
> +	if (kind == LOCKED && mlock(ram, size))
> +		return KSFT_SKIP;

[Severity: Low]
Do the "PMD huge pages" and "mlocked pages" subtests verify the property
they are named for? The THP collapse and the mlock() are done only in
before_exec(). After exec, after_exec() looks at kind only for PROTECTED,
and otherwise compares bytes and pin contents.

A regression that splits the PMD THP or drops VM_LOCKED during the
preserving exec would still report PASS.

Could after_exec() check AnonHugePages in /proc/self/smaps and VmLck in
/proc/self/status for these two kinds?

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261009-vfio_v1-v1-0-1901cd6c3c74@virtuozzo.com?part=4

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH RFC PATCH hci-8.1 3/4] mm: move MADV_DOEXEC memory instead of copying it

[no-reply]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] mm: vma_dup() copies vm_file without get_file() for private /dev/zero VMAs
- [Low] mm: vma_dup() discards ksm_madvise() flags so VM_MERGEABLE stays on moved VMA
- [Low] mm: padata workers serialize on anon_vma write lock in move_ptes()
--

commit b89406aa304220e73194db3c04662412c6cbfe44
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

mm: move MADV_DOEXEC memory instead of copying it

This commit changes MADV_DOEXEC preservation across exec from a
fork-style copy to a move of the page tables into the new mm. vma_dup()
now clones the anon_vma chain, unmerges KSM, drops mlock state, and calls
move_page_range_exec(). The parallel copy code and the exec-specific
pinned-page exception in the rmap dup path are removed.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-103860

> diff --git a/mm/mmap.c b/mm/mmap.c
> --- a/mm/mmap.c
> +++ b/mm/mmap.c

[ ... ]

>  int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
>  {
>  	struct vm_area_struct *vma;
>  	unsigned long npages;
> +	vm_flags_t flags = old_vma->vm_flags;
>  	int ret = -ENOMEM;

[ ... ]

> +	/* KSM tracks mappings by mm and address; unmerge before moving them. */
> +	ret = ksm_madvise(old_vma, old_vma->vm_start, old_vma->vm_end,
> +			  MADV_UNMERGEABLE, &flags);
> +	if (ret)
> +		return ret;

[Severity: Low]
Is the result of this ksm_madvise() call ever applied?

ksm_madvise() clears VM_MERGEABLE only through its vm_flags output
argument:

mm/ksm.c:ksm_madvise() {
    ...
    *vm_flags &= ~VM_MERGEABLE;
    ...
}

Here that argument is the local variable flags, which is never written
back to old_vma->vm_flags and is not used afterwards.

vm_area_dup() then copies the original vm_flags into the new VMA, and the
later vm_flags_clear() on the new VMA does not include VM_MERGEABLE:

> +	vm_flags_clear(vma, VM_LOCKED_MASK | VM_UFFD_MISSING | VM_UFFD_WP |
> +		       VM_UFFD_MINOR | VM_EXEC_KEEP);

[ ... ]

> +	if (!test_bit(MMF_VM_MERGEABLE, &mm->flags))
> +		ksm_fork(mm, old_vma->vm_mm);
> +	return move_page_range_exec(vma, old_vma);

For a VMA with both MADV_DOEXEC and MADV_MERGEABLE, ksm_fork() registers
the new mm with ksmd while the moved VMA still has VM_MERGEABLE set.
Could ksmd then scan and re-merge pages in the moved VMA?

The commit message says the patch will "unmerge KSM". Is the intent to
leave the moved VMA KSM-eligible, as fork() does, or to disable KSM on it?
If the latter, should VM_MERGEABLE be cleared on the new VMA, for example
by adding it to the vm_flags_clear() call?

> +	/*
> +	 * Transfer the reservation to the linked destination VMA. If moving
> +	 * page tables fails partway through, exec cannot resume the old image:
> +	 * it must terminate and tear down both mms. Each mm then frees its
> +	 * remaining mappings, and the destination releases the reservation.
>  	 */
> -	__vm_flags_mod(old_vma, 0, VM_ACCOUNT);
> +	vm_flags_clear(old_vma, VM_ACCOUNT);

[ ... ]

>  	vma = vm_area_dup(old_vma);
>  	if (!vma)
>  		goto fail_nomem;

[Severity: Critical]
Can this leave two VMAs holding a single reference on vm_file?

vm_area_dup() memcpy()s the VMA, including vm_file, and takes no file
reference. Nothing later in vma_dup() calls get_file() or clears vm_file,
and vma_link() then calls vma_link_file() on the new VMA.

The MADV_DOEXEC gate in madvise_vma_behavior() only requires
vma_is_anonymous() and rejects VM_EXEC, VM_SHARED and VM_STACK. It does
not reject vm_file != NULL. A private mapping of /dev/zero passes that
gate, because mmap_zero() calls vma_set_anonymous() and leaves vm_file
set:

drivers/char/mem.c:mmap_zero() {
    ...
    if (vma->vm_flags & VM_SHARED)
        return shmem_zero_setup(vma);
    vma_set_anonymous(vma);
    return 0;
}

The call chain would then be:

exec_mmap()
  vma_dup_some()
    vma_dup()
      vm_area_dup()    /* copies vm_file, no get_file() */
      vma_link()       /* new VMA now in the file's i_mmap */

Each mm's teardown then reaches remove_vma():

mm/vma.c:remove_vma() {
    ...
    if (vma->vm_file)
        fput(vma->vm_file);
}

That would call fput() twice for one reference. Should vma_dup() either
take get_file() when vma->vm_file is set, or reject VMAs with a vm_file in
the MADV_DOEXEC check?

The old vma_dup() had the same vm_area_dup() path. However, this patch
rewrites the function and builds the new move semantics on it, so it seemed
worth raising here.

> diff --git a/mm/mremap.c b/mm/mremap.c

[ ... ]

> static int move_page_range_chunk(unsigned long addr, unsigned long end, void *arg)
> {
> 	...
> 	return move_page_tables(args->old_vma, addr, args->new_vma, addr,
> 				len, true, false) == len ? 0 : -ENOMEM;
> }

[Severity: Low]
Is the 16-thread padata job in move_page_range_exec() able to scale here?

Every worker calls move_page_tables() with need_rmap_locks set to true.
For each PMD extent, move_ptes() then does:

move_page_range_chunk()
  move_page_tables(..., need_rmap_locks = true, ...)
    move_ptes()
      take_rmap_locks()
        anon_vma_lock_write(vma->anon_vma)

The anon_vma write lock is held across the per-PMD PTE move loop and the
TLB flush, and is dropped only in drop_rmap_locks() at the out label. All
workers operate on the same old_vma, so they take the same anon_vma root
rwsem for write.

Does this mostly serialize the workers, so that padata_do_multithreaded()
with max_threads = 16 gives little speedup over a single thread? I don't
see a correctness problem. Has the performance relative to the old
copy_page_range_mt() path been measured?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261009-vfio_v1-v1-0-1901cd6c3c74@virtuozzo.com?part=3

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel