[PATCH] vfio/pci/vfio_pci_core.c: add IORESOURCE_EXCLUSIVE flag

[PATCH] vfio/pci/vfio_pci_core.c: add IORESOURCE_EXCLUSIVE flag

[Seunggyun Lee]

While using a pci device (GPU) through the vfio-pci passthrough in QEMU
VM, host can mmap the PCI device which used by the guest through sysfs.

In this case, when the guest used the PCI device, the host could also
access the data stored in the PCI device memory.

Regarding this, there is a routine to check IORESOURCE_EXCLUSIVE through
iomem_is_exclusive() in pci_mmap_resource() of pci-sysfs.c, but vfio-pci
driver doesn't seem to set that flag.

Wouldn't it be better to use pci_request_selected_regions_exclusive() to
set the IORESOURCE_EXCLUSIVE flag rather than
pci_request_selected_regions() that was used previously?

Signed-off-by: Seunggyun Lee <sglee97@dankook.ac.kr>
---
 drivers/vfio/pci/vfio_pci_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 26a541cc64d1..9731ac35b3ad 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1779,7 +1779,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
 	 * we need to request the region and the barmap tracks that.
 	 */
 	if (!vdev->barmap[index]) {
-		ret = pci_request_selected_regions(pdev,
+		ret = pci_request_selected_regions_exclusive(pdev,
 						   1 << index, "vfio-pci");
 		if (ret)
 			return ret;
-- 
2.25.1

Re: [PATCH] vfio/pci/vfio_pci_core.c: add IORESOURCE_EXCLUSIVE flag

[Alex Williamson]

On Tue,  7 Mar 2023 12:40:18 +0900
Seunggyun Lee <sglee97@dankook.ac.kr> wrote:

> While using a pci device (GPU) through the vfio-pci passthrough in QEMU
> VM, host can mmap the PCI device which used by the guest through sysfs.
> 
> In this case, when the guest used the PCI device, the host could also
> access the data stored in the PCI device memory.
> 
> Regarding this, there is a routine to check IORESOURCE_EXCLUSIVE through
> iomem_is_exclusive() in pci_mmap_resource() of pci-sysfs.c, but vfio-pci
> driver doesn't seem to set that flag.
> 
> Wouldn't it be better to use pci_request_selected_regions_exclusive() to
> set the IORESOURCE_EXCLUSIVE flag rather than
> pci_request_selected_regions() that was used previously?
> 
> Signed-off-by: Seunggyun Lee <sglee97@dankook.ac.kr>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 26a541cc64d1..9731ac35b3ad 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1779,7 +1779,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
>  	 * we need to request the region and the barmap tracks that.
>  	 */
>  	if (!vdev->barmap[index]) {
> -		ret = pci_request_selected_regions(pdev,
> +		ret = pci_request_selected_regions_exclusive(pdev,
>  						   1 << index, "vfio-pci");
>  		if (ret)
>  			return ret;

I don't understand why this request is so pervasive lately, there are
other means to lockdown sysfs access to a device, why are they not
sufficient (ex. LOCKDOWN_PCI_ACCESS).

If this is work towards confidential computing support with VFIO, I'm
afraid that sysfs access to the device is only one potential vector,
QEMU itself is in control of whether a VM directly maps device
resources.

Also, IORESOURCE_EXCLUSIVE is described as preventing userspace mapping
of the resource, so it's a bit ironic that a driver providing userspace
mappings of device resources would mark the resource in such a way.
Thanks,

Alex