Re: [PATCH v4 16/16] vfio: selftests: Add continuous DMA to vfio_pci_liveupdate_kexec_test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing PROT_READ flag in mmap() causes segmentation fault on read.
- [Low] Incorrect memset size in dma_memcpy_start() leaves half of the transfer buffer uninitialized.
--

commit 4781683698837fa3bf4dcf765b9c536579e8545a
Author: David Matlack <dmatlack@google.com>

vfio: selftests: Add continuous DMA to vfio_pci_liveupdate_kexec_test

This commit adds a long-running DMA memcpy operation to the Live Update kexec
test to ensure continuous DMAs do not lead to system instability.

> diff --git a/tools/testing/selftests/vfio/vfio_pci_liveupdate_kexec_test.c b/tools/testing/selftests/vfio/vfio_pci_liveupdate_kexec_test.c
> index 65c48196e44e7..36bddfbb88ed2 100644
> --- a/tools/testing/selftests/vfio/vfio_pci_liveupdate_kexec_test.c
> +++ b/tools/testing/selftests/vfio/vfio_pci_liveupdate_kexec_test.c

[ ... ]

> +static void dma_memcpy_start(struct vfio_pci_device *device)
> +{
> +	void *src = memcpy_region.vaddr, *dst;
> +	u64 count, size;
> +
> +	size = min_t(u64, memcpy_region.size / 2, device->driver.max_memcpy_size);
> +	dst = src + size;
> +
> +	/*
> +	 * Rough Math: If we assume the device will perform memcpy at a rate of
> +	 * 30GB/s then 7200GB of transfers will run for about 4 minutes.
> +	 */
> +	count = (u64)7200 * SZ_1G / size;
> +	count = min_t(u64, count, device->driver.max_memcpy_count);
> +
> +	memset(src, 1, size / 2);
> +	memset(dst, 0, size / 2);

Does this leave half of the transfer buffer uninitialized?

The transfer size is calculated as size, but the initialization loop
only covers size / 2 bytes. When vfio_pci_driver_memcpy_start() initiates
the DMA transfer of size bytes, the device will read from the uninitialized
second half of the buffer.

> +
> +	printf("Kicking off %lu DMA memcpy operations of size 0x%lx...\n", count, size);
> +	vfio_pci_driver_memcpy_start(device,
> +				     to_iova(device, src),
> +				     to_iova(device, dst),
> +				     size, count);
> +}
> +
> +static void dma_memfd_map(struct vfio_pci_device *device, int fd)
> +{
> +	void *vaddr;
> +
> +	vaddr = mmap(NULL, MEMFD_SIZE, PROT_WRITE, MAP_SHARED, fd, 0);

Does this mmap() call need the PROT_READ flag?

The DMA buffer is mapped with only PROT_WRITE, but the test subsequently
performs read operations on this mapped memory:

dma_memcpy_one()
    VFIO_ASSERT_EQ(memcmp(src, dst, size), 0);

On architectures that enforce strict memory permissions and do not
implicitly grant read access for write-only mappings, this will cause a
segmentation fault.

> +	VFIO_ASSERT_NE(vaddr, MAP_FAILED);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511234802.2280368-1-vipinsh@google.com?part=16