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

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162955

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-09

git sha: Not provided in patch format (patchwork submission)
Author: Stephen Hemminger <stephen at networkplumber.org>

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

This patch adds a size parameter to rte_eth_dev_get_name_by_port() to
prevent buffer overflows. The previous implementation used strcpy() into
a caller-provided buffer with no bounds checking. The new implementation
uses strlcpy() and returns -ERANGE if the name is truncated. The copy is
performed under the ethdev shared data lock. The previous ABI is
preserved via symbol versioning (DPDK_26) with a thin wrapper. All
in-tree callers are updated to pass sizeof(name) as the new parameter.

>  lib/ethdev/rte_ethdev.c                      | 42 +++++++++++++++-----

[ ... ]

> @@ -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;
> +	}

Does the code need to validate that size <= SIZE_MAX - 1 to prevent
integer overflow in the strlcpy() call below?

> +
>  	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);
>  	rte_spinlock_unlock(rte_mcfg_ethdev_get_lock());

Does the code follow C99 variable declaration rules? The variable 'n' is
declared in the middle of the function after the spinlock_lock() call.

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

Does this error check occur after the spinlock is released, meaning the
caller receives a truncated name in the buffer when -ERANGE is returned?

> @@ -735,14 +732,18 @@ rte_eth_dev_get_name_by_port(uint16_t port_id, char *name)
>   * @param port_id
>   *   Port identifier of the device.
>   * @param name
> - *   Buffer of size RTE_ETH_NAME_MAX_LEN to store the name.
> + *   Buffer to store the name.
> + * @param size
> + *   Size of the buffer pointed to by @p name. Should be at least
> + *   RTE_ETH_NAME_MAX_LEN bytes.
>   * @return
>   *   - (0) if successful.
>   *   - (-ENODEV) if *port_id* is invalid.
> - *   - (-EINVAL) on failure.
> + *   - (-EINVAL) if *name* is NULL or *size* is 0.
> + *   - (-ERANGE) if the buffer is too small for the device name.

[ ... ]

>  lib/pdump/rte_pdump.c                        |  2 +-

[ ... ]

> @@ -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));

Does this call site pass sizeof(name) where 'name' is a pointer
parameter to pdump_validate_port()? This would result in sizeof(char *)
instead of the actual buffer size.