From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by dpdk.org (Postfix) with ESMTP id 7D05E1B50E for ; Fri, 30 Nov 2018 00:16:15 +0100 (CET) Received: from Internal Mail-Server by MTLPINE1 (envelope-from yskoh@mellanox.com) with ESMTPS (AES256-SHA encrypted); 30 Nov 2018 01:22:05 +0200 Received: from scfae-sc-2.mti.labs.mlnx (scfae-sc-2.mti.labs.mlnx [10.101.0.96]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id wATNCW98032075; Fri, 30 Nov 2018 01:16:09 +0200 From: Yongseok Koh To: Darek Stojaczyk Cc: Maxime Coquelin , Thomas Monjalon , dpdk stable Date: Thu, 29 Nov 2018 15:12:02 -0800 Message-Id: <20181129231202.30436-128-yskoh@mellanox.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20181129231202.30436-1-yskoh@mellanox.com> References: <20181129231202.30436-1-yskoh@mellanox.com> Subject: [dpdk-stable] patch 'eal: fix devargs reference after probing failure' has been queued to LTS release 17.11.5 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 23:16:15 -0000 Hi, FYI, your patch has been queued to LTS release 17.11.5 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/01/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. Yongseok --- >>From abdb0f9a8b0fe291ef411fff1a44d0f1d49a40f6 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Fri, 23 Nov 2018 16:43:28 +0100 Subject: [PATCH] eal: fix devargs reference after probing failure [ upstream commit 161419983da296f329039e15b88e11ea31b15702 ] Even if a device failed to plug, it's still a device object that references the devargs. Those devargs will be freed automatically together with the device, but freeing them any earlier - like it's done in the hotplug error handling path right now - will give us a dangling pointer and a segfault scenario. Consider the following case: * secondary process receives the hotplug request IPC message * devargs are either created or updated * the bus is scanned * a new device object is created with the latest devargs * the device can't be plugged for whatever reason, bus->plug returns error * the devargs are freed, even though they're still referenced by the device object on the bus For PCI devices, the generic device name comes from a buffer within the devargs. Freeing those will make EAL segfault whenever the device name is checked. This patch just prevents the hotplug error handling path from removing the devargs when there's a device that references them. This is done by simply exiting early from the hotplug function. As mentioned in the beginning, those devargs will be freed later, together with the device itself. Fixes: 7e8b26650146 ("eal: fix hotplug add / remove") Signed-off-by: Darek Stojaczyk Acked-by: Maxime Coquelin Acked-by: Thomas Monjalon --- lib/librte_eal/common/eal_common_dev.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c index dda8f5835..582cf3a31 100644 --- a/lib/librte_eal/common/eal_common_dev.c +++ b/lib/librte_eal/common/eal_common_dev.c @@ -183,12 +183,16 @@ int rte_eal_hotplug_add(const char *busname, const char *devname, ret = -ENODEV; goto err_devarg; } + /* Since there is a matching device, it is now its responsibility + * to manage the devargs we've just inserted. From this point + * those devargs shouldn't be removed manually anymore. + */ ret = bus->plug(dev); if (ret) { RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", dev->name); - goto err_devarg; + return ret; } free(name); return 0; -- 2.11.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2018-11-29 15:01:50.787743434 -0800 +++ 0128-eal-fix-devargs-reference-after-probing-failure.patch 2018-11-29 15:01:45.337956000 -0800 @@ -1,8 +1,10 @@ -From 161419983da296f329039e15b88e11ea31b15702 Mon Sep 17 00:00:00 2001 +From abdb0f9a8b0fe291ef411fff1a44d0f1d49a40f6 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Fri, 23 Nov 2018 16:43:28 +0100 Subject: [PATCH] eal: fix devargs reference after probing failure +[ upstream commit 161419983da296f329039e15b88e11ea31b15702 ] + Even if a device failed to plug, it's still a device object that references the devargs. Those devargs will be freed automatically together with the device, but @@ -41,10 +43,10 @@ 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c -index a08dc085f..fd7f5ca7d 100644 +index dda8f5835..582cf3a31 100644 --- a/lib/librte_eal/common/eal_common_dev.c +++ b/lib/librte_eal/common/eal_common_dev.c -@@ -166,12 +166,16 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev) +@@ -183,12 +183,16 @@ int rte_eal_hotplug_add(const char *busname, const char *devname, ret = -ENODEV; goto err_devarg; } @@ -53,15 +55,15 @@ + * those devargs shouldn't be removed manually anymore. + */ - ret = dev->bus->plug(dev); - if (ret && !rte_dev_is_probed(dev)) { /* if hasn't ever succeeded */ + ret = bus->plug(dev); + if (ret) { RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", dev->name); - goto err_devarg; + return ret; } - - *new_dev = dev; + free(name); + return 0; -- 2.11.0