[dpdk-stable] [PATCH v2] ethdev: fix invalid length write on dev detach

[dpdk-stable] [PATCH v2] ethdev: fix invalid length write on dev detach

[Gaetan Rivet]

The name of a device is copied in a provided buffer within
rte_eth_dev_detach(). The current sizeof is done on a pointer instead of
the intended array usually pointed to.

The name field of an rte_device is not assured however to point an
rte_devargs name field. The almost correct length to base this copy over
is thus RTE_DEV_NAME_MAX_LEN.

Almost correct, because unfortunately this function does not allow the
user to pass down a size parameter for the buffer it is meant to write.
This API should be fixed, it is broken by design.

Fixes: a1e7c17555e8 ("ethdev: use device name from device structure")
Cc: stable@dpdk.org
Cc: Ferruh Yigit <ferruh.yigit@intel.com>

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---

v2:

  - Document the function expectations regarding the given buffer.
    While doing so, clarify the intent of the devname parameter, as
    its intent was ambiguous.

 lib/librte_ether/rte_ethdev.c | 4 ++--
 lib/librte_ether/rte_ethdev.h | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 805ef63..0597641 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -436,8 +436,8 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 	if (rte_eth_dev_is_detachable(port_id))
 		goto err;
 
-	snprintf(name, sizeof(rte_eth_devices[port_id].device->name),
-		 "%s", rte_eth_devices[port_id].device->name);
+	snprintf(name, RTE_DEV_NAME_MAX_LEN, "%s",
+		 rte_eth_devices[port_id].device->name);
 
 	ret = rte_eal_dev_detach(rte_eth_devices[port_id].device);
 	if (ret < 0)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 0e99090..0adf327 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1831,7 +1831,8 @@ int rte_eth_dev_attach(const char *devargs, uint8_t *port_id);
  * @param port_id
  *   The port identifier of the device to detach.
  * @param devname
- *  A pointer to a device name actually detached.
+ *   A pointer to a buffer that will be filled with the device name.
+ *   This buffer must be at least RTE_DEV_NAME_MAX_LEN long.
  * @return
  *  0 on success and devname is filled, negative on error
  */
-- 
2.1.4

Re: [dpdk-stable] [PATCH v2] ethdev: fix invalid length write on dev detach

[Thomas Monjalon]

31/07/2017 15:40, Gaetan Rivet:
> The name of a device is copied in a provided buffer within
> rte_eth_dev_detach(). The current sizeof is done on a pointer instead of
> the intended array usually pointed to.
> 
> The name field of an rte_device is not assured however to point an
> rte_devargs name field. The almost correct length to base this copy over
> is thus RTE_DEV_NAME_MAX_LEN.
> 
> Almost correct, because unfortunately this function does not allow the
> user to pass down a size parameter for the buffer it is meant to write.
> This API should be fixed, it is broken by design.
> 
> Fixes: a1e7c17555e8 ("ethdev: use device name from device structure")
> Cc: stable@dpdk.org
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Applied, thanks