[Devel] [PATCH UEK8] mm/huge_memory: recheck a special PMD under the lock in copy_huge_pmd()

[Konstantin Khorenko <khorenko@virtuozzo.com>]

copy_huge_pmd() reads a special PMD without the page table lock and
copies that value to the child. The mmap write lock held by fork does
not stop unmap_mapping_range(): it zaps under i_mmap_rwsem and the PMD
lock.

The zap visits the parent VMA before the child's, as dup_mmap() links
the child right after the parent in i_mmap. If it clears the parent PMD
after fork has read it, and passes the child before fork copies it, the
child keeps a mapping the driver has revoked:

- vfio-pci: the child reaches the BAR after the device memory was
  disabled or the device was reset, instead of getting SIGBUS;
- panthor: the child maps the pages of an evicted buffer object after
  they are freed.

Read the PMD again under the source PMD lock, which the zap takes to
clear it. Either the zap got there first, and the PMD is none, or the
copy is in place before the zap reaches the child. Faults are locked
out, so only a zap can change the PMD, and pmd_none() is the test.

Fixes: bc02afbd4d73 ("mm/fork: accept huge pfnmap entries")
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
Hi Anthony,

A gentle ping on the MADV_DOEXEC patches I sent on October 14:

- [PATCH UEK8 0/4] mm, exec: keep MADV_DOEXEC memory consistent after
  the copy
- [PATCH UEK8 0/5] mm, exec: more MADV_DOEXEC fixes
- [PATCH UEK8] selftests/mm: add a MADV_DOEXEC test

Did you have a chance to look at them, and are they of use to you? They
still apply cleanly to the current uek8/u2 (0f1ac1c0d6eb).

One more fix
============

This patch is one more fix for uek8/u2, outside MADV_DOEXEC: fork can
copy a vfio-pci BAR mapping into the child after the driver has revoked
it. It applies to the same base as the series above and to the current
uek8/u2.

It is a backport of v3 of the mainline fix:
https://lore.kernel.org/r/20261014-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-v3-1-85eaec8417f6@virtuozzo.com

Here the lockless check has no huge zero PMD test and the CoW warning
uses is_cow_mapping(), as uek8/u2 does. Of the two drivers in the
commit message only vfio-pci maps huge PMDs in uek8/u2, panthor does
not.

After David Hildenbrand's review mainline takes patch 3 of v5 instead.
It decides under the lock with vm_normal_page_pmd(), as the PTE path
does, and needs the file THP copy of patches 1-2:
https://lore.kernel.org/r/20261104-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-v5-3-c56d0d26f970@virtuozzo.com

v5 is in mm-new now, patches 1-3 with Cc: stable, and I will do the
6.12 backport. Andrew asked why a special-PMD bug needs the file THP
change, so the final shape may still change. Please pick what suits
UEK8 better: this patch now, or the mainline series once it settles.

Reproducing
===========

The race needs fork to stall between the lockless PMD read and the PMD
lock for longer than the zap takes to get from the parent VMA to the
child's. I have not hit it without help, so this test-only change
widens the window:

    diff --git a/mm/huge_memory.c b/mm/huge_memory.c
    index 66bc643464d7d..8cf5fe773e20e 100644
    --- a/mm/huge_memory.c
    +++ b/mm/huge_memory.c
    @@ -1759,6 +1759,17 @@ struct page *follow_devmap_pmd(struct vm_area_struct *vma, unsigned long addr,
     	return page;
     }

    +/* Test only: widen the window after the lockless read below. */
    +#include <linux/delay.h>
    +static u32 copy_huge_pmd_delay_ms;
    +static int __init copy_huge_pmd_debugfs(void)
    +{
    +	debugfs_create_u32("copy_huge_pmd_delay_ms", 0600, NULL,
    +			   &copy_huge_pmd_delay_ms);
    +	return 0;
    +}
    +late_initcall(copy_huge_pmd_debugfs);
    +
     int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
     		  pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
     		  struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
    @@ -1772,6 +1783,8 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,

     	pmd = pmdp_get_lockless(src_pmd);
     	if (unlikely(pmd_present(pmd) && pmd_special(pmd))) {
    +		if (copy_huge_pmd_delay_ms)
    +			mdelay(copy_huge_pmd_delay_ms);
     		dst_ptl = pmd_lock(dst_mm, dst_pmd);
     		src_ptl = pmd_lockptr(src_mm, src_pmd);
     		spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);

Bind a device with a BAR of 2 MB or more to vfio-pci. In a guest a
spare QXL in no-IOMMU mode will do:

    modprobe vfio enable_unsafe_noiommu_mode=1
    modprobe vfio-pci
    echo vfio-pci > /sys/bus/pci/devices/0000:00:02.0/driver_override
    echo 0000:00:02.0 > /sys/bus/pci/devices/0000:00:02.0/driver/unbind
    echo 0000:00:02.0 > /sys/bus/pci/drivers_probe

Then build the reproducer below and run it as root:

    gcc -O2 -pthread -o fork_revoke fork_revoke.c
    echo 2 > /sys/kernel/debug/copy_huge_pmd_delay_ms
    ./fork_revoke 0000:00:02.0 100 0 3000

It forks while a second thread clears PCI_COMMAND_MEMORY through the
vfio config region, which makes vfio-pci zap the BAR mappings. A child
that still reads the BAR without SIGBUS kept a stale mapping; the last
line counts them. On mainline with the 2 ms delay most children did,
with the fix none did. I have not run it on uek8/u2 yet.

It writes the BAR as a control and reads it with the memory disabled,
which is harmless for QXL VRAM; do not point it at a BAR that holds
device registers.

    // SPDX-License-Identifier: GPL-2.0
    /*
     * Fork while vfio-pci revokes a BAR mapped with huge PMDs.
     *
     * The parent maps a BAR of a vfio-pci device and faults it in. A second
     * thread disables the device memory through the command register, which
     * makes vfio-pci zap all BAR mappings, while the main thread forks. Once
     * the memory is disabled, every access to the BAR must raise SIGBUS. A
     * child that can still read a BAR chunk kept a mapping the zap removed.
     *
     * usage: fork_revoke <domain:bus:dev.fn> [iterations [bar [max_delay_us]]]
     *
     * The revoke starts after a random delay of up to max_delay_us, so that it
     * lands at different points of fork.
     */
    #define _GNU_SOURCE
    #include <errno.h>
    #include <fcntl.h>
    #include <limits.h>
    #include <pthread.h>
    #include <sched.h>
    #include <setjmp.h>
    #include <signal.h>
    #include <stdint.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <unistd.h>
    #include <sys/ioctl.h>
    #include <sys/mman.h>
    #include <sys/wait.h>
    #include <linux/pci_regs.h>
    #include <linux/vfio.h>

    #define SZ_2M	(2UL << 20)

    static int dev;
    static off_t cfg_off;
    static int go_pipe[2], done_pipe[2];
    static volatile uint32_t *shared;	/* [0]: iteration whose revoke finished */
    static unsigned long iter;
    static unsigned int max_delay_us;
    static sigjmp_buf jb;

    static void die(const char *what)
    {
    	fprintf(stderr, "%s: %s\n", what, strerror(errno));
    	exit(2);
    }

    static int device_fd(const char *bdf)
    {
    	char path[PATH_MAX], link[PATH_MAX], *group_id;
    	int container, group, noiommu;
    	ssize_t len;

    	snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/iommu_group", bdf);
    	len = readlink(path, link, sizeof(link) - 1);
    	if (len < 0)
    		die("iommu_group");
    	link[len] = 0;
    	group_id = strrchr(link, '/') + 1;
    	snprintf(path, sizeof(path), "/dev/vfio/%s", group_id);
    	noiommu = access(path, F_OK);
    	if (noiommu)
    		snprintf(path, sizeof(path), "/dev/vfio/noiommu-%s", group_id);
    	container = open("/dev/vfio/vfio", O_RDWR);
    	group = open(path, O_RDWR);
    	if (container < 0 || group < 0)
    		die(path);
    	if (ioctl(group, VFIO_GROUP_SET_CONTAINER, &container) ||
    	    ioctl(container, VFIO_SET_IOMMU,
    		  noiommu ? VFIO_NOIOMMU_IOMMU : VFIO_TYPE1v2_IOMMU))
    		die("vfio container");
    	return ioctl(group, VFIO_GROUP_GET_DEVICE_FD, bdf);
    }

    static void set_memory(int on)
    {
    	uint16_t cmd;

    	if (pread(dev, &cmd, 2, cfg_off + PCI_COMMAND) != 2)
    		die("read PCI_COMMAND");
    	cmd = on ? cmd | PCI_COMMAND_MEMORY : cmd & ~PCI_COMMAND_MEMORY;
    	if (pwrite(dev, &cmd, 2, cfg_off + PCI_COMMAND) != 2)
    		die("write PCI_COMMAND");
    }

    static void *revoker(void *arg)
    {
    	unsigned long i;

    	while (read(go_pipe[0], &i, sizeof(i)) == sizeof(i)) {
    		if (max_delay_us)
    			usleep(random() % max_delay_us);
    		set_memory(0);
    		shared[0] = i;
    		if (write(done_pipe[1], &i, sizeof(i)) != sizeof(i))
    			die("done pipe");
    	}
    	return NULL;
    }

    static void sigbus(int sig)
    {
    	siglongjmp(jb, 1);
    }

    /* Number of 2MB chunks of the BAR that can be read without SIGBUS. */
    static unsigned long readable(volatile uint8_t *bar, size_t size)
    {
    	volatile unsigned long n = 0;
    	volatile size_t off;

    	for (off = 0; off < size; off += SZ_2M) {
    		if (sigsetjmp(jb, 1))
    			continue;
    		(void)bar[off];
    		n++;
    	}
    	return n;
    }

    int main(int argc, char **argv)
    {
    	unsigned long iterations = argc > 2 ? strtoul(argv[2], NULL, 0) : 1000;
    	unsigned int index = argc > 3 ? atoi(argv[3]) : 0;
    	struct vfio_region_info cfg = { .argsz = sizeof(cfg),
    					.index = VFIO_PCI_CONFIG_REGION_INDEX };
    	struct vfio_region_info bar = { .argsz = sizeof(bar), .index = index };
    	unsigned long stale_children = 0, stale_chunks = 0, n;
    	volatile uint8_t *map;
    	pthread_t thread;
    	size_t size, off;
    	uint8_t *resv;
    	int status;
    	pid_t pid;

    	if (argc < 2) {
    		fprintf(stderr, "usage: %s <bdf> [iterations [bar [max_delay_us]]]\n",
    			argv[0]);
    		return 2;
    	}
    	max_delay_us = argc > 4 ? atoi(argv[4]) : 0;
    	signal(SIGBUS, sigbus);
    	dev = device_fd(argv[1]);
    	if (dev < 0)
    		die("device fd");
    	if (ioctl(dev, VFIO_DEVICE_GET_REGION_INFO, &cfg) ||
    	    ioctl(dev, VFIO_DEVICE_GET_REGION_INFO, &bar))
    		die("region info");
    	if (!(bar.flags & VFIO_REGION_INFO_FLAG_MMAP) || bar.size < SZ_2M) {
    		fprintf(stderr, "BAR %u: no mmap or smaller than 2MB\n", index);
    		return 2;
    	}
    	cfg_off = cfg.offset;
    	size = bar.size & ~(SZ_2M - 1);

    	/* A 2MB aligned address lets vfio-pci map the BAR with huge PMDs. */
    	resv = mmap(NULL, size + SZ_2M, PROT_NONE,
    		    MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
    	if (resv == MAP_FAILED)
    		die("reserve");
    	resv = (uint8_t *)(((uintptr_t)resv + SZ_2M - 1) & ~(SZ_2M - 1));
    	map = mmap(resv, size, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED,
    		   dev, bar.offset);
    	if (map == MAP_FAILED)
    		die("mmap BAR");
    	shared = mmap(NULL, 4096, PROT_READ | PROT_WRITE,
    		      MAP_SHARED | MAP_ANONYMOUS, -1, 0);
    	if (shared == MAP_FAILED)
    		die("shared page");

    	/* Control: without a revoke the child reads what the parent wrote. */
    	set_memory(1);
    	for (off = 0; off < size; off += SZ_2M)
    		map[off] = (uint8_t)(off / SZ_2M) ^ 0x5a;
    	pid = fork();
    	if (!pid) {
    		for (off = 0; off < size; off += SZ_2M)
    			if (map[off] != ((uint8_t)(off / SZ_2M) ^ 0x5a))
    				_exit(1);
    		_exit(0);
    	}
    	if (pid < 0 || waitpid(pid, &status, 0) != pid ||
    	    !WIFEXITED(status) || WEXITSTATUS(status)) {
    		fprintf(stderr, "control: child does not see the parent's BAR\n");
    		return 2;
    	}
    	printf("BAR %u: %zu MB, %lu iterations, revoke delay < %u us\n",
    	       index, size >> 20, iterations, max_delay_us);

    	/* Premise: with the memory disabled the BAR cannot be read. */
    	set_memory(0);
    	n = readable(map, size);
    	set_memory(1);
    	if (n) {
    		fprintf(stderr, "memory disabled, parent still reads %lu chunks\n", n);
    		return 2;
    	}

    	if (pipe(go_pipe) || pipe(done_pipe))
    		die("pipe");
    	if (pthread_create(&thread, NULL, revoker, NULL))
    		die("thread");

    	for (iter = 1; iter <= iterations; iter++) {
    		set_memory(1);
    		for (off = 0; off < size; off += SZ_2M)
    			(void)map[off];
    		if (write(go_pipe[1], &iter, sizeof(iter)) != sizeof(iter))
    			die("go pipe");
    		pid = fork();
    		if (pid < 0)
    			die("fork");
    		if (!pid) {
    			while (shared[0] != iter)
    				sched_yield();
    			n = readable(map, size);
    			_exit(n > 255 ? 255 : n);
    		}
    		if (read(done_pipe[0], &n, sizeof(n)) != sizeof(n))
    			die("done pipe");
    		if (waitpid(pid, &status, 0) != pid || !WIFEXITED(status))
    			die("child");
    		if (WEXITSTATUS(status)) {
    			stale_children++;
    			stale_chunks += WEXITSTATUS(status);
    		}
    	}
    	set_memory(1);
    	printf("children with a stale BAR mapping: %lu of %lu (%lu 2MB chunks)\n",
    	       stale_children, iterations, stale_chunks);
    	return stale_children ? 1 : 0;
    }

Assisted-by: LLM
---
 mm/huge_memory.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 66bc643464d7d..6abb55932d552 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1776,13 +1776,22 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 		src_ptl = pmd_lockptr(src_mm, src_pmd);
 		spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
 		/*
-		 * No need to recheck the pmd, it can't change with write
-		 * mmap lock held here.
-		 *
-		 * Meanwhile, making sure it's not a CoW VMA with writable
-		 * mapping, otherwise it means either the anon page wrongly
-		 * applied special bit, or we made the PRIVATE mapping be
-		 * able to wrongly write to the backend MMIO.
+		 * The mmap and VMA write locks of the source keep faults
+		 * out, but unmap_mapping_range() does not take the mmap
+		 * lock and may have cleared the pmd since the lockless
+		 * read. Let copy_pmd_range() skip it then.
+		 */
+		pmd = pmdp_get(src_pmd);
+		if (unlikely(pmd_none(pmd))) {
+			spin_unlock(src_ptl);
+			spin_unlock(dst_ptl);
+			return -EAGAIN;
+		}
+		/*
+		 * Make sure it's not a CoW VMA with writable mapping,
+		 * otherwise it means either the anon page wrongly applied
+		 * special bit, or we made the PRIVATE mapping be able to
+		 * wrongly write to the backend MMIO.
 		 */
 		VM_WARN_ON_ONCE(is_cow_mapping(src_vma->vm_flags) && pmd_write(pmd));
 		goto set_pmd;

---
base-commit: d52df3c00e50ba60bbf7526973910da6d455eee6
change-id: 20261014-b4-prep-khorenko-uek8-copy-huge-pmd-recheck-3fb4a303e107

Best regards,
--  
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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