From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id BD3101B4AA for ; Thu, 29 Nov 2018 14:24:33 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2AC8330842AB; Thu, 29 Nov 2018 13:24:33 +0000 (UTC) Received: from ktraynor.remote.csb (ovpn-117-230.ams2.redhat.com [10.36.117.230]) by smtp.corp.redhat.com (Postfix) with ESMTP id 47F3F1019626; Thu, 29 Nov 2018 13:24:30 +0000 (UTC) From: Kevin Traynor To: Darek Stojaczyk Cc: Anatoly Burakov , Maxime Coquelin , dpdk stable Date: Thu, 29 Nov 2018 13:21:24 +0000 Message-Id: <20181129132128.7609-84-ktraynor@redhat.com> In-Reply-To: <20181129132128.7609-1-ktraynor@redhat.com> References: <20181129132128.7609-1-ktraynor@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Thu, 29 Nov 2018 13:24:33 +0000 (UTC) Subject: [dpdk-stable] patch 'vfio: do not needlessly setup device in secondary process' has been queued to stable release 18.08.1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Nov 2018 13:24:34 -0000 Hi, FYI, your patch has been queued to stable release 18.08.1 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 12/08/18. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. If the code is different (ie: not only metadata diffs), due for example to a change in context or macro names, please double check it. Thanks. Kevin Traynor --- >>From 2de3da91ce9120561965583a962fd1fad3793916 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Wed, 21 Nov 2018 19:41:32 +0100 Subject: [PATCH] vfio: do not needlessly setup device in secondary process [ upstream commit 047e3f9f2a4a4b73da86b707af8a32039ba1cad7 ] Setting up a device that wasn't setup in the primary process will possibly break the primary process. That's because the IPC message to retrieve the group fd in the primary will also *open* that group if it wasn't opened before. Even though the secondary process closes that fd soon after as a part of its error handling path, the primary process leaks it. What's worse, opening that fd on the primary will increment the process-local counter of opened groups. If it was 0 before, then the group will never be added to the vfio container, nor dpdk memory will be ever mapped. This patch moves the proper error checks earlier in the code to fully prevent setting up devices in secondary processes that weren't setup in the primary process. Fixes: 2f4adfad0a69 ("vfio: add multiprocess support") Signed-off-by: Darek Stojaczyk Acked-by: Anatoly Burakov Reviewed-by: Maxime Coquelin --- drivers/bus/pci/linux/pci_vfio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c index 686386d6a..569953666 100644 --- a/drivers/bus/pci/linux/pci_vfio.c +++ b/drivers/bus/pci/linux/pci_vfio.c @@ -552,9 +552,4 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev) loc->domain, loc->bus, loc->devid, loc->function); - ret = rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr, - &vfio_dev_fd, &device_info); - if (ret) - return ret; - /* if we're in a secondary process, just find our tailq entry */ TAILQ_FOREACH(vfio_res, vfio_res_list, next) { @@ -568,7 +563,12 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev) RTE_LOG(ERR, EAL, " %s cannot find TAILQ entry for PCI device!\n", pci_addr); - goto err_vfio_dev_fd; + return -1; } + ret = rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr, + &vfio_dev_fd, &device_info); + if (ret) + return ret; + /* map BARs */ maps = vfio_res->maps; -- 2.19.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2018-11-29 13:11:37.108612284 +0000 +++ 0083-vfio-do-not-needlessly-setup-device-in-secondary-pro.patch 2018-11-29 13:11:35.000000000 +0000 @@ -1,8 +1,10 @@ -From 047e3f9f2a4a4b73da86b707af8a32039ba1cad7 Mon Sep 17 00:00:00 2001 +From 2de3da91ce9120561965583a962fd1fad3793916 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Wed, 21 Nov 2018 19:41:32 +0100 Subject: [PATCH] vfio: do not needlessly setup device in secondary process +[ upstream commit 047e3f9f2a4a4b73da86b707af8a32039ba1cad7 ] + Setting up a device that wasn't setup in the primary process will possibly break the primary process. That's because the IPC message to retrieve the group fd in the @@ -31,10 +33,10 @@ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c -index ffd26f195..54a4c959e 100644 +index 686386d6a..569953666 100644 --- a/drivers/bus/pci/linux/pci_vfio.c +++ b/drivers/bus/pci/linux/pci_vfio.c -@@ -795,9 +795,4 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev) +@@ -552,9 +552,4 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev) loc->domain, loc->bus, loc->devid, loc->function); - ret = rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr, @@ -44,7 +46,7 @@ - /* if we're in a secondary process, just find our tailq entry */ TAILQ_FOREACH(vfio_res, vfio_res_list, next) { -@@ -811,7 +806,12 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev) +@@ -568,7 +563,12 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev) RTE_LOG(ERR, EAL, " %s cannot find TAILQ entry for PCI device!\n", pci_addr); - goto err_vfio_dev_fd;