Re: [PATCH 3.4 096/125] xen/pciback: Save xen_pci_op commands before processing it
On 2016/10/12 20:59, Konrad Rzeszutek Wilk wrote: > On Wed, Oct 12, 2016 at 08:33:32PM +0800, l...@kernel.org wrote: >> From: Konrad Rzeszutek Wilk >> >> 3.4.113-rc1 review patch. If anyone has any objections, please let me know. > > You also need: > > > commit d159457b84395927b5a52adb72f748dd089ad5e5 > Author: Konrad Rzeszutek Wilk > Date: Thu Feb 11 16:10:24 2016 -0500 > > xen/pciback: Save the number of MSI-X entries to be copied later. > > Commit 8135cf8b092723dbfcc611fe6fdcb3a36c9951c5 (xen/pciback: Save > xen_pci_op commands before processing it) broke enabling MSI-X because > it would never copy the resulting vectors into the response. The > I'll queue this up. Thanks!
Re: [PATCH 3.4 096/125] xen/pciback: Save xen_pci_op commands before processing it
On Wed, Oct 12, 2016 at 08:33:32PM +0800, l...@kernel.org wrote:
> From: Konrad Rzeszutek Wilk
>
> 3.4.113-rc1 review patch. If anyone has any objections, please let me know.
You also need:
commit d159457b84395927b5a52adb72f748dd089ad5e5
Author: Konrad Rzeszutek Wilk
Date: Thu Feb 11 16:10:24 2016 -0500
xen/pciback: Save the number of MSI-X entries to be copied later.
Commit 8135cf8b092723dbfcc611fe6fdcb3a36c9951c5 (xen/pciback: Save
xen_pci_op commands before processing it) broke enabling MSI-X because
it would never copy the resulting vectors into the response. The
Thanks.
>
> --
>
>
> commit 8135cf8b092723dbfcc611fe6fdcb3a36c9951c5 upstream.
>
> 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.
>
> Reviewed-by: Konrad Rzeszutek Wilk
> Signed-off-by: Jan Beulich
> Signed-off-by: David Vrabel
> Signed-off-by: Konrad Rzeszutek Wilk
> Signed-off-by: Zefan Li
> ---
> drivers/xen/xen-pciback/pciback.h | 1 +
> drivers/xen/xen-pciback/pciback_ops.c | 15 ++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/xen-pciback/pciback.h
> b/drivers/xen/xen-pciback/pciback.h
> index a7def01..7a642e3 100644
> --- 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 d52703c..a751a66 100644
> --- a/drivers/xen/xen-pciback/pciback_ops.c
> +++ b/drivers/xen/xen-pciback/pciback_ops.c
> @@ -297,9 +297,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)
> @@ -341,6 +343,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);
> --
> 1.9.1
>
[PATCH 3.4 096/125] xen/pciback: Save xen_pci_op commands before processing it
From: Konrad Rzeszutek Wilk
3.4.113-rc1 review patch. If anyone has any objections, please let me know.
--
commit 8135cf8b092723dbfcc611fe6fdcb3a36c9951c5 upstream.
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.
Reviewed-by: Konrad Rzeszutek Wilk
Signed-off-by: Jan Beulich
Signed-off-by: David Vrabel
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Zefan Li
---
drivers/xen/xen-pciback/pciback.h | 1 +
drivers/xen/xen-pciback/pciback_ops.c | 15 ++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/xen-pciback/pciback.h
b/drivers/xen/xen-pciback/pciback.h
index a7def01..7a642e3 100644
--- 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 d52703c..a751a66 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -297,9 +297,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)
@@ -341,6 +343,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);
--
1.9.1
