[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 <stephen at networkplumber.org>

with notes below fixed

Reviewed-by: Andrew Rybchenko <andrew.rybchenko at 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);