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

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

[Konstantin Khorenko]

VSTOR-149847: memory preserved across exec with MADV_DOEXEC loses its
connection to long-term pins (VFIO DMA, io_uring fixed buffers) after a
live-update exec, because the copy into the new mm write protects the
pages and the first write in the new image copies them away from the
pins. Applies on top of "mm, binfmt_elf: two fixes also posted to
mainline".
http://lore.virtuozzo.com/20261014-b4-prep-khorenko-vstor-149847-1-upstream-fixes-v1-0-7fde9ffbebe9@virtuozzo.com

Patches 1-3 are Oracle's fixes for that path, taken from UEK8 (uek8/u2)
as they are: the copy runs under the write lock of the old mm with
write_protect_seq held, and preserved PTEs of COW mappings are no longer
write protected.

Patches 4-6 fix what patch 3 leaves:
- 4: the preserved PTE stayed writable in both mms, which
  page_table_check catches on a debug kernel (kernel BUG at
  mm/page_table_check.c). Write protect both copies but keep
  PageAnonExclusive, so the first write in the new image reuses the
  page.
- 5: MMF_HAS_PINNED is carried to the new mm, otherwise a fork in the
  new image treats pinned pages as unpinned and separates them from
  their pins.
- 6: the preserved ranges of the old mm are zapped right after the copy;
  while the old mm lived, a page was exclusive in two mms and
  UFFDIO_MOVE could move its rmap to an anon_vma that is later freed.
  The zap runs on several threads, as the teardown of these ranges
  did before.

Patch 7 (Andrey) refuses a preserving exec while a CLONE_VM task outside
the thread group shares the old mm, and patch 8 is its selftest, which
also covers the pinned cases above. A kernel thread on the mm counts
too: with vhost in kthread mode, the vz10 default, QEMU has to reset the
vhost owner before a preserving exec.

Tested with the whole stack on release and debug kernels: the
MADV_DOEXEC selftests (including the allocation-failure sweep on the
debug kernel), gup_test, a Container start/exec/stop; no BUG, WARNING
or KASAN.

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

---
Andrey Drobyshev (2):
      exec: protect preserved memory against surviving shared-mm tasks
      selftests/mm: test memory preservation across exec

Anthony Yznaga (3):
      oracle/mm: fix VM_BUG_ON hit when preserving memory
      oracle/mm: hold the source mmap write lock when copying PTEs
      oracle/mm: do not write protect COW mappings when preserving across exec

Konstantin Khorenko (3):
      oracle/mm: write protect preserved PTEs but keep them exclusive
      oracle/mm: set MMF_HAS_PINNED on the mm receiving preserved pages
      oracle/mm: zap the preserved ranges of the old mm after copying them

 fs/exec.c                                 | 111 +++-
 include/linux/binfmts.h                   |   4 +-
 include/linux/mm.h                        |   1 +
 include/linux/mm_types.h                  |   4 +
 include/linux/rmap.h                      |   8 +-
 kernel/fork.c                             |  37 ++
 mm/memory.c                               |  93 +++-
 mm/mmap.c                                 |   7 +-
 tools/testing/selftests/mm/.gitignore     |   1 +
 tools/testing/selftests/mm/Makefile       |   1 +
 tools/testing/selftests/mm/doexec.c       | 814 ++++++++++++++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh |   4 +
 12 files changed, 1050 insertions(+), 35 deletions(-)
---
base-commit: 975b3d461d014d75dbc775f44a4758fb41996d09
change-id: 20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-51b0e4b50c5e

Best regards,
--  
Konstantin Khorenko <khorenko@virtuozzo.com>

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

[Devel] [PATCH hci-8.1 1/8] oracle/mm: fix VM_BUG_ON hit when preserving memory

[Konstantin Khorenko]

From: Anthony Yznaga <anthony.yznaga@oracle.com>

copy_page_range_mt() is a stripped down version of copy_page_range().
Unlike copy_page_range(), it is called during exec with the mmap read
lock held on the outgoing mm. Assert that the mmap lock is held, and
hold the write on the write_protect_seq seqcount_t section. The latter
is needed to avoid a VM_BUG_ON in folio_needs_cow_for_dma().

Orabug: 37588319
Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
Reviewed-by: Liam R. Howlett <liam.howlett@oracle.com>
(cherry picked from commit b4c299d320c22ed30b03c7e665389c9870dd9bab)

Taken from Oracle linux-uek branch uek8/u2. Applied cleanly. Same
Orabug as 90339648df65 ("oracle/mm: fix vma iterator misconfiguration
that triggers VM_WARN_ONs"), which we already have; this is the second
half of that pair.

Fixes: d674aa9506ea ("oracle/mm: use padata for copying page ranges in vma_dup()")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 mm/memory.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 5e414c68f3a9c..d4821379529bf 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1489,10 +1489,16 @@ int copy_page_range_mt(struct vm_area_struct *dst_vma, struct vm_area_struct *sr
 		.min_chunk   = max(1ul << 27, PMD_SIZE),
 		.max_threads = 16,
 	};
+	int ret;
 
 	BUG_ON(!(src_vma->vm_flags & VM_EXEC_KEEP));
 
-	return padata_do_multithreaded(&job);
+	mmap_assert_locked(src_vma->vm_mm);
+	raw_write_seqcount_begin(&src_vma->vm_mm->write_protect_seq);
+	ret = padata_do_multithreaded(&job);
+	raw_write_seqcount_end(&src_vma->vm_mm->write_protect_seq);
+
+	return ret;
 }
 #endif /* CONFIG_PADATA */
 

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 1/8] oracle/mm: fix VM_BUG_ON hit when preserving memory

[no-reply]

> From: Anthony Yznaga <anthony.yznaga@oracle.com>
> 
> copy_page_range_mt() is a stripped down version of copy_page_range().
> Unlike copy_page_range(), it is called during exec with the mmap read
> lock held on the outgoing mm. Assert that the mmap lock is held, and
> hold the write on the write_protect_seq seqcount_t section. The latter
> is needed to avoid a VM_BUG_ON in folio_needs_cow_for_dma().
> 
> Orabug: 37588319
> Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
> Reviewed-by: Liam R. Howlett <liam.howlett@oracle.com>
> (cherry picked from commit b4c299d320c22ed30b03c7e665389c9870dd9bab)
> 
> Taken from Oracle linux-uek branch uek8/u2. Applied cleanly. Same
> Orabug as 90339648df65 ("oracle/mm: fix vma iterator misconfiguration
> that triggers VM_WARN_ONs"), which we already have; this is the second
> half of that pair.
> 
> Fixes: d674aa9506ea ("oracle/mm: use padata for copying page ranges in vma_dup()")
> https://virtuozzo.atlassian.net/browse/VSTOR-150231
> 
> Feature: oracle/mm: MADV_DOEXEC madvise() flag
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=1


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

[Devel] [PATCH hci-8.1 2/8] oracle/mm: hold the source mmap write lock when copying PTEs

[Konstantin Khorenko]

From: Anthony Yznaga <anthony.yznaga@oracle.com>

The copy_p*_range() routines assume the mmap write lock is held
for both the source and destination and therefore there will be
no unexpected THP transitions while PTEs are copied (for example,
due to khugepaged). The write lock is already held on the destination
mm, but until now only the read lock was held on the source.

Orabug: 38029049
Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
(cherry picked from commit 285a140316fa44f0cdc6608c109ba525e52d6c3f)

Taken from Oracle linux-uek branch uek8/u2. Applied cleanly; the only
change is a trailing space dropped from the new comment in exec_mmap().

Fixes: 9e6d16a6c0a1 ("oracle/mm: introduce VM_EXEC_KEEP")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/exec.c | 19 ++++++++++++++++---
 mm/mmap.c |  7 ++-----
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index e861a54b491f8..62010a6d63aab 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1028,10 +1028,20 @@ static int exec_mmap(struct linux_binprm *bprm)
 			up_write(&tsk->signal->exec_update_lock);
 			return ret;
 		}
-		if (bprm->accepts_preserved_mem) {
+		if (unlikely(bprm->accepts_preserved_mem)) {
+			/*
+			 * Acquire the write lock to avoid changes to page
+			 * table entries while they are copied.
+			 */
+			mmap_read_unlock(old_mm);
+			ret = mmap_write_lock_killable(old_mm);
+			if (ret) {
+				up_write(&tsk->signal->exec_update_lock);
+				return ret;
+			}
 			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;
 			}
@@ -1063,7 +1073,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 (unlikely(bprm->accepts_preserved_mem))
+			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/mm/mmap.c b/mm/mmap.c
index 320f397816df9..bc508c6bd934b 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1800,12 +1800,9 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 	 * 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..
+	 * vma to the new one.
 	 */
-	__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);

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 2/8] oracle/mm: hold the source mmap write lock when copying PTEs

[no-reply]

> From: Anthony Yznaga <anthony.yznaga@oracle.com>
> 
> The copy_p*_range() routines assume the mmap write lock is held
> for both the source and destination and therefore there will be
> no unexpected THP transitions while PTEs are copied (for example,
> due to khugepaged). The write lock is already held on the destination
> mm, but until now only the read lock was held on the source.
> 
> Orabug: 38029049
> Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
> (cherry picked from commit 285a140316fa44f0cdc6608c109ba525e52d6c3f)
> 
> Taken from Oracle linux-uek branch uek8/u2. Applied cleanly; the only
> change is a trailing space dropped from the new comment in exec_mmap().
> 
> Fixes: 9e6d16a6c0a1 ("oracle/mm: introduce VM_EXEC_KEEP")
> https://virtuozzo.atlassian.net/browse/VSTOR-150231
> 
> Feature: oracle/mm: MADV_DOEXEC madvise() flag
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=2


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

[Devel] [PATCH hci-8.1 3/8] oracle/mm: do not write protect COW mappings when preserving across exec

[Konstantin Khorenko]

From: Anthony Yznaga <anthony.yznaga@oracle.com>

A mapping that is pinned for DMA and preserved across exec can incur
a COW if write protected. The elevated reference count prevents reuse
of the mapped folio.

Orabug: 38029049
Suggested-by: Steve Sistare <steven.sistare@oracle.com>
Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
(cherry picked from commit f2e56cc70e62fe07efb71a1e1c2360a78cba5aff)

Taken from Oracle linux-uek branch uek8/u2. Applied cleanly.

This is the fix for the ticket: a FOLL_LONGTERM-pinned anonymous page
preserved with MADV_DOEXEC was write-protected and lost
PageAnonExclusive in vma_dup(), so the first write in the new image
COWed it and the pin kept the old page. The pinned pages now stay
exclusive and writable in the new mm, and the old mm keeps its own. The
fixes that follow write protect both again and zap the old mm's mappings
right after the copy.

Fixes: 9e6d16a6c0a1 ("oracle/mm: introduce VM_EXEC_KEEP")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 include/linux/rmap.h |  8 ++++----
 mm/memory.c          | 13 ++++++++++---
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 5dc4cd6e4e2c9..01e46123ab2dc 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -431,22 +431,22 @@ static __always_inline int __folio_try_dup_anon_rmap(struct folio *folio,
 		}
 
 		if (!folio_test_large(folio)) {
-			if (PageAnonExclusive(page))
+			if (PageAnonExclusive(page) && !is_exec_keep)
 				ClearPageAnonExclusive(page);
 			atomic_inc(&folio->_mapcount);
 			break;
 		}
 
 		do {
-			if (PageAnonExclusive(page))
+			if (PageAnonExclusive(page) && !is_exec_keep)
 				ClearPageAnonExclusive(page);
 			atomic_inc(&page->_mapcount);
 		} while (page++, --nr_pages > 0);
 		atomic_add(orig_nr_pages, &folio->_large_mapcount);
 		break;
 	case RMAP_LEVEL_PMD:
-		if (PageAnonExclusive(page)) {
-			if (unlikely(maybe_pinned) && likely(!is_exec_keep))
+		if (PageAnonExclusive(page) && !is_exec_keep) {
+			if (unlikely(maybe_pinned))
 				return -EBUSY;
 			ClearPageAnonExclusive(page);
 		}
diff --git a/mm/memory.c b/mm/memory.c
index d4821379529bf..a240246762751 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -956,9 +956,15 @@ static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma,
 		pte_t pte, unsigned long addr, int nr)
 {
 	struct mm_struct *src_mm = src_vma->vm_mm;
+	bool is_exec_keep = !!(dst_vma->vm_flags & VM_EXEC_KEEP);
 
 	/* If it's a COW mapping, write protect it both processes. */
-	if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte)) {
+	/*
+	 * But don't protect it if PTEs are being copied as part of preserving
+	 * memory across exec since the page may be pinned for DMA and must not
+	 * be COW'd.
+	 */
+	if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte) && !is_exec_keep) {
 		wrprotect_ptes(src_mm, addr, src_pte, nr);
 		pte = pte_wrprotect(pte);
 	}
@@ -991,6 +997,7 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
 	bool any_writable;
 	fpb_t flags = 0;
 	int err, nr;
+	bool is_exec_keep = !!(dst_vma->vm_flags & VM_EXEC_KEEP);
 
 	page = vm_normal_page(src_vma, addr, pte);
 	if (unlikely(!page))
@@ -1019,7 +1026,7 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
 				return -EAGAIN;
 			}
 			rss[MM_ANONPAGES] += nr;
-			VM_WARN_ON_FOLIO(PageAnonExclusive(page), folio);
+			VM_WARN_ON_FOLIO(PageAnonExclusive(page) && !is_exec_keep, folio);
 		} else {
 			folio_dup_file_rmap_ptes(folio, page, nr);
 			rss[mm_counter_file(folio)] += nr;
@@ -1047,7 +1054,7 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
 			return err ? err : 1;
 		}
 		rss[MM_ANONPAGES]++;
-		VM_WARN_ON_FOLIO(PageAnonExclusive(page), folio);
+		VM_WARN_ON_FOLIO(PageAnonExclusive(page) && !is_exec_keep, folio);
 	} else {
 		folio_dup_file_rmap_pte(folio, page);
 		rss[mm_counter_file(folio)]++;

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 3/8] oracle/mm: do not write protect COW mappings when preserving across exec

[no-reply]

> From: Anthony Yznaga <anthony.yznaga@oracle.com>
> 
> A mapping that is pinned for DMA and preserved across exec can incur
> a COW if write protected. The elevated reference count prevents reuse
> of the mapped folio.
> 
> Orabug: 38029049
> Suggested-by: Steve Sistare <steven.sistare@oracle.com>
> Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
> (cherry picked from commit f2e56cc70e62fe07efb71a1e1c2360a78cba5aff)
> 
> Taken from Oracle linux-uek branch uek8/u2. Applied cleanly.
> 
> This is the fix for the ticket: a FOLL_LONGTERM-pinned anonymous page
> preserved with MADV_DOEXEC was write-protected and lost
> PageAnonExclusive in vma_dup(), so the first write in the new image
> COWed it and the pin kept the old page. The pinned pages now stay
> exclusive and writable in the new mm, and the old mm keeps its own. The
> fixes that follow write protect both again and zap the old mm's mappings
> right after the copy.
> 
> Fixes: 9e6d16a6c0a1 ("oracle/mm: introduce VM_EXEC_KEEP")
> https://virtuozzo.atlassian.net/browse/VSTOR-150231
> 
> Feature: oracle/mm: MADV_DOEXEC madvise() flag
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=3


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

[Devel] [PATCH hci-8.1 4/8] oracle/mm: write protect preserved PTEs but keep them exclusive

[Konstantin Khorenko]

Commit 37d9f55655ea ("oracle/mm: do not write protect COW mappings when
preserving across exec") leaves the PTE of a preserved anonymous page
writable in both the outgoing and the new mm. With
CONFIG_PAGE_TABLE_CHECK, which refuses to install a writable PTE for an
anonymous page that is mapped already, the first preserving exec panics:

  kernel BUG at mm/page_table_check.c:118!
  RIP: 0010:page_table_check_set+0x484/0x520
   __page_table_check_ptes_set
   copy_present_ptes
   copy_pte_range
   ...
   copy_page_range_mt
   vma_dup
   exec_mmap

The writable PTE is not what prevents the COW of a pinned page: the
same patch keeps PageAnonExclusive set on the preserved page, and
do_wp_page() reuses an exclusive anonymous page instead of copying it.
Write protect the PTE in both mms again, as fork does. The first write
in the new image takes a fault that reuses the page, which is what the
PMD path already does since copy_huge_pmd() write protects both PMDs.
Unlike fork, the copy does not flush the TLB of the old mm after write
protecting it; the zap of the old ranges later in this series does.

Fixes: 37d9f55655ea ("oracle/mm: do not write protect COW mappings when preserving across exec")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 mm/memory.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index a240246762751..1c128162f5cda 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -956,15 +956,13 @@ static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma,
 		pte_t pte, unsigned long addr, int nr)
 {
 	struct mm_struct *src_mm = src_vma->vm_mm;
-	bool is_exec_keep = !!(dst_vma->vm_flags & VM_EXEC_KEEP);
 
-	/* If it's a COW mapping, write protect it both processes. */
 	/*
-	 * But don't protect it if PTEs are being copied as part of preserving
-	 * memory across exec since the page may be pinned for DMA and must not
-	 * be COW'd.
+	 * If it's a COW mapping, write protect it both processes. A page
+	 * preserved across exec keeps PageAnonExclusive, so the new process
+	 * reuses it on the first write instead of copying it.
 	 */
-	if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte) && !is_exec_keep) {
+	if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte)) {
 		wrprotect_ptes(src_mm, addr, src_pte, nr);
 		pte = pte_wrprotect(pte);
 	}

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 4/8] oracle/mm: write protect preserved PTEs but keep them exclusive

[no-reply]

> Commit 37d9f55655ea ("oracle/mm: do not write protect COW mappings when
> preserving across exec") leaves the PTE of a preserved anonymous page
> writable in both the outgoing and the new mm. With
> CONFIG_PAGE_TABLE_CHECK, which refuses to install a writable PTE for an
> anonymous page that is mapped already, the first preserving exec panics:
> 
>   kernel BUG at mm/page_table_check.c:118!
>   RIP: 0010:page_table_check_set+0x484/0x520
>    __page_table_check_ptes_set
>    copy_present_ptes
>    copy_pte_range
>    ...
>    copy_page_range_mt
>    vma_dup
>    exec_mmap
> [ ... ]
> https://virtuozzo.atlassian.net/browse/VSTOR-150231
> 
> Feature: oracle/mm: MADV_DOEXEC madvise() flag
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=4


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

[Devel] [PATCH hci-8.1 5/8] oracle/mm: set MMF_HAS_PINNED on the mm receiving preserved pages

[Konstantin Khorenko]

fork decides whether an anonymous page may be pinned with
folio_needs_cow_for_dma(), which looks at MMF_HAS_PINNED of the parent
mm before folio_maybe_dma_pinned(). Only GUP sets that flag, in the mm
whose pages it pins. Pages preserved across exec keep a long-term pin
taken through the old mm, but the new mm never has the flag, so a fork
in the new image clears PageAnonExclusive of a pinned page and maps it
read-only in both processes. The next write in the parent copies the
page and the device keeps the old one. A debug kernel catches the
shared page when the pin is dropped:

  page dumped because: VM_BUG_ON_PAGE(!PageAnonExclusive(&folio->page))
  kernel BUG at mm/gup.c:66!
  RIP: 0010:sanity_check_pinned_pages+0x237/0x960
   unpin_user_pages
   gup_test_release
   __fput

Set MMF_HAS_PINNED on the new mm when the old one has it.

Fixes: 37d9f55655ea ("oracle/mm: do not write protect COW mappings when preserving across exec")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/exec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index 62010a6d63aab..8677347963e20 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -984,6 +984,10 @@ static int vma_dup_some(struct mm_struct *old_mm, struct mm_struct *new_mm)
 	VMA_ITERATOR(vmi, old_mm, 0);
 	int ret = 0;
 
+	/* Preserved pages may be pinned, fork has to keep copying them. */
+	if (test_bit(MMF_HAS_PINNED, &old_mm->flags))
+		set_bit(MMF_HAS_PINNED, &new_mm->flags);
+
 	mmap_write_lock_nested(new_mm, SINGLE_DEPTH_NESTING);
 	for_each_vma(vmi, vma)
 		if (vma->vm_flags & VM_EXEC_KEEP) {

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 5/8] oracle/mm: set MMF_HAS_PINNED on the mm receiving preserved pages

[no-reply]

> fork decides whether an anonymous page may be pinned with
> folio_needs_cow_for_dma(), which looks at MMF_HAS_PINNED of the parent
> mm before folio_maybe_dma_pinned(). Only GUP sets that flag, in the mm
> whose pages it pins. Pages preserved across exec keep a long-term pin
> taken through the old mm, but the new mm never has the flag, so a fork
> in the new image clears PageAnonExclusive of a pinned page and maps it
> read-only in both processes. The next write in the parent copies the
> page and the device keeps the old one. A debug kernel catches the
> shared page when the pin is dropped:
> 
>   page dumped because: VM_BUG_ON_PAGE(!PageAnonExclusive(&folio->page))
>   kernel BUG at mm/gup.c:66!
>   RIP: 0010:sanity_check_pinned_pages+0x237/0x960
>    unpin_user_pages
>    gup_test_release
>    __fput
> 
> Set MMF_HAS_PINNED on the new mm when the old one has it.
> 
> Fixes: 37d9f55655ea ("oracle/mm: do not write protect COW mappings when preserving across exec")
> https://virtuozzo.atlassian.net/browse/VSTOR-150231
> 
> Feature: oracle/mm: MADV_DOEXEC madvise() flag
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=5


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

[Devel] [PATCH hci-8.1 6/8] oracle/mm: zap the preserved ranges of the old mm after copying them

[Konstantin Khorenko]

A preserved page keeps PageAnonExclusive in both the old and the new mm
after the copy. exec_mmap() only drops a reference to the old mm, and
anything that holds mm_users keeps it mapped there, such as a
/proc/pid/mem reader stuck in copy_to_user() or process_vm_readv(). Code
that trusts PageAnonExclusive to mean a single mapping then goes wrong.
UFFDIO_MOVE in either mm calls folio_move_anon_rmap() on a folio the
other mm still maps, and once the destination anon_vma is freed the next
rmap walk reads freed memory. try_to_migrate_one() hits VM_BUG_ON_PAGE()
for a writable PTE of a page that the other mapping made shared, and a
writable set_pte_at() in the new mm trips PAGE_TABLE_CHECK. A write
through the old mm reuses the page and lands in the new image.

Zap the VM_EXEC_KEEP ranges of the old mm right after the copy, under
its write lock, so that the copies are the only mappings left. The
pages are not freed, the new mm holds them; a later access through the
old mm faults in fresh memory there. The unmap work moves from the
teardown of the old mm, which ran it with several threads, into the
zap, so zap_vma_mt() spreads an anonymous VMA over several threads as
well, each with an mmu_gather of its own as concurrent MADV_DONTNEED
calls have; the teardown finds these VMAs empty and no longer treats
VM_EXEC_KEEP specially.

Fixes: 37d9f55655ea ("oracle/mm: do not write protect COW mappings when preserving across exec")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/exec.c          | 10 +++++++-
 include/linux/mm.h |  1 +
 mm/memory.c        | 74 +++++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 67 insertions(+), 18 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8677347963e20..32567010a2f27 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -997,7 +997,15 @@ static int vma_dup_some(struct mm_struct *old_mm, struct mm_struct *new_mm)
 		}
 
 	mmap_write_unlock(new_mm);
-	return ret;
+	if (ret)
+		return ret;
+
+	/* Whoever keeps old_mm alive past the exec must not reach the pages through it. */
+	vma_iter_set(&vmi, 0);
+	for_each_vma(vmi, vma)
+		if (vma->vm_flags & VM_EXEC_KEEP)
+			zap_vma_mt(vma);
+	return 0;
 }
 
 /*
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 75b8bcd9f4428..9ebf3c6dcb978 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2462,6 +2462,7 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
 
 void zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
 		  unsigned long size);
+void zap_vma_mt(struct vm_area_struct *vma);
 void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
 			   unsigned long size, struct zap_details *details);
 static inline void zap_vma_pages(struct vm_area_struct *vma)
diff --git a/mm/memory.c b/mm/memory.c
index 1c128162f5cda..2566f02fb7d5e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2043,30 +2043,20 @@ 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.
-	 */
 	if (!tlb->fullmm) {
 		unmap_page_range(tlb, vma, addr, end, details);
 		return;
 	}
 
 	/*
-	 * 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, using multiple threads
+	 * can result in severe lock contention in the swap subsystem if a
+	 * significant 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.
 	 */
-	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;
@@ -2079,6 +2069,17 @@ static void unmap_page_range_mt(struct mmu_gather *tlb,
 	padata_do_multithreaded(&job);
 }
 
+static int zap_vma_chunk(unsigned long addr, unsigned long end, void *arg)
+{
+	struct vm_area_struct *vma = arg;
+	struct mmu_gather tlb;
+
+	tlb_gather_mmu(&tlb, vma->vm_mm);
+	unmap_page_range(&tlb, vma, addr, end, NULL);
+	tlb_finish_mmu(&tlb);
+	return 0;
+}
+
 #endif /* CONFIG_PADATA */
 
 static void unmap_single_vma(struct mmu_gather *tlb,
@@ -2214,6 +2215,45 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
 	hugetlb_zap_end(vma, details);
 }
 
+/**
+ * zap_vma_mt - remove all ptes of a vma, with several threads if it is large
+ * @vma: vm_area_struct holding ptes to be zapped
+ *
+ * The caller holds the mmap lock for writing. An anonymous vma is zapped in
+ * PMD-aligned parts, each with an mmu_gather of its own, by several threads
+ * when it is large; other vmas, and all of them without CONFIG_PADATA, are
+ * zapped by the caller alone.
+ */
+void zap_vma_mt(struct vm_area_struct *vma)
+{
+#ifdef CONFIG_PADATA
+	struct padata_mt_job job = {
+		.thread_fn   = zap_vma_chunk,
+		.fn_arg      = vma,
+		.start       = vma->vm_start,
+		.size        = vma->vm_end - vma->vm_start,
+		.align       = PMD_SIZE,
+		.min_chunk   = max(1ul << 27, PMD_SIZE),
+		.max_threads = 16,
+	};
+	struct mmu_notifier_range range;
+
+	mmap_assert_write_locked(vma->vm_mm);
+	if (vma_is_anonymous(vma)) {
+		lru_add_drain();
+		mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
+					vma->vm_start, vma->vm_end);
+		update_hiwater_rss(vma->vm_mm);
+		mmu_notifier_invalidate_range_start(&range);
+		padata_do_multithreaded(&job);
+		mmu_notifier_invalidate_range_end(&range);
+		return;
+	}
+#endif
+	zap_page_range_single(vma, vma->vm_start, vma->vm_end - vma->vm_start,
+			      NULL);
+}
+
 /**
  * zap_vma_ptes - remove ptes mapping the vma
  * @vma: vm_area_struct holding ptes to be zapped

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 6/8] oracle/mm: zap the preserved ranges of the old mm after copying them

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: zap_vma_mt() clears old_mm PTEs without vma_start_write()
--

commit f34df816bcbe51e2402a6b8ae419dea8ef679f49
Author: Konstantin Khorenko <khorenko@virtuozzo.com>

oracle/mm: zap the preserved ranges of the old mm after copying them

After the VM_EXEC_KEEP ranges are copied into the new mm, this patch zaps
them from the old mm under its write lock. This is meant to leave the
copies as the only mappings of the preserved pages. A new zap_vma_mt()
helper spreads the zap of an anonymous VMA over several threads.

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

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

[ ... ]

> @@ -997,7 +997,15 @@ static int vma_dup_some(struct mm_struct *old_mm, struct mm_struct *new_mm)
>  		}
>  
>  	mmap_write_unlock(new_mm);
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	/* Whoever keeps old_mm alive past the exec must not reach the pages through it. */
> +	vma_iter_set(&vmi, 0);
> +	for_each_vma(vmi, vma)
> +		if (vma->vm_flags & VM_EXEC_KEEP)
> +			zap_vma_mt(vma);
> +	return 0;
>  }

[Severity: Medium]
Can this zap race with a per-VMA lock holder on old_mm?

The loop calls zap_vma_mt() on old_mm VMAs without calling
vma_start_write(vma) first:

    vma_dup_some()
      for_each_vma(vmi, vma)
        zap_vma_mt(vma)

mmap_write_lock() alone does not drain holders that already took the
per-VMA read lock through lock_vma_under_rcu().

One such holder is UFFDIO_MOVE:

    uffd_move_lock()
      *src_vmap = lock_vma_under_rcu(mm, src_start);

This path never takes mmap_lock. A task that owns a userfaultfd context
bound to old_mm has mm_users on old_mm, and its own t->mm is a different
mm. reserve_preserved_mem() only rejects tasks whose t->mm == mm, so such
a task is not excluded.

It could therefore run folio_move_anon_rmap() or a fault in old_mm while
the zap clears the same PTEs. That would reintroduce the problems the
commit message describes, since the preserved pages still carry
PageAnonExclusive in both mms.

Would it make sense to call vma_start_write(vma) before zapping each
VM_EXEC_KEEP VMA, either in vma_dup_some() or at the start of
zap_vma_mt()?

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

[ ... ]

> +void zap_vma_mt(struct vm_area_struct *vma)
> +{
> +#ifdef CONFIG_PADATA
> +	struct padata_mt_job job = {

[ ... ]

> +	mmap_assert_write_locked(vma->vm_mm);
> +	if (vma_is_anonymous(vma)) {
> +		lru_add_drain();
> +		mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
> +					vma->vm_start, vma->vm_end);
> +		update_hiwater_rss(vma->vm_mm);
> +		mmu_notifier_invalidate_range_start(&range);
> +		padata_do_multithreaded(&job);
> +		mmu_notifier_invalidate_range_end(&range);
> +		return;
> +	}
> +#endif
> +	zap_page_range_single(vma, vma->vm_start, vma->vm_end - vma->vm_start,
> +			      NULL);
> +}

[Severity: Medium]
The same concern applies here. zap_vma_mt() only asserts the mmap write
lock with mmap_assert_write_locked() before clearing PTEs, either in the
padata worker threads through zap_vma_chunk() or in
zap_page_range_single().

It never calls vma_start_write(vma).

For the anonymous case, can PTEs be cleared while a per-VMA locked fault
or UFFDIO_MOVE is still operating on the same VMA? Both the padata path
and the zap_page_range_single() path appear to be exposed.

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=6

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

[Devel] [PATCH hci-8.1 7/8] exec: protect preserved memory against surviving shared-mm tasks

[Konstantin Khorenko]

From: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

exec_mmap() copies the MADV_DOEXEC ranges into the new mm and then zaps
them in the old one, so that the copies are the only mappings left.
de_thread() removes the thread group, but a task sharing the mm via
CLONE_VM without CLONE_THREAD survives exec. It races with the copy,
writing into pages the new image takes over, and afterwards finds the
ranges empty and silently goes on with fresh memory.

Refuse such an exec before the point of no return:

- begin_new_exec(): when the binary accepts preserved memory and the
  mm has a VM_EXEC_KEEP VMA, mark the mm as being preserved and return
  EBUSY if a task outside the thread group has it as its mm. Sibling
  threads, which de_thread() removes, and plain mm references such as
  /proc readers are allowed.
- copy_mm(): refuse a CLONE_VM clone that is not a thread while the mm
  is marked, and count such clones in flight until the child is on the
  task list, so the check above cannot miss one. The check walks the
  task list under RCU; it sees every child whose count was dropped,
  since the drop follows the insertion and both are ordered by
  page_table_lock.
- exec_mmap() and free_bprm(): clear the mark once the mm is switched
  or the exec fails.
- exec_mmap(): take the preserve path, including the source mmap
  write lock, only when the mark was set (bprm->preserving_mem), so an
  accepting binary without a VM_EXEC_KEEP VMA takes only the read lock,
  as an exec without preserved memory does.

A kernel thread running on the mm through kthread_use_mm() is outside
the thread group as well, so it makes the exec fail the same way. This
covers a vhost worker in kthread mode, the default here, for as long as
its device has an owner: the caller has to reset the owner first, as
the worker would otherwise go on writing guest memory into the old mm.
A kernel thread that takes the mm only after the check, for instance
to complete an AIO or fence request made before the exec, is not
refused: a write before the zap of the old ranges reaches the preserved
pages, as one made just before the exec would, and a later one lands in
fresh memory of the old mm and is lost, as after any exec.

Fixes: 37d9f55655ea ("oracle/mm: do not write protect COW mappings when preserving across exec")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
[khorenko: rationale after the zap of the old ranges, count only clones
 that are not threads, walk the task list under RCU, keep the upstream
 begin_new_exec() comment, note kthread_use_mm() users]
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/exec.c                | 82 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/binfmts.h  |  4 ++-
 include/linux/mm_types.h |  4 +++
 kernel/fork.c            | 37 ++++++++++++++++++++++
 4 files changed, 124 insertions(+), 3 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 32567010a2f27..5bf856e829af3 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -978,6 +978,78 @@ 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. Tasks
+ * outside our thread group survive de_thread() and would find the preserved
+ * ranges empty once exec_mmap() zaps them in this mm.
+ */
+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;
+
+	rcu_read_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:
+	rcu_read_unlock();
+	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;
@@ -1040,7 +1112,7 @@ static int exec_mmap(struct linux_binprm *bprm)
 			up_write(&tsk->signal->exec_update_lock);
 			return ret;
 		}
-		if (unlikely(bprm->accepts_preserved_mem)) {
+		if (unlikely(bprm->preserving_mem)) {
 			/*
 			 * Acquire the write lock to avoid changes to page
 			 * table entries while they are copied.
@@ -1085,13 +1157,14 @@ static int exec_mmap(struct linux_binprm *bprm)
 	task_unlock(tsk);
 	lru_gen_use_mm(mm);
 	if (old_mm) {
-		if (unlikely(bprm->accepts_preserved_mem))
+		if (unlikely(bprm->preserving_mem))
 			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);
+		release_preserved_mem(bprm, old_mm);
 		mmput(old_mm);
 		return 0;
 	}
@@ -1300,6 +1373,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
@@ -1552,6 +1629,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 9f03c39498e92..f42487c043178 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -47,7 +47,9 @@ struct linux_binprm {
 		 * Set if the binary being exec'd will accept memory marked
 		 * for preservation by the outgoing process.
 		 */
-		accepts_preserved_mem:1;
+		accepts_preserved_mem:1,
+		/* Outgoing mm has VM_EXEC_KEEP VMAs and is reserved for exec. */
+		preserving_mem:1;
 #ifdef __alpha__
 	unsigned int taso:1;
 #endif
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index cf50c74516b09..befdf38ef855b 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -994,6 +994,10 @@ struct mm_struct {
 		unsigned long flags; /* Must use atomic bitops to access */
 
 		unsigned int vps_dumpable:2;
+
+		/* Shared-mm clone publication, serialized by page_table_lock. */
+		unsigned int exec_keep_clones;
+		bool exec_keep_active;
 #ifdef CONFIG_AIO
 		spinlock_t			ioctx_lock;
 		struct kioctx_table __rcu	*ioctx_table;
diff --git a/kernel/fork.c b/kernel/fork.c
index 00d9b4febd85b..92a7e9aae9278 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,12 @@ static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
 		return 0;
 
 	if (clone_flags & CLONE_VM) {
+		/* de_thread() takes care of our own threads. */
+		if (!(clone_flags & CLONE_THREAD)) {
+			ret = exec_keep_clone_begin(oldmm);
+			if (ret)
+				return ret;
+		}
 		mmget(oldmm);
 		mm = oldmm;
 	} else {
@@ -2638,6 +2669,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 | CLONE_THREAD)) == CLONE_VM && p->mm)
+		exec_keep_clone_end(p->mm);
+
 	if (pidfile)
 		fd_install(pidfd, pidfile);
 
@@ -2679,6 +2714,8 @@ __latent_entropy struct task_struct *copy_process(
 	exit_ve_namespace(p);
 bad_fork_cleanup_mm:
 	if (p->mm) {
+		if ((clone_flags & (CLONE_VM | CLONE_THREAD)) == CLONE_VM)
+			exec_keep_clone_end(p->mm);
 		mm_clear_owner(p->mm, p);
 		mmput(p->mm);
 	}

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 7/8] exec: protect preserved memory against surviving shared-mm tasks

[no-reply]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] fork: copy_mm()/copy_process() take page_table_lock twice per shared-mm clone
- [Low] exec: VM_EXEC_KEEP scan in reserve_preserved_mem() is stale by exec_mmap()
--

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

exec: protect preserved memory against surviving shared-mm tasks

This patch makes an exec that accepts preserved memory fail with EBUSY
when a task outside the thread group still uses the mm and the mm has a
VM_EXEC_KEEP VMA. copy_mm() refuses and counts CLONE_VM, non-thread clones
while the mm is marked. exec_mmap() takes the preserve path only when the
mm was marked.

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

> 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)
> +{
> +	struct mm_struct *mm = current->mm;

[ ... ]

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

[Severity: Low]
Can the result of this scan be stale by the time exec_mmap() runs?

When no VM_EXEC_KEEP VMA is found here, nothing is reserved:
exec_keep_active and bprm->preserving_mem both stay unset.  Sibling
threads have not yet been removed by de_thread(), and copy_mm() only
refuses clones while exec_keep_active is set.  So a sibling thread or a
CLONE_VM task can still call madvise(MADV_DOEXEC) after mmap_read_unlock()
and set VM_EXEC_KEEP on a range.

> @@ -1040,7 +1112,7 @@ static int exec_mmap(struct linux_binprm *bprm)
>  			up_write(&tsk->signal->exec_update_lock);
>  			return ret;
>  		}
> -		if (unlikely(bprm->accepts_preserved_mem)) {
> +		if (unlikely(bprm->preserving_mem)) {

Here the preserve path (write lock plus vma_dup_some()) is now selected by
the early sample rather than by the VMAs present under the lock at copy
time.  A VM_EXEC_KEEP VMA created after the scan in
reserve_preserved_mem() would not be copied, and the range would be lost
with the old mm without any error.

Before this patch, accepts_preserved_mem alone selected the path, and
vma_dup_some() copied any VM_EXEC_KEEP VMA present under
mmap_write_lock.  Should the decision be revalidated, or the mark taken
before the scan, so that a range marked during begin_new_exec() is not
silently dropped?

[ ... ]

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

[ ... ]

> +/* 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);
> +}

[Severity: Low]
This isn't a correctness problem, but is the cost of this accounting
intended for every spawn?

exec_keep_clone_begin() in copy_mm() and exec_keep_clone_end() in
copy_process() each take and release the parent's mm->page_table_lock.
This happens on every CLONE_VM clone that is not a thread (vfork(),
posix_spawn(), clone(CLONE_VM)), whether or not the mm has any
VM_EXEC_KEEP VMA and whether or not an exec is in progress.

page_table_lock also protects page table operations, so a multithreaded
parent taking page faults can contend with it.

Would an atomic counter, or skipping the accounting until the mm has ever
had MADV_DOEXEC applied, avoid the two lock round trips per spawn?

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=7

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

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

[Konstantin Khorenko]

From: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

Add doexec, a MADV_DOEXEC test with the ELF opt-in note, and wire it
into run_vmtests.sh. The pin cases need CONFIG_GUP_TEST.

What it checks:

- CPU writes match what a long-term pin held through gup_test reads
  back, across two execs and a later fork, for base pages, PMD huge
  pages, mlocked pages, a read-only mapping, read-only pins and fast
  GUP pins.
- A COW mapping shared with a child forked before exec stays intact on
  both sides.
- exec is refused with EBUSY while a CLONE_VM task outside the thread
  group has the mm, and succeeds once that task is gone.
- Own threads, a non-leader thread calling exec, /proc readers during
  repeated exec and a vhost device whose owner was reset before exec do
  not block it.
- A CLONE_VM clone racing with exec does not end up sharing the
  preserved memory.
- Optionally, a sweep over task-local allocation failures.

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

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
[khorenko: exec through an fd opened before fail-nth is armed, as /proc
 fails the path lookup with ENOENT; accept E2BIG, ELIBBAD and EIO, which
 exec returns for some failed allocations; report an unexpected child
 status; add the binary to .gitignore; use %note, which every
 architecture's assembler accepts; accept EINVAL from MADV_NOHUGEPAGE
 on kernels without THP; run it in a doexec category of its own; let
 the blocked CLONE_VM child drop its copy of the pipe write end]
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 tools/testing/selftests/mm/.gitignore     |   1 +
 tools/testing/selftests/mm/Makefile       |   1 +
 tools/testing/selftests/mm/doexec.c       | 814 ++++++++++++++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh |   4 +
 4 files changed, 820 insertions(+)

diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
index 8f01f4da1c0de..2bdaebdbb87ea 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 cow
+doexec
 hugepage-mmap
 hugepage-mremap
 hugepage-shm
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index c837b0549c892..7f9efb0e0a386 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 0000000000000..33529aa41367b
--- /dev/null
+++ b/tools/testing/selftests/mm/doexec.c
@@ -0,0 +1,814 @@
+// 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/syscall.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) &&
+	    (kind == HUGE || errno != EINVAL))
+		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;
+}
+
+/* Without its own copy of the write end, the clone ends with its parent. */
+static int blocked_clone(void *arg)
+{
+	close(((int *)arg)[1]);
+	return blocked_task(arg);
+}
+
+/* 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_clone, stack + 65536, CLONE_VM | SIGCHLD, pipe_fd);
+	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;
+}
+
+static int check_exec_fd(int exe, char *ram, size_t size, size_t stride)
+{
+	char addr_arg[32], size_arg[32], stride_arg[32];
+	char *args[] = { "doexec", "--check", addr_arg, size_arg, stride_arg, NULL };
+
+	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);
+	syscall(__NR_execveat, exe, "", args, environ, AT_EMPTY_PATH);
+	return errno;
+}
+
+static int unexpected_status(unsigned int nth, int status)
+{
+	ksft_print_msg("fail-nth %u: wait status %#x\n", nth, status);
+	return KSFT_FAIL;
+}
+
+/* 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, exe;
+
+			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) && errno != EINVAL))
+				_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);
+			/* Keep the path lookup, which /proc fails with ENOENT, out of the sweep. */
+			exe = open("/proc/self/exe", O_RDONLY | O_CLOEXEC);
+			fd = open("/proc/self/fail-nth", O_WRONLY | O_CLOEXEC);
+			len = snprintf(number, sizeof(number), "%u", nth);
+			if (exe < 0 || fd < 0 || write(fd, number, len) != len)
+				_exit(KSFT_FAIL);
+			err = check_exec_fd(exe, ram, size, stride);
+			/* 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);
+			/*
+			 * Failed allocations also surface as E2BIG from
+			 * copy_strings(), and as ELIBBAD or EIO from reading the
+			 * ELF interpreter.
+			 */
+			if (err == ENOMEM || err == EFAULT || err == EAGAIN ||
+			    err == E2BIG || err == ELIBBAD || err == EIO)
+				_exit(10);
+			ksft_print_msg("fail-nth %u: exec: %s\n", nth, strerror(err));
+			_exit(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 unexpected_status(nth, status);
+	}
+	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 6ac8ba7e24471..5604fc8939a33 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -77,6 +77,8 @@ to the -t argument containing one or more of the following categories
 	test prctl(PR_SET_MDWE, ...)
 - page_frag
 	test handling of page fragment allocation and freeing
+- doexec
+	test memory preserved across exec with MADV_DOEXEC
 
 example: ./run_vmtests.sh -t "hmm mmap ksm"
 EOF
@@ -297,6 +299,8 @@ fi
 CATEGORY="gup_test" run_test ./gup_test -ct -F 0x1 0 19 0x1000
 CATEGORY="gup_test" run_test ./gup_longterm
 
+CATEGORY="doexec" run_test ./doexec
+
 CATEGORY="userfaultfd" run_test ./uffd-unit-tests
 uffd_stress_bin=./uffd-stress
 CATEGORY="userfaultfd" run_test ${uffd_stress_bin} anon 20 16

-- 
2.43.0

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

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

[no-reply]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] selftests/mm: doexec child diagnostics are lost by _exit()/exec
- [Low] selftests/mm: doexec ignores write() return values
- [Low] selftests/mm: doexec treats any SIGSEGV in allocation sweep as expected
--

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

selftests/mm: test memory preservation across exec

This commit adds doexec, a selftest for MADV_DOEXEC. It checks that memory
backing long-term GUP pins and other mappings survives repeated exec and
fork. It also covers CLONE_VM handling, threads, /proc readers and an
optional allocation-failure sweep. The test is wired into run_vmtests.sh.

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

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

[ ... ]

> +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];
> +
> [ ... ]
> +	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);
> +}

[Severity: Low]
Can the diagnostics printed by the forked children get lost here?

ksft_print_msg() writes through buffered stdio. When stdout is a pipe or a
file, as under run_vmtests.sh or a CI harness, the "exec: ..." message is
still in the buffer when _exit(KSFT_FAIL) runs, and _exit() does not flush
it.

The same applies to the other messages printed from the per-test children,
for example:

    before_exec()  -> "pin: ...", "MADV_DOEXEC: ..."
    check_pin()    -> "read pin: ...", "CPU/pin mismatch at byte N"
    allocation_failures() -> "fail-nth N: ..."

In main(), the fflush(stdout) calls run in the parent before fork(). They
prevent duplicated output but do not flush what the child prints itself.
Messages printed before the execl() into the --exec stage are likewise
discarded by the exec.

A failing run would then show only a bare "not ok" line. Would an
fflush(stdout) before each _exit() and execl() in the children be
appropriate?

> +static int shared_mm_exec(bool own_thread)
> +{

[ ... ]

> +	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]
Is it intended that the write() results are ignored here?

write() is warn_unused_result under glibc _FORTIFY_SOURCE, so this can
produce -Wunused-result warnings.

If this write ever failed, the CLONE_VM child or thread would stay blocked
in read(), and wait_child() or pthread_join() would hang because the test
has no timeout.

> +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;
> +}

[Severity: Low]
Should the result of this write() be checked as well?

This write is the only signal that the shared memory was stolen. If it fails,
clone_exec_race() sees EOF on the report pipe and passes despite the detected
failure. clone_race_thread() and the --check handler in main() also drop
write() results.

> +	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;
> +	}

[ ... ]

> +		if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
> +			fatal++;

[ ... ]

> +	/* A sweep that injected no recoverable/fatal errors provides no coverage. */
> +	return early && fatal ? KSFT_PASS : KSFT_SKIP;
> +}

[Severity: Low]
Can the allocation_failures() sweep pass even when the preserved mapping has
been lost?

do_execveat_common() deliberately raises SIGSEGV when exec fails after the
point of no return:

    if (bprm->point_of_no_return && !fatal_signal_pending(current))
            force_fatal_sig(SIGSEGV);

So SIGSEGV is a legitimate outcome of the sweep. However, the test cannot
tell that kernel-forced SIGSEGV apart from a user-space fault.

After a successful preserving exec, the --check image dereferences ram[i]
in main(). If a kernel bug dropped or unmapped the preserved range, that load
would also raise SIGSEGV. allocation_failures() would count it as fatal++.

The final check is "early && fatal ? KSFT_PASS : KSFT_SKIP", and it requires
at least one SIGSEGV. A regression that loses the mapping can therefore help
the test pass.

Is there a way to distinguish the two cases? One option is to have the --check
image report a lost mapping without faulting, for example by probing the range
with mincore() or msync() before reading it.

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-2-doexec-pinned-memory-v1-0-36f1120f9b57@virtuozzo.com?part=8

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