linux-kernel - Re: [PATCH 3/3] VFIO V4: VFIO driver: Non-privileged user level PCI drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1285620073.4951.44.camel@x201>
Date:	Mon, 27 Sep 2010 14:41:13 -0600
From:	Alex Williamson <alex.williamson@...hat.com>
To:	Tom Lyon <pugs@...co.com>
Cc:	linux-pci@...r.kernel.org, jbarnes@...tuousgeek.org,
	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	randy.dunlap@...cle.com, arnd@...db.de, joro@...tes.org,
	hjk@...utronix.de, avi@...hat.com, gregkh@...e.de,
	chrisw@...s-sol.org, mst@...hat.com
Subject: Re: [PATCH 3/3] VFIO V4: VFIO driver: Non-privileged user level
 PCI drivers

On Wed, 2010-09-22 at 14:18 -0700, Tom Lyon wrote:
> +/*
> + * Pretend we're hardware and tweak the values
> + * of the *virtual* pci BARs to reflect the hardware
> + * capabilities
> + */
> +static void vfio_bar_fixup(struct vfio_dev *vdev)
> +{
> +	struct pci_dev *pdev = vdev->pdev;
> +	int bar;
> +	u32 *lp;
> +	u64 mask;
> +
> +	for (bar = 0; bar <= 5; bar++) {
> +		if (pci_resource_start(pdev, bar))
> +			mask = ~(pci_resource_len(pdev, bar) - 1);
> +		else
> +			mask = 0;
> +		lp = (u32 *)vdev->vconfig + PCI_BASE_ADDRESS_0 + 4*bar;
> +		*lp &= (u32)mask;
> +
> +		if (pci_resource_flags(pdev, bar) & IORESOURCE_IO)
> +			*lp |= PCI_BASE_ADDRESS_SPACE_IO;
> +		else if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
> +			*lp |= PCI_BASE_ADDRESS_SPACE_MEMORY;
> +			if (pci_resource_flags(pdev, bar) & IORESOURCE_PREFETCH)
> +				*lp |= PCI_BASE_ADDRESS_MEM_PREFETCH;
> +			if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM_64) {
> +				*lp |= PCI_BASE_ADDRESS_MEM_TYPE_64;
> +				lp++;
> +				*lp &= (u32)(mask >> 32);
> +				bar++;
> +			}
> +		}
> +	}
> +
> +	if (pci_resource_start(pdev, PCI_ROM_RESOURCE))
> +		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
> +	else
> +		mask = 0;
> +	lp = (u32 *)vdev->vconfig + PCI_ROM_ADDRESS;
> +	*lp &= (u32)mask;
> +
> +	vdev->bardirty = 0;
> +}

Hey Tom,

A couple bugs have snuck into the above.  The (u32 *) cast needs to be
done after the vconfig index, and for some reason we're no longer
preserving the ROM enable bit.  I think we need something like this:


diff --git a/drivers/vfio/vfio_pci_config.c b/drivers/vfio/vfio_pci_config.c
index b7de0bf..b1ee352 100644
--- a/drivers/vfio/vfio_pci_config.c
+++ b/drivers/vfio/vfio_pci_config.c
@@ -402,7 +402,7 @@ static void vfio_bar_fixup(struct vfio_dev *vdev)
 			mask = ~(pci_resource_len(pdev, bar) - 1);
 		else
 			mask = 0;
-		lp = (u32 *)vdev->vconfig + PCI_BASE_ADDRESS_0 + 4*bar;
+		lp = (u32 *)(vdev->vconfig + PCI_BASE_ADDRESS_0 + 4*bar);
 		*lp &= (u32)mask;
 
 		if (pci_resource_flags(pdev, bar) & IORESOURCE_IO)
@@ -420,11 +420,12 @@ static void vfio_bar_fixup(struct vfio_dev *vdev)
 		}
 	}
 
-	if (pci_resource_start(pdev, PCI_ROM_RESOURCE))
+	if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
 		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
-	else
+		mask |= PCI_ROM_ADDRESS_ENABLE;
+	} else
 		mask = 0;
-	lp = (u32 *)vdev->vconfig + PCI_ROM_ADDRESS;
+	lp = (u32 *)(vdev->vconfig + PCI_ROM_ADDRESS);
 	*lp &= (u32)mask;
 
 	vdev->bardirty = 0;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/