DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] bus/pci: fix VF bus error for memory access
@ 2020-06-21 17:40 Haiyue Wang
  2020-06-22  6:30 ` [dpdk-dev] [PATCH v2] " Haiyue Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Haiyue Wang @ 2020-06-21 17:40 UTC (permalink / raw)
  To: dev, anatoly.burakov; +Cc: Haiyue Wang, stable

To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
and block MMIO access on disabled memory, it will send a SIGBUS to the
application.

In fact, vfio-pci will enable the memory command when openning the PCI
device, but according to the PCIe specification, this enablement by real
PCI write command doesn't have effect, it still has 0 value:

    Table 9-13 Command Register Changes
Bit Location | PF and VF Register Differences | PF         | VF
             | From Base                      | Attributes | Attributes
-------------+--------------------------------+------------+-----------
             | Memory Space Enable - Does not |            |
             | apply to VFs. Must be hardwired|  Base      |  0b
     1       | to 0b for VFs. VF Memory Space |            |
             | is controlled by the VF MSE bit|            |
             | in the VF Control register.    |            |
-------------+--------------------------------+------------+-----------

So that when the vfio-pci initializes its own PCI configuration space
data called 'vconfig' by reading the VF's real configuration space, it
will have the memory command with 0b value, then, the vfio-pci finds the
BAR memory is disabled by checking the its vconfig space, and the SIGBUS
will be triggerred.

So it needs to enable PCI bus memory command explicitly to avoid access
on disabled memory, which will call vfio-pci ioctl to change the memory
command in vconfig space to 1b.

Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
Cc: stable@dpdk.org

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
Put the long link here, since the patch doesn't support to add so long
line.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476
---
 drivers/bus/pci/linux/pci_vfio.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 64cd84a68..9b6e45da5 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -149,6 +149,38 @@ pci_vfio_get_msix_bar(int fd, struct pci_msix_table *msix_table)
 	return 0;
 }
 
+/* enable PCI bus memory command */
+static int
+pci_vfio_enable_bus_memory(int dev_fd)
+{
+	uint16_t cmd;
+	int ret;
+
+	ret = pread64(dev_fd, &cmd, sizeof(cmd),
+		      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		      PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n");
+		return -1;
+	}
+
+	if (cmd & PCI_COMMAND_MEMORY)
+		return 0;
+
+	cmd |= PCI_COMMAND_MEMORY;
+	ret = pwrite64(dev_fd, &cmd, sizeof(cmd),
+		       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		       PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 /* set PCI bus mastering */
 static int
 pci_vfio_set_bus_master(int dev_fd, bool op)
@@ -427,6 +459,11 @@ pci_rte_vfio_setup_device(struct rte_pci_device *dev, int vfio_dev_fd)
 		return -1;
 	}
 
+	if (pci_vfio_enable_bus_memory(vfio_dev_fd)) {
+		RTE_LOG(ERR, EAL, "Cannot enable bus memory command!\n");
+		return -1;
+	}
+
 	/* set bus mastering for the device */
 	if (pci_vfio_set_bus_master(vfio_dev_fd, true)) {
 		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-- 
2.27.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v2] bus/pci: fix VF bus error for memory access
  2020-06-21 17:40 [dpdk-dev] [PATCH v1] bus/pci: fix VF bus error for memory access Haiyue Wang
@ 2020-06-22  6:30 ` Haiyue Wang
  2020-06-22  8:52   ` Burakov, Anatoly
  2020-06-22 11:13 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2020-06-25  3:50 ` [dpdk-dev] [PATCH v4] " Haiyue Wang
  2 siblings, 1 reply; 16+ messages in thread
From: Haiyue Wang @ 2020-06-22  6:30 UTC (permalink / raw)
  To: dev, anatoly.burakov; +Cc: Haiyue Wang, stable

To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
and block MMIO access on disabled memory, it will send a SIGBUS to the
application:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476

When the application opens the vfio PCI device, the vfio-pci module will
enable the memory bus command through PCI read/write access. According
to the PCIe specification, for VF, the 'Memory Space Enable' is always
zero:

             Table 9-13 Command Register Changes

Bit Location | PF and VF Register Differences | PF         | VF
             | From Base                      | Attributes | Attributes
-------------+--------------------------------+------------+-----------
             | Memory Space Enable - Does not |            |
             | apply to VFs. Must be hardwired|  Base      |  0b
     1       | to 0b for VFs. VF Memory Space |            |
             | is controlled by the VF MSE bit|            |
             | in the VF Control register.    |            |
-------------+--------------------------------+------------+-----------

Then the vfio-pci module initializes its own virtual PCI config space
data ('vconfig') by reading the VF's physical PCI config space, so the
'Memory Space Enable' bit in vconfig will also have 0b value. This will
make the vfio-pci find that the BAR memory is disabled, and the SIGBUS
will be triggerred if access these BARs.

So it needs to enable PCI bus memory command explicitly to avoid access
on disabled memory, which will call vfio-pci virtual PCI read/write API
to set the 'Memory Space Enable' in vconfig space to 1b.

Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
Cc: stable@dpdk.org

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v2: Rewrite the commit log, and put the link into it even it is long.
---
 drivers/bus/pci/linux/pci_vfio.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 64cd84a68..9b6e45da5 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -149,6 +149,38 @@ pci_vfio_get_msix_bar(int fd, struct pci_msix_table *msix_table)
 	return 0;
 }
 
+/* enable PCI bus memory command */
+static int
+pci_vfio_enable_bus_memory(int dev_fd)
+{
+	uint16_t cmd;
+	int ret;
+
+	ret = pread64(dev_fd, &cmd, sizeof(cmd),
+		      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		      PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n");
+		return -1;
+	}
+
+	if (cmd & PCI_COMMAND_MEMORY)
+		return 0;
+
+	cmd |= PCI_COMMAND_MEMORY;
+	ret = pwrite64(dev_fd, &cmd, sizeof(cmd),
+		       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		       PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 /* set PCI bus mastering */
 static int
 pci_vfio_set_bus_master(int dev_fd, bool op)
@@ -427,6 +459,11 @@ pci_rte_vfio_setup_device(struct rte_pci_device *dev, int vfio_dev_fd)
 		return -1;
 	}
 
+	if (pci_vfio_enable_bus_memory(vfio_dev_fd)) {
+		RTE_LOG(ERR, EAL, "Cannot enable bus memory command!\n");
+		return -1;
+	}
+
 	/* set bus mastering for the device */
 	if (pci_vfio_set_bus_master(vfio_dev_fd, true)) {
 		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-- 
2.27.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2] bus/pci: fix VF bus error for memory access
  2020-06-22  6:30 ` [dpdk-dev] [PATCH v2] " Haiyue Wang
@ 2020-06-22  8:52   ` Burakov, Anatoly
  2020-06-22 11:25     ` Wang, Haiyue
  0 siblings, 1 reply; 16+ messages in thread
From: Burakov, Anatoly @ 2020-06-22  8:52 UTC (permalink / raw)
  To: Haiyue Wang, dev; +Cc: stable

On 22-Jun-20 7:30 AM, Haiyue Wang wrote:
> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> and block MMIO access on disabled memory, it will send a SIGBUS to the
> application:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476
> 
> When the application opens the vfio PCI device, the vfio-pci module will
> enable the memory bus command through PCI read/write access. According
> to the PCIe specification, for VF, the 'Memory Space Enable' is always
> zero:
> 
>               Table 9-13 Command Register Changes
> 
> Bit Location | PF and VF Register Differences | PF         | VF
>               | From Base                      | Attributes | Attributes
> -------------+--------------------------------+------------+-----------
>               | Memory Space Enable - Does not |            |
>               | apply to VFs. Must be hardwired|  Base      |  0b
>       1       | to 0b for VFs. VF Memory Space |            |
>               | is controlled by the VF MSE bit|            |
>               | in the VF Control register.    |            |
> -------------+--------------------------------+------------+-----------
> 
> Then the vfio-pci module initializes its own virtual PCI config space
> data ('vconfig') by reading the VF's physical PCI config space, so the
> 'Memory Space Enable' bit in vconfig will also have 0b value. This will
> make the vfio-pci find that the BAR memory is disabled, and the SIGBUS
> will be triggerred if access these BARs.
> 
> So it needs to enable PCI bus memory command explicitly to avoid access
> on disabled memory, which will call vfio-pci virtual PCI read/write API
> to set the 'Memory Space Enable' in vconfig space to 1b.
> 
> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

The patch itself looks good, but i wonder how far back do these go, and 
do we need workarounds for older kernels. IIRC v17.11 is still a 
supported release, and its kernel support might go back all the way to 
v3.6 when VFIO was first introduced.

> ---
> v2: Rewrite the commit log, and put the link into it even it is long.
> ---
>   drivers/bus/pci/linux/pci_vfio.c | 37 ++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
> index 64cd84a68..9b6e45da5 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -149,6 +149,38 @@ pci_vfio_get_msix_bar(int fd, struct pci_msix_table *msix_table)
>   	return 0;
>   }
>   
> +/* enable PCI bus memory command */
> +static int
> +pci_vfio_enable_bus_memory(int dev_fd)
> +{
> +	uint16_t cmd;
> +	int ret;
> +
> +	ret = pread64(dev_fd, &cmd, sizeof(cmd),
> +		      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
> +		      PCI_COMMAND);
> +
> +	if (ret != sizeof(cmd)) {
> +		RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n");
> +		return -1;
> +	}
> +
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		return 0;
> +
> +	cmd |= PCI_COMMAND_MEMORY;
> +	ret = pwrite64(dev_fd, &cmd, sizeof(cmd),
> +		       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
> +		       PCI_COMMAND);
> +
> +	if (ret != sizeof(cmd)) {
> +		RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>   /* set PCI bus mastering */
>   static int
>   pci_vfio_set_bus_master(int dev_fd, bool op)
> @@ -427,6 +459,11 @@ pci_rte_vfio_setup_device(struct rte_pci_device *dev, int vfio_dev_fd)
>   		return -1;
>   	}
>   
> +	if (pci_vfio_enable_bus_memory(vfio_dev_fd)) {
> +		RTE_LOG(ERR, EAL, "Cannot enable bus memory command!\n");

Nitpick, but i think the word "command" is unneeded here :)

> +		return -1;
> +	}
> +
>   	/* set bus mastering for the device */
>   	if (pci_vfio_set_bus_master(vfio_dev_fd, true)) {
>   		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
> 


-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v3] bus/pci: fix VF bus error for memory access
  2020-06-21 17:40 [dpdk-dev] [PATCH v1] bus/pci: fix VF bus error for memory access Haiyue Wang
  2020-06-22  6:30 ` [dpdk-dev] [PATCH v2] " Haiyue Wang
@ 2020-06-22 11:13 ` Haiyue Wang
  2020-06-22 12:11   ` Burakov, Anatoly
                     ` (2 more replies)
  2020-06-25  3:50 ` [dpdk-dev] [PATCH v4] " Haiyue Wang
  2 siblings, 3 replies; 16+ messages in thread
From: Haiyue Wang @ 2020-06-22 11:13 UTC (permalink / raw)
  To: dev, anatoly.burakov; +Cc: Haiyue Wang, stable

To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
and block MMIO access on disabled memory, it will send a SIGBUS to the
application:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476

When the application opens the vfio PCI device, the vfio-pci module will
enable the bus memory space through PCI read/write access. According to
the PCIe specification, the 'Memory Space Enable' is always zero for VF:

             Table 9-13 Command Register Changes

Bit Location | PF and VF Register Differences | PF         | VF
             | From Base                      | Attributes | Attributes
-------------+--------------------------------+------------+-----------
             | Memory Space Enable - Does not |            |
             | apply to VFs. Must be hardwired|  Base      |  0b
     1       | to 0b for VFs. VF Memory Space |            |
             | is controlled by the VF MSE bit|            |
             | in the VF Control register.    |            |
-------------+--------------------------------+------------+-----------

Afterwards the vfio-pci will initialize its own virtual PCI config space
data ('vconfig') by reading the VF's physical PCI config space, then the
'Memory Space Enable' bit in vconfig will always be 0b value. This will
make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
will be triggerred if access these BARs.

By investigation, the VF PCI device *passthrough* into the Guest OS by
QEMU has the 'Memory Space Enable' with 1b value. That's because every
PCI driver will start to enable the memory space, and this action will
be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
VF runs in host OS has 'Mem-'.

Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
bus memory space explicitly to avoid access on disabled memory.

Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
Cc: stable@dpdk.org

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v3: update the commit log, and fix one debug log with redundant
description.
v2: Rewrite the commit log, and put the link into it even it is long.
---
 drivers/bus/pci/linux/pci_vfio.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 64cd84a68..ba60e7ce9 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -149,6 +149,38 @@ pci_vfio_get_msix_bar(int fd, struct pci_msix_table *msix_table)
 	return 0;
 }
 
+/* enable PCI bus memory space */
+static int
+pci_vfio_enable_bus_memory(int dev_fd)
+{
+	uint16_t cmd;
+	int ret;
+
+	ret = pread64(dev_fd, &cmd, sizeof(cmd),
+		      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		      PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n");
+		return -1;
+	}
+
+	if (cmd & PCI_COMMAND_MEMORY)
+		return 0;
+
+	cmd |= PCI_COMMAND_MEMORY;
+	ret = pwrite64(dev_fd, &cmd, sizeof(cmd),
+		       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		       PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 /* set PCI bus mastering */
 static int
 pci_vfio_set_bus_master(int dev_fd, bool op)
@@ -427,6 +459,11 @@ pci_rte_vfio_setup_device(struct rte_pci_device *dev, int vfio_dev_fd)
 		return -1;
 	}
 
+	if (pci_vfio_enable_bus_memory(vfio_dev_fd)) {
+		RTE_LOG(ERR, EAL, "Cannot enable bus memory!\n");
+		return -1;
+	}
+
 	/* set bus mastering for the device */
 	if (pci_vfio_set_bus_master(vfio_dev_fd, true)) {
 		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-- 
2.27.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2] bus/pci: fix VF bus error for memory access
  2020-06-22  8:52   ` Burakov, Anatoly
@ 2020-06-22 11:25     ` Wang, Haiyue
  0 siblings, 0 replies; 16+ messages in thread
From: Wang, Haiyue @ 2020-06-22 11:25 UTC (permalink / raw)
  To: Burakov, Anatoly, dev; +Cc: stable

> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Monday, June 22, 2020 16:53
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: Re: [PATCH v2] bus/pci: fix VF bus error for memory access
> 
> On 22-Jun-20 7:30 AM, Haiyue Wang wrote:
> > To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> > and block MMIO access on disabled memory, it will send a SIGBUS to the
> > application:
> >
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee
> 1dcde08fc0eb8476
> >
> > When the application opens the vfio PCI device, the vfio-pci module will
> > enable the memory bus command through PCI read/write access. According
> > to the PCIe specification, for VF, the 'Memory Space Enable' is always
> > zero:
> >
> >               Table 9-13 Command Register Changes
> >
> > Bit Location | PF and VF Register Differences | PF         | VF
> >               | From Base                      | Attributes | Attributes
> > -------------+--------------------------------+------------+-----------
> >               | Memory Space Enable - Does not |            |
> >               | apply to VFs. Must be hardwired|  Base      |  0b
> >       1       | to 0b for VFs. VF Memory Space |            |
> >               | is controlled by the VF MSE bit|            |
> >               | in the VF Control register.    |            |
> > -------------+--------------------------------+------------+-----------
> >
> > Then the vfio-pci module initializes its own virtual PCI config space
> > data ('vconfig') by reading the VF's physical PCI config space, so the
> > 'Memory Space Enable' bit in vconfig will also have 0b value. This will
> > make the vfio-pci find that the BAR memory is disabled, and the SIGBUS
> > will be triggerred if access these BARs.
> >
> > So it needs to enable PCI bus memory command explicitly to avoid access
> > on disabled memory, which will call vfio-pci virtual PCI read/write API
> > to set the 'Memory Space Enable' in vconfig space to 1b.
> >
> > Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> 
> The patch itself looks good, but i wonder how far back do these go, and
> do we need workarounds for older kernels. IIRC v17.11 is still a
> supported release, and its kernel support might go back all the way to
> v3.6 when VFIO was first introduced.
> 

Seems not a workaround, since I found the VF in qemu has 1 for memory space,
please see the v3 commit log for detail. I added more investigation in it.

> > ---
> > v2: Rewrite the commit log, and put the link into it even it is long.
> > ---
> >   drivers/bus/pci/linux/pci_vfio.c | 37 ++++++++++++++++++++++++++++++++
> >   1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
> > index 64cd84a68..9b6e45da5 100644
> > --- a/drivers/bus/pci/linux/pci_vfio.c
> > +++ b/drivers/bus/pci/linux/pci_vfio.c
> > @@ -149,6 +149,38 @@ pci_vfio_get_msix_bar(int fd, struct pci_msix_table *msix_table)
> >   	return 0;
> >   }
> >
> > +/* enable PCI bus memory command */
> > +static int
> > +pci_vfio_enable_bus_memory(int dev_fd)
> > +{
> > +	uint16_t cmd;
> > +	int ret;
> > +
> > +	ret = pread64(dev_fd, &cmd, sizeof(cmd),
> > +		      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
> > +		      PCI_COMMAND);
> > +
> > +	if (ret != sizeof(cmd)) {
> > +		RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n");
> > +		return -1;
> > +	}
> > +
> > +	if (cmd & PCI_COMMAND_MEMORY)
> > +		return 0;
> > +
> > +	cmd |= PCI_COMMAND_MEMORY;
> > +	ret = pwrite64(dev_fd, &cmd, sizeof(cmd),
> > +		       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
> > +		       PCI_COMMAND);
> > +
> > +	if (ret != sizeof(cmd)) {
> > +		RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   /* set PCI bus mastering */
> >   static int
> >   pci_vfio_set_bus_master(int dev_fd, bool op)
> > @@ -427,6 +459,11 @@ pci_rte_vfio_setup_device(struct rte_pci_device *dev, int vfio_dev_fd)
> >   		return -1;
> >   	}
> >
> > +	if (pci_vfio_enable_bus_memory(vfio_dev_fd)) {
> > +		RTE_LOG(ERR, EAL, "Cannot enable bus memory command!\n");
> 
> Nitpick, but i think the word "command" is unneeded here :)
> 

Fixed in v3.

> > +		return -1;
> > +	}
> > +
> >   	/* set bus mastering for the device */
> >   	if (pci_vfio_set_bus_master(vfio_dev_fd, true)) {
> >   		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
> >
> 
> 
> --
> Thanks,
> Anatoly

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v3] bus/pci: fix VF bus error for memory access
  2020-06-22 11:13 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
@ 2020-06-22 12:11   ` Burakov, Anatoly
  2020-06-23 15:12   ` Harman Kalra
  2020-06-24 20:01   ` David Marchand
  2 siblings, 0 replies; 16+ messages in thread
From: Burakov, Anatoly @ 2020-06-22 12:11 UTC (permalink / raw)
  To: Haiyue Wang, dev; +Cc: stable

On 22-Jun-20 12:13 PM, Haiyue Wang wrote:
> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> and block MMIO access on disabled memory, it will send a SIGBUS to the
> application:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476
> 
> When the application opens the vfio PCI device, the vfio-pci module will
> enable the bus memory space through PCI read/write access. According to
> the PCIe specification, the 'Memory Space Enable' is always zero for VF:
> 
>               Table 9-13 Command Register Changes
> 
> Bit Location | PF and VF Register Differences | PF         | VF
>               | From Base                      | Attributes | Attributes
> -------------+--------------------------------+------------+-----------
>               | Memory Space Enable - Does not |            |
>               | apply to VFs. Must be hardwired|  Base      |  0b
>       1       | to 0b for VFs. VF Memory Space |            |
>               | is controlled by the VF MSE bit|            |
>               | in the VF Control register.    |            |
> -------------+--------------------------------+------------+-----------
> 
> Afterwards the vfio-pci will initialize its own virtual PCI config space
> data ('vconfig') by reading the VF's physical PCI config space, then the
> 'Memory Space Enable' bit in vconfig will always be 0b value. This will
> make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
> will be triggerred if access these BARs.
> 
> By investigation, the VF PCI device *passthrough* into the Guest OS by
> QEMU has the 'Memory Space Enable' with 1b value. That's because every
> PCI driver will start to enable the memory space, and this action will
> be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
> Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
> VF runs in host OS has 'Mem-'.
> 
> Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
> bus memory space explicitly to avoid access on disabled memory.
> 
> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> v3: update the commit log, and fix one debug log with redundant
> description.
> v2: Rewrite the commit log, and put the link into it even it is long.
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v3] bus/pci: fix VF bus error for memory access
  2020-06-22 11:13 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2020-06-22 12:11   ` Burakov, Anatoly
@ 2020-06-23 15:12   ` Harman Kalra
  2020-06-24 20:01   ` David Marchand
  2 siblings, 0 replies; 16+ messages in thread
From: Harman Kalra @ 2020-06-23 15:12 UTC (permalink / raw)
  To: Haiyue Wang; +Cc: dev, anatoly.burakov, stable

On Mon, Jun 22, 2020 at 07:13:51PM +0800, Haiyue Wang wrote:
> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> and block MMIO access on disabled memory, it will send a SIGBUS to the
> application:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_commit_-3Fid-3Dabafbc551fddede3e0a08dee1dcde08fc0eb8476&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=5ESHPj7V-7JdkxT_Z_SU6RrS37ys4UXudBQ_rrS5LRo&m=L4wvNYSLDBSsweJGHfGSw3cDNDi0ioJdsQbH2-pEVqE&s=9v3r9tYw6p5Paet2O3Nc2IPdOL1-o77RjYJx5H8G0vc&e= 
> 
> When the application opens the vfio PCI device, the vfio-pci module will
> enable the bus memory space through PCI read/write access. According to
> the PCIe specification, the 'Memory Space Enable' is always zero for VF:
> 
>              Table 9-13 Command Register Changes
> 
> Bit Location | PF and VF Register Differences | PF         | VF
>              | From Base                      | Attributes | Attributes
> -------------+--------------------------------+------------+-----------
>              | Memory Space Enable - Does not |            |
>              | apply to VFs. Must be hardwired|  Base      |  0b
>      1       | to 0b for VFs. VF Memory Space |            |
>              | is controlled by the VF MSE bit|            |
>              | in the VF Control register.    |            |
> -------------+--------------------------------+------------+-----------
> 
> Afterwards the vfio-pci will initialize its own virtual PCI config space
> data ('vconfig') by reading the VF's physical PCI config space, then the
> 'Memory Space Enable' bit in vconfig will always be 0b value. This will
> make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
> will be triggerred if access these BARs.
> 
> By investigation, the VF PCI device *passthrough* into the Guest OS by
> QEMU has the 'Memory Space Enable' with 1b value. That's because every
> PCI driver will start to enable the memory space, and this action will
> be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
> Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
> VF runs in host OS has 'Mem-'.
> 
> Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
> bus memory space explicitly to avoid access on disabled memory.
> 
> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Tested-by: Harman Kalra <hkalra@marvell.com>

> ---
> v3: update the commit log, and fix one debug log with redundant
> description.
> v2: Rewrite the commit log, and put the link into it even it is long.
> ---
>  drivers/bus/pci/linux/pci_vfio.c | 37 ++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
> index 64cd84a68..ba60e7ce9 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -149,6 +149,38 @@ pci_vfio_get_msix_bar(int fd, struct pci_msix_table *msix_table)
>  	return 0;
>  }
>  
> +/* enable PCI bus memory space */
> +static int
> +pci_vfio_enable_bus_memory(int dev_fd)
> +{
> +	uint16_t cmd;
> +	int ret;
> +
> +	ret = pread64(dev_fd, &cmd, sizeof(cmd),
> +		      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
> +		      PCI_COMMAND);
> +
> +	if (ret != sizeof(cmd)) {
> +		RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n");
> +		return -1;
> +	}
> +
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		return 0;
> +
> +	cmd |= PCI_COMMAND_MEMORY;
> +	ret = pwrite64(dev_fd, &cmd, sizeof(cmd),
> +		       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
> +		       PCI_COMMAND);
> +
> +	if (ret != sizeof(cmd)) {
> +		RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  /* set PCI bus mastering */
>  static int
>  pci_vfio_set_bus_master(int dev_fd, bool op)
> @@ -427,6 +459,11 @@ pci_rte_vfio_setup_device(struct rte_pci_device *dev, int vfio_dev_fd)
>  		return -1;
>  	}
>  
> +	if (pci_vfio_enable_bus_memory(vfio_dev_fd)) {
> +		RTE_LOG(ERR, EAL, "Cannot enable bus memory!\n");
> +		return -1;
> +	}
> +
>  	/* set bus mastering for the device */
>  	if (pci_vfio_set_bus_master(vfio_dev_fd, true)) {
>  		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v3] bus/pci: fix VF bus error for memory access
  2020-06-22 11:13 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2020-06-22 12:11   ` Burakov, Anatoly
  2020-06-23 15:12   ` Harman Kalra
@ 2020-06-24 20:01   ` David Marchand
  2020-06-25  4:01     ` Wang, Haiyue
  2 siblings, 1 reply; 16+ messages in thread
From: David Marchand @ 2020-06-24 20:01 UTC (permalink / raw)
  To: Haiyue Wang; +Cc: dev, Burakov, Anatoly, dpdk stable, Maxime Coquelin

On Mon, Jun 22, 2020 at 1:23 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
>
> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> and block MMIO access on disabled memory, it will send a SIGBUS to the
> application:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476
>
> When the application opens the vfio PCI device, the vfio-pci module will
> enable the bus memory space through PCI read/write access. According to
> the PCIe specification, the 'Memory Space Enable' is always zero for VF:
>
>              Table 9-13 Command Register Changes
>
> Bit Location | PF and VF Register Differences | PF         | VF
>              | From Base                      | Attributes | Attributes
> -------------+--------------------------------+------------+-----------
>              | Memory Space Enable - Does not |            |
>              | apply to VFs. Must be hardwired|  Base      |  0b
>      1       | to 0b for VFs. VF Memory Space |            |
>              | is controlled by the VF MSE bit|            |
>              | in the VF Control register.    |            |
> -------------+--------------------------------+------------+-----------
>
> Afterwards the vfio-pci will initialize its own virtual PCI config space
> data ('vconfig') by reading the VF's physical PCI config space, then the
> 'Memory Space Enable' bit in vconfig will always be 0b value. This will
> make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
> will be triggerred if access these BARs.

triggered

>
> By investigation, the VF PCI device *passthrough* into the Guest OS by
> QEMU has the 'Memory Space Enable' with 1b value. That's because every
> PCI driver will start to enable the memory space, and this action will
> be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
> Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
> VF runs in host OS has 'Mem-'.
>
> Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
> bus memory space explicitly to avoid access on disabled memory.
>
> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> Cc: stable@dpdk.org
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Ouch, we just hit it.
Thanks for fixing!

Tested-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v4] bus/pci: fix VF bus error for memory access
  2020-06-21 17:40 [dpdk-dev] [PATCH v1] bus/pci: fix VF bus error for memory access Haiyue Wang
  2020-06-22  6:30 ` [dpdk-dev] [PATCH v2] " Haiyue Wang
  2020-06-22 11:13 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
@ 2020-06-25  3:50 ` Haiyue Wang
  2020-06-25 14:09   ` David Marchand
  2 siblings, 1 reply; 16+ messages in thread
From: Haiyue Wang @ 2020-06-25  3:50 UTC (permalink / raw)
  To: dev, anatoly.burakov; +Cc: Haiyue Wang, stable, Harman Kalra, David Marchand

To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
and block MMIO access on disabled memory, it will send a SIGBUS to the
application:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476

When the application opens the vfio PCI device, the vfio-pci module will
enable the bus memory space through PCI read/write access. According to
the PCIe specification, the 'Memory Space Enable' is always zero for VF:

             Table 9-13 Command Register Changes

Bit Location | PF and VF Register Differences | PF         | VF
             | From Base                      | Attributes | Attributes
-------------+--------------------------------+------------+-----------
             | Memory Space Enable - Does not |            |
             | apply to VFs. Must be hardwired|  Base      |  0b
     1       | to 0b for VFs. VF Memory Space |            |
             | is controlled by the VF MSE bit|            |
             | in the VF Control register.    |            |
-------------+--------------------------------+------------+-----------

Afterwards the vfio-pci will initialize its own virtual PCI config space
data ('vconfig') by reading the VF's physical PCI config space, then the
'Memory Space Enable' bit in vconfig will always be 0b value. This will
make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
will be triggered if access these BARs.

By investigation, the VF PCI device *passthrough* into the Guest OS by
QEMU has the 'Memory Space Enable' with 1b value. That's because every
PCI driver will start to enable the memory space, and this action will
be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
VF runs in host OS has 'Mem-'.

Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
bus memory space explicitly to avoid access on disabled memory.

Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
Cc: stable@dpdk.org

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Tested-by: Harman Kalra <hkalra@marvell.com>
Tested-by: David Marchand <david.marchand@redhat.com>
---
v4: Fix commit message typo.
v3: Update the commit log, and fix one debug log with redundant
description.
v2: Rewrite the commit log, and put the link into it even it is long.
---
 drivers/bus/pci/linux/pci_vfio.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 64cd84a68..ba60e7ce9 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -149,6 +149,38 @@ pci_vfio_get_msix_bar(int fd, struct pci_msix_table *msix_table)
 	return 0;
 }
 
+/* enable PCI bus memory space */
+static int
+pci_vfio_enable_bus_memory(int dev_fd)
+{
+	uint16_t cmd;
+	int ret;
+
+	ret = pread64(dev_fd, &cmd, sizeof(cmd),
+		      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		      PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot read command from PCI config space!\n");
+		return -1;
+	}
+
+	if (cmd & PCI_COMMAND_MEMORY)
+		return 0;
+
+	cmd |= PCI_COMMAND_MEMORY;
+	ret = pwrite64(dev_fd, &cmd, sizeof(cmd),
+		       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) +
+		       PCI_COMMAND);
+
+	if (ret != sizeof(cmd)) {
+		RTE_LOG(ERR, EAL, "Cannot write command to PCI config space!\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 /* set PCI bus mastering */
 static int
 pci_vfio_set_bus_master(int dev_fd, bool op)
@@ -427,6 +459,11 @@ pci_rte_vfio_setup_device(struct rte_pci_device *dev, int vfio_dev_fd)
 		return -1;
 	}
 
+	if (pci_vfio_enable_bus_memory(vfio_dev_fd)) {
+		RTE_LOG(ERR, EAL, "Cannot enable bus memory!\n");
+		return -1;
+	}
+
 	/* set bus mastering for the device */
 	if (pci_vfio_set_bus_master(vfio_dev_fd, true)) {
 		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-- 
2.27.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v3] bus/pci: fix VF bus error for memory access
  2020-06-24 20:01   ` David Marchand
@ 2020-06-25  4:01     ` Wang, Haiyue
  0 siblings, 0 replies; 16+ messages in thread
From: Wang, Haiyue @ 2020-06-25  4:01 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Burakov, Anatoly, dpdk stable, Maxime Coquelin

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, June 25, 2020 04:01
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev <dev@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com>; dpdk stable <stable@dpdk.org>;
> Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH v3] bus/pci: fix VF bus error for memory access
> 
> On Mon, Jun 22, 2020 at 1:23 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
> >
> > To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> > and block MMIO access on disabled memory, it will send a SIGBUS to the
> > application:
> >
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee
> 1dcde08fc0eb8476
> >
> > When the application opens the vfio PCI device, the vfio-pci module will
> > enable the bus memory space through PCI read/write access. According to
> > the PCIe specification, the 'Memory Space Enable' is always zero for VF:
> >
> >              Table 9-13 Command Register Changes
> >
> > Bit Location | PF and VF Register Differences | PF         | VF
> >              | From Base                      | Attributes | Attributes
> > -------------+--------------------------------+------------+-----------
> >              | Memory Space Enable - Does not |            |
> >              | apply to VFs. Must be hardwired|  Base      |  0b
> >      1       | to 0b for VFs. VF Memory Space |            |
> >              | is controlled by the VF MSE bit|            |
> >              | in the VF Control register.    |            |
> > -------------+--------------------------------+------------+-----------
> >
> > Afterwards the vfio-pci will initialize its own virtual PCI config space
> > data ('vconfig') by reading the VF's physical PCI config space, then the
> > 'Memory Space Enable' bit in vconfig will always be 0b value. This will
> > make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
> > will be triggerred if access these BARs.
> 
> triggered
> 

Fixed in v4.

> >
> > By investigation, the VF PCI device *passthrough* into the Guest OS by
> > QEMU has the 'Memory Space Enable' with 1b value. That's because every
> > PCI driver will start to enable the memory space, and this action will
> > be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
> > Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
> > VF runs in host OS has 'Mem-'.
> >
> > Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
> > bus memory space explicitly to avoid access on disabled memory.
> >
> > Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> 
> Ouch, we just hit it.
> Thanks for fixing!
> 
> Tested-by: David Marchand <david.marchand@redhat.com>
> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4] bus/pci: fix VF bus error for memory access
  2020-06-25  3:50 ` [dpdk-dev] [PATCH v4] " Haiyue Wang
@ 2020-06-25 14:09   ` David Marchand
  2020-06-25 16:45     ` Kevin Traynor
  0 siblings, 1 reply; 16+ messages in thread
From: David Marchand @ 2020-06-25 14:09 UTC (permalink / raw)
  To: Haiyue Wang, Kevin Traynor, Luca Boccassi
  Cc: dev, Burakov, Anatoly, dpdk stable, Harman Kalra

On Thu, Jun 25, 2020 at 6:00 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
>
> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> and block MMIO access on disabled memory, it will send a SIGBUS to the
> application:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476
>
> When the application opens the vfio PCI device, the vfio-pci module will
> enable the bus memory space through PCI read/write access. According to
> the PCIe specification, the 'Memory Space Enable' is always zero for VF:
>
>              Table 9-13 Command Register Changes
>
> Bit Location | PF and VF Register Differences | PF         | VF
>              | From Base                      | Attributes | Attributes
> -------------+--------------------------------+------------+-----------
>              | Memory Space Enable - Does not |            |
>              | apply to VFs. Must be hardwired|  Base      |  0b
>      1       | to 0b for VFs. VF Memory Space |            |
>              | is controlled by the VF MSE bit|            |
>              | in the VF Control register.    |            |
> -------------+--------------------------------+------------+-----------
>
> Afterwards the vfio-pci will initialize its own virtual PCI config space
> data ('vconfig') by reading the VF's physical PCI config space, then the
> 'Memory Space Enable' bit in vconfig will always be 0b value. This will
> make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
> will be triggered if access these BARs.
>
> By investigation, the VF PCI device *passthrough* into the Guest OS by
> QEMU has the 'Memory Space Enable' with 1b value. That's because every
> PCI driver will start to enable the memory space, and this action will
> be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
> Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
> VF runs in host OS has 'Mem-'.
>
> Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
> bus memory space explicitly to avoid access on disabled memory.
>
> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> Cc: stable@dpdk.org
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Tested-by: Harman Kalra <hkalra@marvell.com>
> Tested-by: David Marchand <david.marchand@redhat.com>
Tested-by: Thierry Martin <thierry.martin.public@gmail.com>

Applied, thanks again Haiyue.


Kevin, Luca,

I can see that some distros have already started backporting the fix
in kernel (fc31, fc32 and rhel7 at least for what I saw).
18.11 and 19.11 will need this fix at some point.
I'll let you decide on the proper timing.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4] bus/pci: fix VF bus error for memory access
  2020-06-25 14:09   ` David Marchand
@ 2020-06-25 16:45     ` Kevin Traynor
  2020-06-25 18:33       ` Wang, Haiyue
  0 siblings, 1 reply; 16+ messages in thread
From: Kevin Traynor @ 2020-06-25 16:45 UTC (permalink / raw)
  To: David Marchand, Haiyue Wang, Luca Boccassi
  Cc: dev, Burakov, Anatoly, dpdk stable, Harman Kalra

On 25/06/2020 15:09, David Marchand wrote:
> On Thu, Jun 25, 2020 at 6:00 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
>>
>> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
>> and block MMIO access on disabled memory, it will send a SIGBUS to the
>> application:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee1dcde08fc0eb8476
>>
>> When the application opens the vfio PCI device, the vfio-pci module will
>> enable the bus memory space through PCI read/write access. According to
>> the PCIe specification, the 'Memory Space Enable' is always zero for VF:
>>
>>              Table 9-13 Command Register Changes
>>
>> Bit Location | PF and VF Register Differences | PF         | VF
>>              | From Base                      | Attributes | Attributes
>> -------------+--------------------------------+------------+-----------
>>              | Memory Space Enable - Does not |            |
>>              | apply to VFs. Must be hardwired|  Base      |  0b
>>      1       | to 0b for VFs. VF Memory Space |            |
>>              | is controlled by the VF MSE bit|            |
>>              | in the VF Control register.    |            |
>> -------------+--------------------------------+------------+-----------
>>
>> Afterwards the vfio-pci will initialize its own virtual PCI config space
>> data ('vconfig') by reading the VF's physical PCI config space, then the
>> 'Memory Space Enable' bit in vconfig will always be 0b value. This will
>> make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
>> will be triggered if access these BARs.
>>
>> By investigation, the VF PCI device *passthrough* into the Guest OS by
>> QEMU has the 'Memory Space Enable' with 1b value. That's because every
>> PCI driver will start to enable the memory space, and this action will
>> be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
>> Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
>> VF runs in host OS has 'Mem-'.
>>
>> Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
>> bus memory space explicitly to avoid access on disabled memory.
>>
>> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> Tested-by: Harman Kalra <hkalra@marvell.com>
>> Tested-by: David Marchand <david.marchand@redhat.com>
> Tested-by: Thierry Martin <thierry.martin.public@gmail.com>
> 
> Applied, thanks again Haiyue.
> 
> 
> Kevin, Luca,
> 
> I can see that some distros have already started backporting the fix
> in kernel (fc31, fc32 and rhel7 at least for what I saw).
> 18.11 and 19.11 will need this fix at some point.
> I'll let you decide on the proper timing.
> 
> 

It looks an important fix. I think it's worth having in 18.11.9. I will
apply and create an 18.11.9-rc2 tomorrow, so if anyone hasn't started
validation already, they can validate with it in.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4] bus/pci: fix VF bus error for memory access
  2020-06-25 16:45     ` Kevin Traynor
@ 2020-06-25 18:33       ` Wang, Haiyue
  2020-06-26  9:10         ` Kevin Traynor
  2020-06-26  9:17         ` David Marchand
  0 siblings, 2 replies; 16+ messages in thread
From: Wang, Haiyue @ 2020-06-25 18:33 UTC (permalink / raw)
  To: Kevin Traynor, David Marchand, Luca Boccassi
  Cc: dev, Burakov, Anatoly, dpdk stable, Harman Kalra

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Friday, June 26, 2020 00:46
> To: David Marchand <david.marchand@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com>; Luca Boccassi
> <bluca@debian.org>
> Cc: dev <dev@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com>; dpdk stable <stable@dpdk.org>;
> Harman Kalra <hkalra@marvell.com>
> Subject: Re: [PATCH v4] bus/pci: fix VF bus error for memory access
> 
> On 25/06/2020 15:09, David Marchand wrote:
> > On Thu, Jun 25, 2020 at 6:00 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
> >>
> >> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
> >> and block MMIO access on disabled memory, it will send a SIGBUS to the
> >> application:
> >>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee
> 1dcde08fc0eb8476
> >>
> >> When the application opens the vfio PCI device, the vfio-pci module will
> >> enable the bus memory space through PCI read/write access. According to
> >> the PCIe specification, the 'Memory Space Enable' is always zero for VF:
> >>
> >>              Table 9-13 Command Register Changes
> >>
> >> Bit Location | PF and VF Register Differences | PF         | VF
> >>              | From Base                      | Attributes | Attributes
> >> -------------+--------------------------------+------------+-----------
> >>              | Memory Space Enable - Does not |            |
> >>              | apply to VFs. Must be hardwired|  Base      |  0b
> >>      1       | to 0b for VFs. VF Memory Space |            |
> >>              | is controlled by the VF MSE bit|            |
> >>              | in the VF Control register.    |            |
> >> -------------+--------------------------------+------------+-----------
> >>
> >> Afterwards the vfio-pci will initialize its own virtual PCI config space
> >> data ('vconfig') by reading the VF's physical PCI config space, then the
> >> 'Memory Space Enable' bit in vconfig will always be 0b value. This will
> >> make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
> >> will be triggered if access these BARs.
> >>
> >> By investigation, the VF PCI device *passthrough* into the Guest OS by
> >> QEMU has the 'Memory Space Enable' with 1b value. That's because every
> >> PCI driver will start to enable the memory space, and this action will
> >> be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
> >> Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
> >> VF runs in host OS has 'Mem-'.
> >>
> >> Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
> >> bus memory space explicitly to avoid access on disabled memory.
> >>
> >> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> Tested-by: Harman Kalra <hkalra@marvell.com>
> >> Tested-by: David Marchand <david.marchand@redhat.com>
> > Tested-by: Thierry Martin <thierry.martin.public@gmail.com>
> >
> > Applied, thanks again Haiyue.
> >
> >
> > Kevin, Luca,
> >
> > I can see that some distros have already started backporting the fix
> > in kernel (fc31, fc32 and rhel7 at least for what I saw).
> > 18.11 and 19.11 will need this fix at some point.
> > I'll let you decide on the proper timing.
> >
> >
> 
> It looks an important fix. I think it's worth having in 18.11.9. I will
> apply and create an 18.11.9-rc2 tomorrow, so if anyone hasn't started
> validation already, they can validate with it in.

Alex post a fix in kernel just now. So looks like the DPDK patch is nice
to have, not a MUST. ;-)

https://lore.kernel.org/kvm/159310421505.27590.16617666489295503039.stgit@gimli.home/T/#u


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4] bus/pci: fix VF bus error for memory access
  2020-06-25 18:33       ` Wang, Haiyue
@ 2020-06-26  9:10         ` Kevin Traynor
  2020-06-26  9:17         ` David Marchand
  1 sibling, 0 replies; 16+ messages in thread
From: Kevin Traynor @ 2020-06-26  9:10 UTC (permalink / raw)
  To: Wang, Haiyue, David Marchand, Luca Boccassi
  Cc: dev, Burakov, Anatoly, dpdk stable, Harman Kalra

On 25/06/2020 19:33, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Friday, June 26, 2020 00:46
>> To: David Marchand <david.marchand@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com>; Luca Boccassi
>> <bluca@debian.org>
>> Cc: dev <dev@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com>; dpdk stable <stable@dpdk.org>;
>> Harman Kalra <hkalra@marvell.com>
>> Subject: Re: [PATCH v4] bus/pci: fix VF bus error for memory access
>>
>> On 25/06/2020 15:09, David Marchand wrote:
>>> On Thu, Jun 25, 2020 at 6:00 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
>>>>
>>>> To fix CVE-2020-12888, the linux vfio-pci module will invalidate mmaps
>>>> and block MMIO access on disabled memory, it will send a SIGBUS to the
>>>> application:
>>>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abafbc551fddede3e0a08dee
>> 1dcde08fc0eb8476
>>>>
>>>> When the application opens the vfio PCI device, the vfio-pci module will
>>>> enable the bus memory space through PCI read/write access. According to
>>>> the PCIe specification, the 'Memory Space Enable' is always zero for VF:
>>>>
>>>>              Table 9-13 Command Register Changes
>>>>
>>>> Bit Location | PF and VF Register Differences | PF         | VF
>>>>              | From Base                      | Attributes | Attributes
>>>> -------------+--------------------------------+------------+-----------
>>>>              | Memory Space Enable - Does not |            |
>>>>              | apply to VFs. Must be hardwired|  Base      |  0b
>>>>      1       | to 0b for VFs. VF Memory Space |            |
>>>>              | is controlled by the VF MSE bit|            |
>>>>              | in the VF Control register.    |            |
>>>> -------------+--------------------------------+------------+-----------
>>>>
>>>> Afterwards the vfio-pci will initialize its own virtual PCI config space
>>>> data ('vconfig') by reading the VF's physical PCI config space, then the
>>>> 'Memory Space Enable' bit in vconfig will always be 0b value. This will
>>>> make the vfio-pci treat the BAR memory space as disabled, and the SIGBUS
>>>> will be triggered if access these BARs.
>>>>
>>>> By investigation, the VF PCI device *passthrough* into the Guest OS by
>>>> QEMU has the 'Memory Space Enable' with 1b value. That's because every
>>>> PCI driver will start to enable the memory space, and this action will
>>>> be hooked by vfio-pci virtual PCI read/write to set the 'Memory Space
>>>> Enable' in vconfig space to 1b. So VF runs in guest OS has 'Mem+', but
>>>> VF runs in host OS has 'Mem-'.
>>>>
>>>> Align with PCI working mode in Guest/QEMU/Host, in DPDK, enable the PCI
>>>> bus memory space explicitly to avoid access on disabled memory.
>>>>
>>>> Fixes: 33604c31354a ("vfio: refactor PCI BAR mapping")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>>>> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> Tested-by: Harman Kalra <hkalra@marvell.com>
>>>> Tested-by: David Marchand <david.marchand@redhat.com>
>>> Tested-by: Thierry Martin <thierry.martin.public@gmail.com>
>>>
>>> Applied, thanks again Haiyue.
>>>
>>>
>>> Kevin, Luca,
>>>
>>> I can see that some distros have already started backporting the fix
>>> in kernel (fc31, fc32 and rhel7 at least for what I saw).
>>> 18.11 and 19.11 will need this fix at some point.
>>> I'll let you decide on the proper timing.
>>>
>>>
>>
>> It looks an important fix. I think it's worth having in 18.11.9. I will
>> apply and create an 18.11.9-rc2 tomorrow, so if anyone hasn't started
>> validation already, they can validate with it in.
> 
> Alex post a fix in kernel just now. So looks like the DPDK patch is nice
> to have, not a MUST. ;-)
> 

Thanks for the update Haiyue. That may be true in the future, but not at
the moment. The patch is just submitted yesterday, so I don't know how
long it will take to filter through to all the distro kernels (and users
to update).

I think it's still worth to take this patch now in 18.11. I will wait
until this afternoon in case anyone has reasons not to.

thanks,
Kevin.

> https://lore.kernel.org/kvm/159310421505.27590.16617666489295503039.stgit@gimli.home/T/#u
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4] bus/pci: fix VF bus error for memory access
  2020-06-25 18:33       ` Wang, Haiyue
  2020-06-26  9:10         ` Kevin Traynor
@ 2020-06-26  9:17         ` David Marchand
  2020-06-26 14:14           ` Wang, Haiyue
  1 sibling, 1 reply; 16+ messages in thread
From: David Marchand @ 2020-06-26  9:17 UTC (permalink / raw)
  To: Wang, Haiyue
  Cc: Kevin Traynor, Luca Boccassi, dev, Burakov, Anatoly, dpdk stable,
	Harman Kalra

On Thu, Jun 25, 2020 at 8:34 PM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> > > I can see that some distros have already started backporting the fix
> > > in kernel (fc31, fc32 and rhel7 at least for what I saw).
> > > 18.11 and 19.11 will need this fix at some point.
> > > I'll let you decide on the proper timing.
> > >
> > >
> >
> > It looks an important fix. I think it's worth having in 18.11.9. I will
> > apply and create an 18.11.9-rc2 tomorrow, so if anyone hasn't started
> > validation already, they can validate with it in.
>
> Alex post a fix in kernel just now. So looks like the DPDK patch is nice
> to have, not a MUST. ;-)
>
> https://lore.kernel.org/kvm/159310421505.27590.16617666489295503039.stgit@gimli.home/T/#u

Yes, this has been discussed offlist and Alex proposed an adjustment
on the CVE fix.
But we still have to live with the situation where people will have
only the first part of the fix.
I am still for backporting this change to stable branches.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4] bus/pci: fix VF bus error for memory access
  2020-06-26  9:17         ` David Marchand
@ 2020-06-26 14:14           ` Wang, Haiyue
  0 siblings, 0 replies; 16+ messages in thread
From: Wang, Haiyue @ 2020-06-26 14:14 UTC (permalink / raw)
  To: David Marchand
  Cc: Kevin Traynor, Luca Boccassi, dev, Burakov, Anatoly, dpdk stable,
	Harman Kalra

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, June 26, 2020 17:17
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: Kevin Traynor <ktraynor@redhat.com>; Luca Boccassi <bluca@debian.org>; dev <dev@dpdk.org>; Burakov,
> Anatoly <anatoly.burakov@intel.com>; dpdk stable <stable@dpdk.org>; Harman Kalra <hkalra@marvell.com>
> Subject: Re: [PATCH v4] bus/pci: fix VF bus error for memory access
> 
> On Thu, Jun 25, 2020 at 8:34 PM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> > > > I can see that some distros have already started backporting the fix
> > > > in kernel (fc31, fc32 and rhel7 at least for what I saw).
> > > > 18.11 and 19.11 will need this fix at some point.
> > > > I'll let you decide on the proper timing.
> > > >
> > > >
> > >
> > > It looks an important fix. I think it's worth having in 18.11.9. I will
> > > apply and create an 18.11.9-rc2 tomorrow, so if anyone hasn't started
> > > validation already, they can validate with it in.
> >
> > Alex post a fix in kernel just now. So looks like the DPDK patch is nice
> > to have, not a MUST. ;-)
> >
> > https://lore.kernel.org/kvm/159310421505.27590.16617666489295503039.stgit@gimli.home/T/#u
> 
> Yes, this has been discussed offlist and Alex proposed an adjustment
> on the CVE fix.
> But we still have to live with the situation where people will have
> only the first part of the fix.
> I am still for backporting this change to stable branches.
> 

Got it, thanks for sharing.

> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-06-26 14:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-21 17:40 [dpdk-dev] [PATCH v1] bus/pci: fix VF bus error for memory access Haiyue Wang
2020-06-22  6:30 ` [dpdk-dev] [PATCH v2] " Haiyue Wang
2020-06-22  8:52   ` Burakov, Anatoly
2020-06-22 11:25     ` Wang, Haiyue
2020-06-22 11:13 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
2020-06-22 12:11   ` Burakov, Anatoly
2020-06-23 15:12   ` Harman Kalra
2020-06-24 20:01   ` David Marchand
2020-06-25  4:01     ` Wang, Haiyue
2020-06-25  3:50 ` [dpdk-dev] [PATCH v4] " Haiyue Wang
2020-06-25 14:09   ` David Marchand
2020-06-25 16:45     ` Kevin Traynor
2020-06-25 18:33       ` Wang, Haiyue
2020-06-26  9:10         ` Kevin Traynor
2020-06-26  9:17         ` David Marchand
2020-06-26 14:14           ` Wang, Haiyue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).