[xen] pciback: Fix state validation in MSI control operations (CVE-2015-8551, CVE-2015-8852, XSA-157)

This commit is contained in:
Ben Hutchings 2015-12-27 05:54:06 +00:00
parent 94b974ce0f
commit 7b6f99cec5
7 changed files with 365 additions and 0 deletions

2
debian/changelog vendored
View File

@ -6,6 +6,8 @@ linux (4.3.3-3) UNRELEASED; urgency=medium
* pptp: verify sockaddr_len in pptp_bind() and pptp_connect() (CVE-2015-8569)
* bluetooth: Validate socket address length in sco_sock_bind() (CVE-2015-8575)
* [xen] Fix race conditions in back-end drivers (CVE-2015-8550, XSA-155)
* [xen] pciback: Fix state validation in MSI control operations
(CVE-2015-8551, CVE-2015-8852, XSA-157)
[ Salvatore Bonaccorso ]
* ovl: fix permission checking for setattr (CVE-2015-8660)

View File

@ -0,0 +1,77 @@
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 2 Nov 2015 17:24:08 -0500
Subject: [3/5] xen/pciback: Do not install an IRQ handler for MSI interrupts.
Origin: https://git.kernel.org/linus/a396f3a210c3a61e94d6b87ec05a75d0be2a60d0
Otherwise an guest can subvert the generic MSI code to trigger
an BUG_ON condition during MSI interrupt freeing:
for (i = 0; i < entry->nvec_used; i++)
BUG_ON(irq_has_action(entry->irq + i));
Xen PCI backed installs an IRQ handler (request_irq) for
the dev->irq whenever the guest writes PCI_COMMAND_MEMORY
(or PCI_COMMAND_IO) to the PCI_COMMAND register. This is
done in case the device has legacy interrupts the GSI line
is shared by the backend devices.
To subvert the backend the guest needs to make the backend
to change the dev->irq from the GSI to the MSI interrupt line,
make the backend allocate an interrupt handler, and then command
the backend to free the MSI interrupt and hit the BUG_ON.
Since the backend only calls 'request_irq' when the guest
writes to the PCI_COMMAND register the guest needs to call
XEN_PCI_OP_enable_msi before any other operation. This will
cause the generic MSI code to setup an MSI entry and
populate dev->irq with the new PIRQ value.
Then the guest can write to PCI_COMMAND PCI_COMMAND_MEMORY
and cause the backend to setup an IRQ handler for dev->irq
(which instead of the GSI value has the MSI pirq). See
'xen_pcibk_control_isr'.
Then the guest disables the MSI: XEN_PCI_OP_disable_msi
which ends up triggering the BUG_ON condition in 'free_msi_irqs'
as there is an IRQ handler for the entry->irq (dev->irq).
Note that this cannot be done using MSI-X as the generic
code does not over-write dev->irq with the MSI-X PIRQ values.
The patch inhibits setting up the IRQ handler if MSI or
MSI-X (for symmetry reasons) code had been called successfully.
P.S.
Xen PCIBack when it sets up the device for the guest consumption
ends up writting 0 to the PCI_COMMAND (see xen_pcibk_reset_device).
XSA-120 addendum patch removed that - however when upstreaming said
addendum we found that it caused issues with qemu upstream. That
has now been fixed in qemu upstream.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 029f33d..d0696ce 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -70,6 +70,13 @@ static void xen_pcibk_control_isr(struct pci_dev *dev, int reset)
enable ? "enable" : "disable");
if (enable) {
+ /*
+ * The MSI or MSI-X should not have an IRQ handler. Otherwise
+ * if the guest terminates we BUG_ON in free_msi_irqs.
+ */
+ if (dev->msi_enabled || dev->msix_enabled)
+ goto out;
+
rc = request_irq(dev_data->irq,
xen_pcibk_guest_interrupt, IRQF_SHARED,
dev_data->irq_name, dev);

View File

@ -0,0 +1,61 @@
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 2 Nov 2015 18:13:27 -0500
Subject: [5/5] xen/pciback: Don't allow MSI-X ops if PCI_COMMAND_MEMORY is not
set.
Origin: https://git.kernel.org/linus/408fb0e5aa7fda0059db282ff58c3b2a4278baa0
commit f598282f51 ("PCI: Fix the NIU MSI-X problem in a better way")
teaches us that dealing with MSI-X can be troublesome.
Further checks in the MSI-X architecture shows that if the
PCI_COMMAND_MEMORY bit is turned of in the PCI_COMMAND we
may not be able to access the BAR (since they are memory regions).
Since the MSI-X tables are located in there.. that can lead
to us causing PCIe errors. Inhibit us performing any
operation on the MSI-X unless the MEMORY bit is set.
Note that Xen hypervisor with:
"x86/MSI-X: access MSI-X table only after having enabled MSI-X"
will return:
xen_pciback: 0000:0a:00.1: error -6 enabling MSI-X for guest 3!
When the generic MSI code tries to setup the PIRQ without
MEMORY bit set. Which means with later versions of Xen
(4.6) this patch is not neccessary.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 4ee5fc0..73dafdc 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -212,6 +212,7 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
struct xen_pcibk_dev_data *dev_data;
int i, result;
struct msix_entry *entries;
+ u16 cmd;
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n",
@@ -223,7 +224,12 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
if (dev->msix_enabled)
return -EALREADY;
- if (dev->msi_enabled)
+ /*
+ * PCI_COMMAND_MEMORY must be enabled, otherwise we may not be able
+ * to access the BARs where the MSI-X entries reside.
+ */
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ if (dev->msi_enabled || !(cmd & PCI_COMMAND_MEMORY))
return -ENXIO;
entries = kmalloc(op->value * sizeof(*entries), GFP_KERNEL);

View File

@ -0,0 +1,102 @@
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 1 Apr 2015 10:49:47 -0400
Subject: [4/5] xen/pciback: For XEN_PCI_OP_disable_msi[|x] only disable if
device has MSI(X) enabled.
Origin: https://git.kernel.org/linus/7cfb905b9638982862f0331b36ccaaca5d383b49
Otherwise just continue on, returning the same values as
previously (return of 0, and op->result has the PIRQ value).
This does not change the behavior of XEN_PCI_OP_disable_msi[|x].
The pci_disable_msi or pci_disable_msix have the checks for
msi_enabled or msix_enabled so they will error out immediately.
However the guest can still call these operations and cause
us to disable the 'ack_intr'. That means the backend IRQ handler
for the legacy interrupt will not respond to interrupts anymore.
This will lead to (if the device is causing an interrupt storm)
for the Linux generic code to disable the interrupt line.
Naturally this will only happen if the device in question
is plugged in on the motherboard on shared level interrupt GSI.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index d0696ce..4ee5fc0 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -185,20 +185,23 @@ static
int xen_pcibk_disable_msi(struct xen_pcibk_device *pdev,
struct pci_dev *dev, struct xen_pci_op *op)
{
- struct xen_pcibk_dev_data *dev_data;
-
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: disable MSI\n",
pci_name(dev));
- pci_disable_msi(dev);
+ if (dev->msi_enabled) {
+ struct xen_pcibk_dev_data *dev_data;
+
+ pci_disable_msi(dev);
+
+ dev_data = pci_get_drvdata(dev);
+ if (dev_data)
+ dev_data->ack_intr = 1;
+ }
op->value = dev->irq ? xen_pirq_from_irq(dev->irq) : 0;
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: MSI: %d\n", pci_name(dev),
op->value);
- dev_data = pci_get_drvdata(dev);
- if (dev_data)
- dev_data->ack_intr = 1;
return 0;
}
@@ -264,23 +267,27 @@ static
int xen_pcibk_disable_msix(struct xen_pcibk_device *pdev,
struct pci_dev *dev, struct xen_pci_op *op)
{
- struct xen_pcibk_dev_data *dev_data;
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: disable MSI-X\n",
pci_name(dev));
- pci_disable_msix(dev);
+ if (dev->msix_enabled) {
+ struct xen_pcibk_dev_data *dev_data;
+
+ pci_disable_msix(dev);
+
+ dev_data = pci_get_drvdata(dev);
+ if (dev_data)
+ dev_data->ack_intr = 1;
+ }
/*
* SR-IOV devices (which don't have any legacy IRQ) have
* an undefined IRQ value of zero.
*/
op->value = dev->irq ? xen_pirq_from_irq(dev->irq) : 0;
if (unlikely(verbose_request))
- printk(KERN_DEBUG DRV_NAME ": %s: MSI-X: %d\n", pci_name(dev),
- op->value);
- dev_data = pci_get_drvdata(dev);
- if (dev_data)
- dev_data->ack_intr = 1;
+ printk(KERN_DEBUG DRV_NAME ": %s: MSI-X: %d\n",
+ pci_name(dev), op->value);
return 0;
}
#endif

View File

@ -0,0 +1,58 @@
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 3 Apr 2015 11:08:22 -0400
Subject: [1/5] xen/pciback: Return error on XEN_PCI_OP_enable_msi when device
has MSI or MSI-X enabled
Origin: https://git.kernel.org/linus/56441f3c8e5bd45aab10dd9f8c505dd4bec03b0d
The guest sequence of:
a) XEN_PCI_OP_enable_msi
b) XEN_PCI_OP_enable_msi
c) XEN_PCI_OP_disable_msi
results in hitting an BUG_ON condition in the msi.c code.
The MSI code uses an dev->msi_list to which it adds MSI entries.
Under the above conditions an BUG_ON() can be hit. The device
passed in the guest MUST have MSI capability.
The a) adds the entry to the dev->msi_list and sets msi_enabled.
The b) adds a second entry but adding in to SysFS fails (duplicate entry)
and deletes all of the entries from msi_list and returns (with msi_enabled
is still set). c) pci_disable_msi passes the msi_enabled checks and hits:
BUG_ON(list_empty(dev_to_msi_list(&dev->dev)));
and blows up.
The patch adds a simple check in the XEN_PCI_OP_enable_msi to guard
against that. The check for msix_enabled is not stricly neccessary.
This is part of XSA-157.
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index a0e0e3e..8bfb87c 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -144,7 +144,12 @@ int xen_pcibk_enable_msi(struct xen_pcibk_device *pdev,
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI\n", pci_name(dev));
- status = pci_enable_msi(dev);
+ if (dev->msi_enabled)
+ status = -EALREADY;
+ else if (dev->msix_enabled)
+ status = -ENXIO;
+ else
+ status = pci_enable_msi(dev);
if (status) {
pr_warn_ratelimited("%s: error enabling MSI for guest %u: err %d\n",

View File

@ -0,0 +1,60 @@
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 2 Nov 2015 18:07:44 -0500
Subject: [2/5] xen/pciback: Return error on XEN_PCI_OP_enable_msix when device
has MSI or MSI-X enabled
Origin: https://git.kernel.org/linus/5e0ce1455c09dd61d029b8ad45d82e1ac0b6c4c9
The guest sequence of:
a) XEN_PCI_OP_enable_msix
b) XEN_PCI_OP_enable_msix
results in hitting an NULL pointer due to using freed pointers.
The device passed in the guest MUST have MSI-X capability.
The a) constructs and SysFS representation of MSI and MSI groups.
The b) adds a second set of them but adding in to SysFS fails (duplicate entry).
'populate_msi_sysfs' frees the newly allocated msi_irq_groups (note that
in a) pdev->msi_irq_groups is still set) and also free's ALL of the
MSI-X entries of the device (the ones allocated in step a) and b)).
The unwind code: 'free_msi_irqs' deletes all the entries and tries to
delete the pdev->msi_irq_groups (which hasn't been set to NULL).
However the pointers in the SysFS are already freed and we hit an
NULL pointer further on when 'strlen' is attempted on a freed pointer.
The patch adds a simple check in the XEN_PCI_OP_enable_msix to guard
against that. The check for msi_enabled is not stricly neccessary.
This is part of XSA-157
CC: stable@vger.kernel.org
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pciback_ops.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 8bfb87c..029f33d 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -206,9 +206,16 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n",
pci_name(dev));
+
if (op->value > SH_INFO_MAX_VEC)
return -EINVAL;
+ if (dev->msix_enabled)
+ return -EALREADY;
+
+ if (dev->msi_enabled)
+ return -ENXIO;
+
entries = kmalloc(op->value * sizeof(*entries), GFP_KERNEL);
if (entries == NULL)
return -ENOMEM;

View File

@ -116,3 +116,8 @@ bugfix/all/xen-blkback-only-read-request-operation-from-shared-.patch
bugfix/all/xen-blkback-read-from-indirect-descriptors-only-once.patch
bugfix/all/xen-scsiback-safely-copy-requests.patch
bugfix/all/xen-pciback-save-xen_pci_op-commands-before-processi.patch
bugfix/all/xen-pciback-return-error-on-xen_pci_op_enable_msi-wh.patch
bugfix/all/xen-pciback-return-error-on-xen_pci_op_enable_msix-w.patch
bugfix/all/xen-pciback-do-not-install-an-irq-handler-for-msi-in.patch
bugfix/all/xen-pciback-for-xen_pci_op_disable_msi-x-only-disabl.patch
bugfix/all/xen-pciback-don-t-allow-msi-x-ops-if-pci_command_mem.patch