Re: [RFC] ethdev: add buffer size parameter to rte_eth_dev_get_name_by_port()

[Andrew Rybchenko]

On 4/9/26 4:36 AM, Stephen Hemminger wrote:

rte_eth_dev_get_name_by_port() uses strcpy() into a caller-provided
buffer with no bounds checking. A mistaken caller that supplies a
buffer smaller than RTE_ETH_NAME_MAX_LEN can overflow the buffer.

Add a size parameter and replace strcpy() with strlcpy(), returning
-ERANGE if the name is truncated. The copy is now performed under
the ethdev shared data lock so that the name cannot be mutated by
another process between reading and copying.

The previous ABI is preserved via symbol versioning (DPDK_26) with a
thin wrapper that delegates to the new implementation with
RTE_ETH_NAME_MAX_LEN. The versioned symbol will be removed in 26.11.

Update all in-tree callers to pass sizeof(name) as the new parameter.

Signed-off-by: Stephen Hemminger <step...@networkplumber.org>

with notes below fixed

Reviewed-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 2edc7a362e..586a56bd46 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -721,12 +721,9 @@ rte_eth_dev_count_total(void)
        return count;
  }

-RTE_EXPORT_SYMBOL(rte_eth_dev_get_name_by_port)

-int
-rte_eth_dev_get_name_by_port(uint16_t port_id, char *name)
+static int
+eth_dev_get_name_by_port(uint16_t port_id, char *name, size_t size)
  {
-       char *tmp;
-
        RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);

if (name == NULL) {

@@ -735,19 +732,46 @@ rte_eth_dev_get_name_by_port(uint16_t port_id, char *name)
                return -EINVAL;
        }

+ if (size == 0) {

+               RTE_ETHDEV_LOG_LINE(ERR,
+                       "Cannot get ethdev port %u name with zero-size buffer", 
port_id);
+               return -EINVAL;
+       }
+
        rte_spinlock_lock(rte_mcfg_ethdev_get_lock());
-       /* shouldn't check 'rte_eth_devices[i].data',
-        * because it might be overwritten by VDEV PMD */
-       tmp = eth_dev_shared_data->data[port_id].name;
+       /*
+        * Use the shared data name rather than rte_eth_devices[].data->name
+        * because VDEV PMDs may overwrite the per-process data pointer.
+        */
+       size_t n = strlcpy(name, eth_dev_shared_data->data[port_id].name, size);

As far as I know typically variable declaration and code are not mixed
in DPDK.

        rte_spinlock_unlock(rte_mcfg_ethdev_get_lock());

- strcpy(name, tmp);

+       if (n >= size) {
+               RTE_ETHDEV_LOG_LINE(ERR,
+                                   "ethdev port %u name exceeds buffer size 
%zu",
+                                   port_id, size);
+               return -ERANGE;
+       }

rte_ethdev_trace_get_name_by_port(port_id, name); return 0;

  }

[...]

diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
index ac94efe7ff..57c81d4322 100644
--- a/lib/pdump/rte_pdump.c
+++ b/lib/pdump/rte_pdump.c
@@ -736,7 +736,7 @@ pdump_validate_port(uint16_t port, char *name)
                return -1;
        }

- ret = rte_eth_dev_get_name_by_port(port, name);

+       ret = rte_eth_dev_get_name_by_port(port, name, sizeof(name));

It is definitely a bug here since name is a pointer.

        if (ret < 0) {
                PDUMP_LOG_LINE(ERR, "port %u to name mapping failed",
                          port);