Re: [RFC PATCH 2/4] vfio/pci: Allow hiding BAR ranges from the VFIO user

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] vfio/pci: MSIX_MAPPABLE cap added before vfio_pci_hidden_add_sparse_mmap()
--

commit 34f058d0ce2fee9063501ec235bccdb3f8fe57f0
Author: Piotr Piórkowski <piotr.piorkowski@intel.com>

vfio/pci: Allow hiding BAR ranges from the VFIO user

This commit adds vfio_pci_core_hide_bar_ranges() to allow VFIO PCI drivers
to hide parts of their memory BARs from userspace across region reads and
writes, mmap, ioeventfd, and dma-buf operations, while listing the hidden
ranges in sysfs.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index fcbb2da860e2a..cd6cb3d829b51 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1147,6 +1147,10 @@ int vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
>  				if (ret)
>  					return ret;
>  			}
> +
> +			ret = vfio_pci_hidden_add_sparse_mmap(vdev, info, caps);
> +			if (ret)
> +				return ret;
>  		}
>  
>  		break;

[Severity: Medium]
Does calling msix_mmappable_cap() before vfio_pci_hidden_add_sparse_mmap()
report inconsistent region capabilities when hidden ranges are present on
vdev->msix_bar?

In vfio_pci_ioctl_get_region_info(), when vdev->bar_mmap_supported[info->index]
is true and info->index == vdev->msix_bar, msix_mmappable_cap() appends
VFIO_REGION_INFO_CAP_MSIX_MAPPABLE to caps before
vfio_pci_hidden_add_sparse_mmap() runs:

drivers/vfio/pci/vfio_pci_core.c:vfio_pci_ioctl_get_region_info() {
		if (vdev->bar_mmap_supported[info->index]) {
			info->flags |= VFIO_REGION_INFO_FLAG_MMAP;
			if (info->index == vdev->msix_bar) {
				ret = msix_mmappable_cap(vdev, caps);
				if (ret)
					return ret;
			}

			ret = vfio_pci_hidden_add_sparse_mmap(vdev, info, caps);
			if (ret)
				return ret;
		}
}

If hidden ranges cover all pages of that BAR, vfio_pci_hidden_add_sparse_mmap()
computes nr_areas == 0 and clears VFIO_REGION_INFO_FLAG_MMAP:

drivers/vfio/pci/vfio_pci_hidden_bar_ranges.c:vfio_pci_hidden_add_sparse_mmap() {
	if (!nr_areas) {
		info->flags &= ~VFIO_REGION_INFO_FLAG_MMAP;
		goto out;
	}
}

Because VFIO_REGION_INFO_CAP_MSIX_MAPPABLE was already added to caps,
VFIO_DEVICE_GET_REGION_INFO will return a region with
VFIO_REGION_INFO_FLAG_MMAP cleared while still advertising
VFIO_REGION_INFO_FLAG_CAPS and VFIO_REGION_INFO_CAP_MSIX_MAPPABLE.

Similarly, even when nr_areas > 0, if a hidden range overlaps the page(s)
containing the MSI-X table (vdev->msix_offset to vdev->msix_offset +
vdev->msix_size), those pages are excluded from
VFIO_REGION_INFO_CAP_SPARSE_MMAP and faulting them in
vfio_pci_mmap_huge_fault() returns VM_FAULT_SIGBUS, while
VFIO_REGION_INFO_CAP_MSIX_MAPPABLE still tells userspace the MSI-X range is
mmappable.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105153738.1938345-1-piotr.piorkowski@intel.com?part=2