[PATCH v2 0/2] vfio/pci: fix huge_fault write handling and add selftest

[PATCH v2 0/2] vfio/pci: fix huge_fault write handling and add selftest

[Mina Almasry]

Fix vfio_pci_vmf_insert_pfn() passing write=false to
vmf_insert_pfn_pmd() and vmf_insert_pfn_pud(), which causes an infinite
write page-fault loop when vma->vm_page_prot starts read-only (such as
after soft-dirty clearing via /proc/<pid>/clear_refs in CRIU or live
migration, or on arm64 without hardware DBM), and add a companion VFIO
PCI selftest covering both read-then-write upgrade and initial write
faults after clear_refs.

Changes in v2:
- Expand Patch 1/2 commit log with practical trigger scenarios (CRIU /
  clear_refs soft-dirty clearing and arm64 software dirty tracking) for
  stable backport evaluation, per Alex Williamson.
- Drop the Assisted-by tag from Patch 1/2.
- Add Patch 2/2 selftest in vfio_pci_device_test (bar_write_fault_after_clear_refs),
  as suggested by Alex Williamson.
- Link to v1: https://lore.kernel.org/all/20261024015540.538758-1-almasrymina@google.com/

Mina Almasry (2):
  vfio/pci: pass FAULT_FLAG_WRITE to vmf_insert_pfn_pmd/pud()
  vfio: selftests: Test BAR write faults after clear_refs

 drivers/vfio/pci/vfio_pci_core.c              |  6 +-
 .../selftests/vfio/vfio_pci_device_test.c     | 83 +++++++++++++++++++
 2 files changed, 87 insertions(+), 2 deletions(-)


base-commit: 076d7e989cb69f6ded7d60f2a080eb909930061b
-- 
2.56.0.385.gac671c819a-goog

[PATCH v2 1/2] vfio/pci: pass FAULT_FLAG_WRITE to vmf_insert_pfn_pmd/pud()

[Mina Almasry]

vfio_pci_vmf_insert_pfn() is the only caller of vmf_insert_pfn_pmd() and
vmf_insert_pfn_pud() in the kernel that hardcodes write=false instead of
passing vmf->flags & FAULT_FLAG_WRITE (unlike drm, panthor, and mshv).

On x86_64, normal MAP_SHARED | PROT_WRITE VFIO BAR mappings usually mask
this bug because vma->vm_page_prot already has _PAGE_RW set. However,
when vma->vm_page_prot starts read-only, insert_pmd() and insert_pud()
with write=false install a read-only huge entry without calling
pmd_mkdirty()/maybe_pmd_mkwrite() (and similarly for PUD). Subsequent
write-protection faults enter wp_huge_pmd() or wp_huge_pud(), which call
vma->vm_ops->huge_fault() to upgrade the entry in place. Because
vfio_pci_vmf_insert_pfn() passes write=false, insert_pmd() and
insert_pud() see !pmd_none(*pmd) with !write, do nothing, and return
VM_FAULT_NOPAGE, causing an unrecoverable infinite write page-fault loop
and host CPU soft lockup on the next userspace or KVM guest MMIO store.

In practice, vma->vm_page_prot is read-only on a writable MAP_SHARED BAR
mapping in two situations:

1. Userspace soft-dirty tracking (CONFIG_MEM_SOFT_DIRTY) when a process
   such as CRIU (during pre-dump/checkpoint), a VMM live-migration loop,
   a memory profiler, or an unprivileged process holding a VFIO device
   fd writes "4" (CLEAR_REFS_SOFT_DIRTY) to /proc/<pid>/clear_refs. That
   clears VM_SOFTDIRTY on all VMAs (including VM_PFNMAP) and calls
   vma_set_page_prot(), stripping _PAGE_RW from vma->vm_page_prot for
   subsequent huge faults.

2. Architectures where shared writable mappings start read-only for
   software dirty tracking, such as arm64 without hardware DBM
   (CONFIG_ARM64_HW_AFDBM off or FEAT_HAFDBS absent), where _PAGE_SHARED
   sets PTE_RDONLY | PTE_WRITE and pgprot_noncached() preserves
   PTE_RDONLY on MT_DEVICE_nGnRnE mappings until pmd_mkdirty() clears
   it on a write fault.

This bug was found by code inspection while auditing huge_fault handlers
and reproduced with a VFIO PCI selftest on a 2 MiB-aligned BAR mapping
after writing "4" to /proc/self/clear_refs.

Pass vmf->flags & FAULT_FLAG_WRITE to vmf_insert_pfn_pmd() and
vmf_insert_pfn_pud() so write faults properly mark huge PFNMAP entries
dirty and writable.

Fixes: f9e54c3a2f5b ("vfio/pci: implement huge_fault support")
Cc: stable@vger.kernel.org
Cc: Ankit Agrawal <ankita@nvidia.com>
Cc: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Cc: David Matlack <dmatlack@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 drivers/vfio/pci/vfio_pci_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index fcbb2da860e2..204d959086e9 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1745,6 +1745,8 @@ vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev,
 				   unsigned long pfn,
 				   unsigned int order)
 {
+	bool write = vmf->flags & FAULT_FLAG_WRITE;
+
 	lockdep_assert_held_read(&vdev->memory_lock);
 
 	if (vdev->pm_runtime_engaged || !__vfio_pci_memory_enabled(vdev))
@@ -1754,10 +1756,10 @@ vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev,
 		return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
 
 	if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PMD_PFNMAP) && order == PMD_ORDER)
-		return vmf_insert_pfn_pmd(vmf, pfn, false);
+		return vmf_insert_pfn_pmd(vmf, pfn, write);
 
 	if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PUD_PFNMAP) && order == PUD_ORDER)
-		return vmf_insert_pfn_pud(vmf, pfn, false);
+		return vmf_insert_pfn_pud(vmf, pfn, write);
 
 	return VM_FAULT_FALLBACK;
 }
-- 
2.56.0.385.gac671c819a-goog

[PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs

[Mina Almasry]

Add a regression test in vfio_pci_device_test to verify that huge_fault
on writable MAP_SHARED VFIO PCI BAR mappings handles write faults after
soft-dirty tracking clears VM_SOFTDIRTY and downgrades vma->vm_page_prot
to read-only via /proc/self/clear_refs ("4").

Specifically, for each mappable read/write BAR (already aligned up to
SZ_1G by vfio_pci_bar_map() with MADV_HUGEPAGE), verify both:

1. Read-then-write fault sequence after clear_refs, exercising in-place
   huge PMD/PUD write-protection upgrade via wp_huge_pmd()/wp_huge_pud().
2. Direct initial write fault on a freshly mapped unpopulated BAR VMA
   after clear_refs, exercising vmf_insert_pfn_pmd()/vmf_insert_pfn_pud()
   with FAULT_FLAG_WRITE set.

Skip gracefully when /proc/self/clear_refs is absent or soft-dirty
tracking (CONFIG_MEM_SOFT_DIRTY) is disabled, or when the device exposes
no mappable read/write BARs.

Cc: David Matlack <dmatlack@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 .../selftests/vfio/vfio_pci_device_test.c     | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/tools/testing/selftests/vfio/vfio_pci_device_test.c b/tools/testing/selftests/vfio/vfio_pci_device_test.c
index 93c11fd5e081..75ccc557efb9 100644
--- a/tools/testing/selftests/vfio/vfio_pci_device_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_device_test.c
@@ -1,10 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0-only
+#include <errno.h>
 #include <fcntl.h>
 #include <stdlib.h>
+#include <unistd.h>
 
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
+#include <linux/compiler.h>
 #include <linux/limits.h>
 #include <linux/pci_regs.h>
 #include <linux/sizes.h>
@@ -102,6 +105,86 @@ TEST_F(vfio_pci_device_test, validate_bars)
 	}
 }
 
+static int clear_soft_dirty_refs(void)
+{
+	ssize_t ret;
+	int fd;
+
+	fd = open("/proc/self/clear_refs", O_WRONLY);
+	if (fd < 0)
+		return -errno;
+
+	ret = write(fd, "4\n", 2);
+	if (ret < 0)
+		ret = -errno;
+	else
+		VFIO_ASSERT_EQ(ret, 2);
+
+	VFIO_ASSERT_EQ(close(fd), 0);
+	return ret < 0 ? ret : 0;
+}
+
+TEST_F(vfio_pci_device_test, bar_write_fault_after_clear_refs)
+{
+	const u32 rw_mmap_flags = VFIO_REGION_INFO_FLAG_MMAP |
+				  VFIO_REGION_INFO_FLAG_READ |
+				  VFIO_REGION_INFO_FLAG_WRITE;
+	struct vfio_pci_bar *bar;
+	bool tested = false;
+	int ret, i;
+	u32 val;
+
+	ret = clear_soft_dirty_refs();
+	if (ret == -ENOENT || ret == -EINVAL)
+		SKIP(return, "soft-dirty clear_refs unsupported (%d)\n", ret);
+	ASSERT_EQ(0, ret);
+
+	vfio_pci_cmd_set(self->device, PCI_COMMAND_MEMORY);
+
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
+		void *vaddr;
+
+		bar = &self->device->bars[i];
+		if ((bar->info.flags & rw_mmap_flags) != rw_mmap_flags)
+			continue;
+
+		ASSERT_NE(NULL, bar->vaddr);
+
+		/*
+		 * clear_soft_dirty_refs() stripped write permissions from
+		 * vma->vm_page_prot on the unpopulated BAR mapping created by
+		 * vfio_pci_device_init(). Fault it read-only first, then write
+		 * to exercise in-place write-upgrade (wp_huge_pmd/wp_huge_pud).
+		 */
+		val = READ_ONCE(*(u32 *)bar->vaddr);
+		WRITE_ONCE(*(u32 *)bar->vaddr, val);
+
+		/*
+		 * Replace the VMA with a fresh unpopulated mapping at the same
+		 * hugepage-aligned address, clear soft-dirty again, and fault
+		 * with an immediate store to exercise initial write insertion
+		 * (vmf_insert_pfn_pmd/pud with write=true).
+		 */
+		vaddr = mmap(bar->vaddr, bar->info.size, PROT_READ | PROT_WRITE,
+			     MAP_SHARED | MAP_FIXED, self->device->fd,
+			     bar->info.offset);
+		ASSERT_EQ(bar->vaddr, vaddr);
+		madvise(bar->vaddr, bar->info.size, MADV_HUGEPAGE);
+
+		ASSERT_EQ(0, clear_soft_dirty_refs());
+
+		WRITE_ONCE(*(u32 *)bar->vaddr, val);
+		(void)READ_ONCE(*(u32 *)bar->vaddr);
+
+		tested = true;
+	}
+
+	vfio_pci_cmd_clear(self->device, PCI_COMMAND_MEMORY);
+
+	if (!tested)
+		SKIP(return, "No mappable read/write BAR on %s\n", device_bdf);
+}
+
 FIXTURE(vfio_pci_irq_test) {
 	struct iommu *iommu;
 	struct vfio_pci_device *device;
-- 
2.56.0.385.gac671c819a-goog

Re: [PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] vfio: selftests: probe pre-faults BAR 0 before clear_soft_dirty_refs()
--

commit 43368d01d06eff4702b5d6cf5af459ab154dd5bc
Author: Mina Almasry <almasrymina@google.com>

vfio: selftests: Test BAR write faults after clear_refs

This commit adds a regression test in vfio_pci_device_test to verify that
huge_fault on writable MAP_SHARED VFIO PCI BAR mappings handles both
read-then-write and initial direct write faults after soft-dirty tracking
clears VM_SOFTDIRTY via /proc/self/clear_refs.

> diff --git a/tools/testing/selftests/vfio/vfio_pci_device_test.c b/tools/testing/selftests/vfio/vfio_pci_device_test.c
> index 93c11fd5e0818..75ccc557efb92 100644
> --- a/tools/testing/selftests/vfio/vfio_pci_device_test.c
> +++ b/tools/testing/selftests/vfio/vfio_pci_device_test.c

[ ... ]

> @@ -102,6 +105,86 @@ TEST_F(vfio_pci_device_test, validate_bars)
>  	}
>  }
>  
> +static int clear_soft_dirty_refs(void)
> +{
> +	ssize_t ret;
> +	int fd;
> +
> +	fd = open("/proc/self/clear_refs", O_WRONLY);
> +	if (fd < 0)
> +		return -errno;
> +
> +	ret = write(fd, "4\n", 2);
> +	if (ret < 0)
> +		ret = -errno;
> +	else
> +		VFIO_ASSERT_EQ(ret, 2);
> +
> +	VFIO_ASSERT_EQ(close(fd), 0);
> +	return ret < 0 ? ret : 0;
> +}
> +
> +TEST_F(vfio_pci_device_test, bar_write_fault_after_clear_refs)
> +{
> +	const u32 rw_mmap_flags = VFIO_REGION_INFO_FLAG_MMAP |
> +				  VFIO_REGION_INFO_FLAG_READ |
> +				  VFIO_REGION_INFO_FLAG_WRITE;
> +	struct vfio_pci_bar *bar;
> +	bool tested = false;
> +	int ret, i;
> +	u32 val;
> +
> +	ret = clear_soft_dirty_refs();
> +	if (ret == -ENOENT || ret == -EINVAL)
> +		SKIP(return, "soft-dirty clear_refs unsupported (%d)\n", ret);
> +	ASSERT_EQ(0, ret);
> +
> +	vfio_pci_cmd_set(self->device, PCI_COMMAND_MEMORY);
> +
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> +		void *vaddr;
> +
> +		bar = &self->device->bars[i];
> +		if ((bar->info.flags & rw_mmap_flags) != rw_mmap_flags)
> +			continue;
> +
> +		ASSERT_NE(NULL, bar->vaddr);
> +
> +		/*
> +		 * clear_soft_dirty_refs() stripped write permissions from
> +		 * vma->vm_page_prot on the unpopulated BAR mapping created by
> +		 * vfio_pci_device_init(). Fault it read-only first, then write
> +		 * to exercise in-place write-upgrade (wp_huge_pmd/wp_huge_pud).
> +		 */
> +		val = READ_ONCE(*(u32 *)bar->vaddr);
> +		WRITE_ONCE(*(u32 *)bar->vaddr, val);

[Severity: Medium]
Is bar->vaddr guaranteed to be unpopulated at this point for BAR 0?

During FIXTURE_SETUP(vfio_pci_device_test), vfio_pci_device_init() maps the
BARs and then calls vfio_pci_driver_probe():

tools/testing/selftests/vfio/lib/vfio_pci_device.c:vfio_pci_device_init() {
	...
	vfio_pci_device_setup(device);
	vfio_pci_driver_probe(device);
	return device;
}

Several selftest drivers read from BAR 0 during probe:

- dsa_probe() -> dsa_int_handle_request_required() reads
  bar0 + IDXD_GENCAP_OFFSET
- ioat_probe() reads device->bars[0].vaddr + IOAT_VER_OFFSET
- nv_falcon_probe() reads bar0 + NV_PMC_BOOT_0

Because those reads happen before clear_soft_dirty_refs() is called,
VM_SOFTDIRTY is still set on the VMA and the fault installs a page table entry
that is already writable.

When clear_soft_dirty_refs() subsequently runs, clear_refs_write() updates
vma->vm_page_prot, but clear_refs_test_walk() skips walking page tables for
VM_PFNMAP VMAs:

fs/proc/task_mmu.c:clear_refs_test_walk() {
	...
	if (vma->vm_flags & VM_PFNMAP)
		return 1;
	...
}

As a result, the existing writable page table entry on BAR 0 remains in place,
and the READ_ONCE() and WRITE_ONCE() above will not fault or exercise
wp_huge_pmd() / wp_huge_pud() on BAR 0 for those devices.

Should the test replace the VMA with a fresh mmap(MAP_FIXED) and
madvise(MADV_HUGEPAGE) (or zap the BAR mappings) before the first
clear_soft_dirty_refs() call so that the read-then-write sequence always starts
from an unpopulated mapping?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101062049.998160-1-almasrymina@google.com?part=2