[dpdk-dev] [PATCH v2] dev: don't remove devargs that are still referenced

Darek Stojaczyk dariusz.stojaczyk at intel.com
Wed Nov 21 20:38:27 CET 2018


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")
Cc: gaetan.rivet at 6wind.com
Cc: thomas at monjalon.net

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk at intel.com>
---
 lib/librte_eal/common/eal_common_dev.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 1fdc9ab17..b6fc5e437 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -169,11 +169,10 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
 
 	ret = dev->bus->plug(dev);
 	if (ret) {
-		if (rte_dev_is_probed(dev)) /* if already succeeded earlier */
-			return ret; /* no rollback */
-		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
-			dev->name);
-		goto err_devarg;
+		if (!rte_dev_is_probed(dev)) /* if hasn't succeeded earlier */
+			RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
+				dev->name);
+		return ret;
 	}
 
 	*new_dev = dev;
-- 
2.17.1



More information about the dev mailing list