[Devel] [PATCH hci-8.1 v2 1/2] mm: allow MADV_DOEXEC on shared PFN mappings of device memory

[Konstantin Khorenko <khorenko@virtuozzo.com>]

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

QEMU live update (cpr-exec) keeps guest RAM across exec with MADV_DOEXEC,
but VFIO device BARs mapped into QEMU are lost on exec, and the new QEMU
has to mmap them again on the same device fd.

Some drivers don't allow that. The NVIDIA vGPU VFIO driver records each
BAR mapping and refuses an overlapping one with -EINVAL. For VF BARs the
record is dropped only when the device fd is closed, so after exec the
new QEMU can't map BAR1/BAR3 and the vGPU becomes unusable.

Let MADV_DOEXEC also preserve shared VM_PFNMAP mappings of a VFIO device
file, the BARs. vma_dup() duplicates them the way dup_mmap() duplicates a
shared file mapping: clear the inherited VM_PAT so that track_pfn_copy()
reserves the memtype for the copy, fork the anon_vma if the old VMA
ever got one, link the VMA into the file's i_mmap (so
unmap_mapping_range() on device reset still reaches it), call
vm_ops->open() and copy the special PTEs with copy_page_range().
vma_link() places the copy after the original in i_mmap, since an equal
key goes right in the interval tree, which is the order dup_mmap() gets
from vma_interval_tree_insert_after().

Admit only a VFIO file, checked with vfio_file_is_valid() as KVM does;
of those only a device file can be mapped. Other drivers keep per-mm
state in such mappings and rely on VM_DONTCOPY to keep fork away: perf
charges a ring buffer to the mm's pinned_vm, so a copy would drive the
counter of the new mm negative and lift RLIMIT_MEMLOCK, and Xen privcmd
frees per-mapping state in ->close() without an ->open(). VM_DONTCOPY
itself cannot be the test, because QEMU marks every RAM block
MADV_DONTFORK, VFIO BARs included.

A VFIO mapping whose vm_ops has ->close() but no ->open() is admitted
too: the NVIDIA vGPU driver maps the BAR0 pieces of a vGPU that way.
The driver then gets ->close() for the original VMA at the exec, while
the copy is mapped, and again for the copy later, with no ->open() in
between. A fork that copied the mapping would do the same, but QEMU's
MADV_DONTFORK keeps fork away from it, so this sequence is new to the
driver. qemu-update with a vGPU was tested this way.

No in-tree VFIO driver keeps per-VMA state, and the vfio-pci family
revokes BAR mappings through the device file's mapping, which reaches
the copy. An out-of-tree driver that tracks its mappings has to count
the copy in ->open(). One that maps with remap_pfn_range(), which sets
VM_PAT, and revokes with unmap_mapping_range() inherits the windows fork
has between track_pfn_copy() and a concurrent zap, and the one munmap has
between the zap of the old VMA and a device reset; no in-tree VFIO
driver does both.

Dropping the mark gives VM_MAYEXEC back only to anonymous memory, where
do_mmap() decides it. A marked PFN map does not get it back, since there
the driver had the last word.

Once the copy is in i_mmap, a zap from a device reset may clear VM_PAT
of either VMA holding only i_mmap_rwsem. So clear VM_EXEC_KEEP of the
copy before linking it, and touch VM_ACCOUNT of the old VMA only when it
is set. The source VMA is write-locked explicitly, as in dup_mmap(),
instead of through that vm_flags_clear().

Such a VMA may have NULL vm_ops, in which case vma_is_anonymous()
returns true for it. So the new vma_is_shared_pfnmap() helper is checked
first, also in zap_vma_mt(), whose threaded path for anonymous memory
would skip untrack_pfn().

https://virtuozzo.atlassian.net/browse/VSTOR-150233
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Feature: oracle/mm: MADV_DOEXEC madvise() flag
(cherry picked from commit f1903df83b065cd59ebe30cfcf5fde6391fcc4e3)

Changes while porting: added the untrack_pfn_clear() call, the
unconditional anon_vma_fork(), the VFIO-only admission, the VM_MAYEXEC
handling and the flag ordering above; the file reference now comes from
an earlier fix that takes it for every file-backed VMA.

Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 include/linux/mm.h | 12 ++++++++++--
 mm/madvise.c       | 28 +++++++++++++++++++++++-----
 mm/memory.c        |  2 +-
 mm/mmap.c          | 53 ++++++++++++++++++++++++++++++++++++++---------------
 4 files changed, 72 insertions(+), 23 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 91f305fd5c025..496cbdfc7141a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1018,6 +1018,16 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
 	return !vma->vm_ops;
 }
 
+/*
+ * Shared PFN mapping of a file, such as a device BAR mapped through VFIO.
+ * Its vm_ops may be NULL, so it must be tested before vma_is_anonymous().
+ */
+static inline bool vma_is_shared_pfnmap(struct vm_area_struct *vma)
+{
+	return vma->vm_file &&
+	       (vma->vm_flags & (VM_SHARED | VM_PFNMAP)) == (VM_SHARED | VM_PFNMAP);
+}
+
 /*
  * Indicate if the VMA is a heap for the given task; for
  * /proc/PID/maps that is the heap of the main task.
@@ -2481,10 +2491,8 @@ 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/mm/madvise.c b/mm/madvise.c
index e6972bcfc9749..cf3e580894b17 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -31,6 +31,7 @@
 #include <linux/swapops.h>
 #include <linux/shmem_fs.h>
 #include <linux/mmu_notifier.h>
+#include <linux/vfio.h>
 
 #include <asm/tlb.h>
 
@@ -1249,6 +1250,20 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
 			       &guard_remove_walk_ops, NULL);
 }
 
+/* Only VFIO device mappings are known to cope with a copy in another mm. */
+static bool vma_doexec_pfnmap_ok(struct vm_area_struct *vma)
+{
+	bool (*is_vfio)(struct file *file);
+	bool ret;
+
+	is_vfio = symbol_get(vfio_file_is_valid);
+	if (!is_vfio)
+		return false;
+	ret = is_vfio(vma->vm_file);
+	symbol_put(vfio_file_is_valid);
+	return ret;
+}
+
 /*
  * Apply an madvise behavior to a region of a vma.  madvise_update_vma
  * will handle splitting a vm area into separate areas, each area with its own
@@ -1309,12 +1324,15 @@ static int madvise_vma_behavior(struct vm_area_struct *vma,
 		break;
 	case MADV_DOEXEC:
 		/*
-		 * MADV_DOEXEC is only supported on private, non-executable,
-		 * non-stack, non-special anonymous memory and if the
-		 * VM_EXEC_KEEP flag is available.
+		 * MADV_DOEXEC is supported on private, non-executable,
+		 * non-stack, non-special anonymous memory, and on shared
+		 * PFN mappings of VFIO devices, if the VM_EXEC_KEEP flag
+		 * is available.
 		 */
-		if (!VM_EXEC_KEEP || !vma_is_anonymous(vma) ||
-		    vma->vm_flags & (VM_EXEC | VM_SHARED | VM_STACK | VM_SPECIAL)) {
+		if (!VM_EXEC_KEEP || vma->vm_flags & (VM_EXEC | VM_STACK) ||
+		    (vma_is_shared_pfnmap(vma) ? !vma_doexec_pfnmap_ok(vma) :
+		     !vma_is_anonymous(vma) ||
+		     vma->vm_flags & (VM_SHARED | VM_SPECIAL))) {
 			error = -EINVAL;
 			goto out;
 		}
diff --git a/mm/memory.c b/mm/memory.c
index 2566f02fb7d5e..2f15bc06ccf90 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2239,7 +2239,7 @@ void zap_vma_mt(struct vm_area_struct *vma)
 	struct mmu_notifier_range range;
 
 	mmap_assert_write_locked(vma->vm_mm);
-	if (vma_is_anonymous(vma)) {
+	if (vma_is_anonymous(vma) && !vma_is_shared_pfnmap(vma)) {
 		lru_add_drain();
 		mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
 					vma->vm_start, vma->vm_end);
diff --git a/mm/mmap.c b/mm/mmap.c
index 69dd1514d750e..959810d8047a8 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1752,31 +1752,41 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
 
 /*
  * The VM_MAYEXEC that MADV_DOEXEC took from @vma, to give back when the mark
- * goes: what do_mmap() grants such a mapping.
+ * goes: what do_mmap() grants such a mapping. A driver may have withheld it
+ * from a PFN map, so leave that one without it.
  */
 vm_flags_t vma_doexec_mayexec(struct vm_area_struct *vma)
 {
-	if (vma->vm_file && path_noexec(&vma->vm_file->f_path))
+	if (vma_is_shared_pfnmap(vma) ||
+	    (vma->vm_file && path_noexec(&vma->vm_file->f_path)))
 		return 0;
 	return VM_MAYEXEC;
 }
 
 /*
- * Duplicate an anonymous VMA into a new mm as part of preserving
- * it across exec.
+ * Duplicate a VMA into a new mm as part of preserving it across exec.
+ *
+ * Anonymous VMAs get their pages shared with the new mm fork-style.
+ * Shared PFN mappings (e.g. VFIO BARs) are duplicated the way fork
+ * duplicates a shared file mapping: same file, same special PTEs,
+ * nothing to copy-on-write.
  */
 int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 {
 	struct vm_area_struct *vma;
 	unsigned long npages;
+	bool pfnmap = vma_is_shared_pfnmap(old_vma);
 	int ret = -ENOMEM;
 
-	if (WARN_ON(!vma_is_anonymous(old_vma)))
+	if (WARN_ON(!pfnmap && !vma_is_anonymous(old_vma)))
 		return -EINVAL;
 
 	if (find_vma_intersection(mm, old_vma->vm_start, old_vma->vm_end))
 		return -EEXIST;
 
+	/* As in dup_mmap(), keep per-VMA lock users out of the source. */
+	vma_start_write(old_vma);
+
 	npages = vma_pages(old_vma);
 	vm_stat_account(mm, old_vma->vm_flags, npages);
 
@@ -1784,6 +1794,10 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 	if (!vma)
 		goto fail_nomem;
 
+	/* track_pfn_copy() will later take care of copying internal state. */
+	if (pfnmap)
+		untrack_pfn_clear(vma);
+
 	ret = vma_dup_policy(old_vma, vma);
 	if (ret)
 		goto fail_nomem_policy;
@@ -1797,17 +1811,24 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 
 	/*
 	 * 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. Note that VM_EXEC_KEEP of an anonymous copy 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.
 	 */
 	vm_flags_clear(vma, VM_LOCKED|VM_LOCKONFAULT|VM_UFFD_MISSING|VM_UFFD_WP|VM_UFFD_MINOR);
 	vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
+	/* Once linked, unmap_mapping_range() may modify the flags concurrently. */
+	if (pfnmap)
+		vm_flags_clear(vma, VM_EXEC_KEEP);
 
+	/* Also links a file VMA into the file's i_mmap, for unmap_mapping_range(). */
 	ret = vma_link(mm, vma);
 	if (ret)
 		goto fail_nomem_vma_link;
 
+	if (pfnmap && vma->vm_ops && vma->vm_ops->open)
+		vma->vm_ops->open(vma);
+
 	/*
 	 * 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
@@ -1815,14 +1836,16 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
 	 * process, this effectively transfers the accounting from the old
 	 * vma to the new one.
 	 */
-	vm_flags_clear(old_vma, VM_ACCOUNT);
+	if (old_vma->vm_flags & 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_mod(vma, vma_doexec_mayexec(vma), VM_EXEC_KEEP);
+	/* For PFN maps this copies the device PTEs and the PAT memtype */
+	if (pfnmap || !IS_ENABLED(CONFIG_PADATA))
+		ret = copy_page_range(vma, old_vma);
+	else
+		ret = copy_page_range_mt(vma, old_vma);
+	if (!pfnmap)
+		vm_flags_mod(vma, vma_doexec_mayexec(vma), VM_EXEC_KEEP);
 
 	return ret;
 

-- 
2.43.0

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