From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753238AbdBFOfH (ORCPT ); Mon, 6 Feb 2017 09:35:07 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:39039 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752953AbdBFOfD (ORCPT ); Mon, 6 Feb 2017 09:35:03 -0500 Date: Mon, 6 Feb 2017 09:33:45 -0500 From: Konrad Rzeszutek Wilk To: Willy Tarreau Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, linux@roeck-us.net, Jan Beulich , David Vrabel Subject: Re: [PATCH 3.10 037/319] xen/pciback: Save xen_pci_op commands before processing it Message-ID: <20170206143345.GC19124@char.us.oracle.com> References: <1486322486-8024-1-git-send-email-w@1wt.eu> <1486322486-8024-8-git-send-email-w@1wt.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1486322486-8024-8-git-send-email-w@1wt.eu> User-Agent: Mutt/1.7.1 (2016-10-04) X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Feb 05, 2017 at 08:20:24PM +0100, Willy Tarreau wrote: > From: Konrad Rzeszutek Wilk > > commit 8135cf8b092723dbfcc611fe6fdcb3a36c9951c5 upstream. 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 number of vectors requested was being overwritten by the return value (typically zero for success). > > 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 > Cc: "Jan Beulich" > Signed-off-by: Willy Tarreau > --- > 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 f72af87..560b3ec 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 b98cf0c..32f83f0 100644 > --- a/drivers/xen/xen-pciback/pciback_ops.c > +++ b/drivers/xen/xen-pciback/pciback_ops.c > @@ -295,9 +295,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) > @@ -339,6 +341,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); > -- > 2.8.0.rc2.1.gbe9624a >