From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BCC70A0350; Mon, 22 Jun 2020 10:52:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 35A781D53C; Mon, 22 Jun 2020 10:52:43 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 905261D536; Mon, 22 Jun 2020 10:52:41 +0200 (CEST) IronPort-SDR: TLRgbmqp+L5r1yn4vn4naChaFV6ALuvOVmK+ooVNgDeXMQQLJ6e/Uo5pHkgIreJW52ERqOtPDv 5wzchY+MSpXw== X-IronPort-AV: E=McAfee;i="6000,8403,9659"; a="205180829" X-IronPort-AV: E=Sophos;i="5.75,266,1589266800"; d="scan'208";a="205180829" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Jun 2020 01:52:39 -0700 IronPort-SDR: DEMtGskpzgx8eSigbtrujn9QMDj6GxFEmv6EfYmNVixCOKBdyu+SXm70koXLgZCNvDBkmjagjf rPk5eg3Db2Cg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,266,1589266800"; d="scan'208";a="279749848" Received: from esterlrx-mobl.ger.corp.intel.com (HELO [10.252.33.112]) ([10.252.33.112]) by orsmga007.jf.intel.com with ESMTP; 22 Jun 2020 01:52:38 -0700 To: Haiyue Wang , dev@dpdk.org Cc: stable@dpdk.org References: <20200621174035.6858-1-haiyue.wang@intel.com> <20200622063029.95985-1-haiyue.wang@intel.com> From: "Burakov, Anatoly" Message-ID: <7ea880c0-68d5-11f0-0499-73e3b87660b9@intel.com> Date: Mon, 22 Jun 2020 09:52:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <20200622063029.95985-1-haiyue.wang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] bus/pci: fix VF bus error for memory access X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 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