xen/pciback: Save xen_pci_op commands before processing it

Double fetch vulnerabilities that happen when a variable is
fetched twice from shared memory but a security check is only
performed the first time.

The xen_pcibk_do_op function performs a switch statements on the op->cmd
value which is stored in shared memory. Interestingly this can result
in a double fetch vulnerability depending on the performed compiler
optimization.

This patch fixes it by saving the xen_pci_op command before
processing it. We also use 'barrier' to make sure that the
compiler does not perform any optimization.

This is part of XSA155.

CC: stable@vger.kernel.org
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h
index 58e38d5..4d529f3 100644 (file)
--- a/drivers/xen/xen-pciback/pciback.h
+++ b/drivers/xen/xen-pciback/pciback.h
@@ -37,6 +37,7 @@ struct xen_pcibk_device {
        struct xen_pci_sharedinfo *sh_info;
        unsigned long flags;
        struct work_struct op_work;
+       struct xen_pci_op op;
 };
 
 struct xen_pcibk_dev_data {

diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index c4a0666..a0e0e3e 100644 (file)
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -298,9 +298,11 @@ void xen_pcibk_do_op(struct work_struct *data)
                container_of(data, struct xen_pcibk_device, op_work);
        struct pci_dev *dev;
        struct xen_pcibk_dev_data *dev_data = NULL;
-       struct xen_pci_op *op = &pdev->sh_info->op;
+       struct xen_pci_op *op = &pdev->op;
        int test_intx = 0;
 
+       *op = pdev->sh_info->op;
+       barrier();
        dev = xen_pcibk_get_pci_dev(pdev, op->domain, op->bus, op->devfn);
 
        if (dev == NULL)
@@ -342,6 +344,17 @@ void xen_pcibk_do_op(struct work_struct *data)
                if ((dev_data->enable_intx != test_intx))
                        xen_pcibk_control_isr(dev, 0 /* no reset */);
        }
+       pdev->sh_info->op.err = op->err;
+       pdev->sh_info->op.value = op->value;
+#ifdef CONFIG_PCI_MSI
+       if (op->cmd == XEN_PCI_OP_enable_msix && op->err == 0) {
+               unsigned int i;
+
+               for (i = 0; i < op->value; i++)
+                       pdev->sh_info->op.msix_entries[i].vector =
+                               op->msix_entries[i].vector;
+       }
+#endif
        /* Tell the driver domain that we're done. */
        wmb();
        clear_bit(_XEN_PCIF_active, (unsigned long *)&pdev->sh_info->flags);