[dpdk-dev] ether: fix invalid string length in ethdev name comparison

Message ID 1519687296-2872-1-git-send-email-mohammad.abdul.awal@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Mohammad Abdul Awal Feb. 26, 2018, 11:21 p.m. UTC
  The current code compares two strings upto the length of 1st string
(searched name). If the 1st string is prefix of 2nd string (existing name),
the string comparison returns the port_id of earliest prefix matches.
This patch fixes the bug by comparing the strings upto the length of larger
string.

Fixes: 9c5b8d8b9fe ("ethdev: clean port id retrieval when attaching")

Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
---
 lib/librte_ether/rte_ethdev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
  

Comments

Ananyev, Konstantin Feb. 27, 2018, 12:15 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mohammad Abdul Awal
> Sent: Monday, February 26, 2018 11:22 PM
> To: thomas@monjalon.net
> Cc: rkerur@gmail.com; dev@dpdk.org; Awal, Mohammad Abdul <mohammad.abdul.awal@intel.com>
> Subject: [dpdk-dev] [PATCH] ether: fix invalid string length in ethdev name comparison
> 
> The current code compares two strings upto the length of 1st string
> (searched name). If the 1st string is prefix of 2nd string (existing name),
> the string comparison returns the port_id of earliest prefix matches.
> This patch fixes the bug by comparing the strings upto the length of larger
> string.
> 
> Fixes: 9c5b8d8b9fe ("ethdev: clean port id retrieval when attaching")
> 
> Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 0590f0c..8e8097b 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -563,17 +563,20 @@ rte_eth_dev_get_name_by_port(uint16_t port_id, char *name)
>  int
>  rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id)
>  {
> -	uint32_t pid;
> +	uint32_t pid, len, len1, len2;
> 
>  	if (name == NULL) {
>  		RTE_PMD_DEBUG_TRACE("Null pointer is specified\n");
>  		return -EINVAL;
>  	}
> 
> +	len1 = strlen(name);
>  	for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> +		len2 = strlen(rte_eth_dev_shared_data->data[pid].name);
> +		len = len1 > len2 ? len1 : len2;
>  		if (rte_eth_devices[pid].state != RTE_ETH_DEV_UNUSED &&
>  		    !strncmp(name, rte_eth_dev_shared_data->data[pid].name,
> -			     strlen(name))) {
> +			     len)) {

Why just not simply use strcmp()? :)

>  			*port_id = pid;
>  			return 0;
>  		}
> --
> 2.7.4
  
Mohammad Abdul Awal Feb. 27, 2018, 8:51 a.m. UTC | #2
On 27/02/2018 00:15, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mohammad Abdul Awal
>>
>> +	len1 = strlen(name);
>>   	for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
>> +		len2 = strlen(rte_eth_dev_shared_data->data[pid].name);
>> +		len = len1 > len2 ? len1 : len2;
>>   		if (rte_eth_devices[pid].state != RTE_ETH_DEV_UNUSED &&
>>   		    !strncmp(name, rte_eth_dev_shared_data->data[pid].name,
>> -			     strlen(name))) {
>> +			     len)) {
> Why just not simply use strcmp()? :)
That is the best I would say. I will submit a V2.

>
>>   			*port_id = pid;
>>   			return 0;
>>   		}
>> --
>> 2.7.4
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 0590f0c..8e8097b 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -563,17 +563,20 @@  rte_eth_dev_get_name_by_port(uint16_t port_id, char *name)
 int
 rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id)
 {
-	uint32_t pid;
+	uint32_t pid, len, len1, len2;
 
 	if (name == NULL) {
 		RTE_PMD_DEBUG_TRACE("Null pointer is specified\n");
 		return -EINVAL;
 	}
 
+	len1 = strlen(name);
 	for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
+		len2 = strlen(rte_eth_dev_shared_data->data[pid].name);
+		len = len1 > len2 ? len1 : len2;
 		if (rte_eth_devices[pid].state != RTE_ETH_DEV_UNUSED &&
 		    !strncmp(name, rte_eth_dev_shared_data->data[pid].name,
-			     strlen(name))) {
+			     len)) {
 			*port_id = pid;
 			return 0;
 		}