diff mbox series

[v2] lib/cryptodev: fix driver name comparison

Message ID 1552283715-18723-1-git-send-email-anoobj@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series [v2] lib/cryptodev: fix driver name comparison |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Anoob Joseph March 11, 2019, 5:55 a.m. UTC 
  The string compare to the length of driver name might give false
positives when there are drivers with similar names (one being the
subset of another).

Following is such a naming which could result in false positive.
1. crypto_driver
2. crypto_driver1

When strncmp with len = strlen("crypto_driver") is done, it could give
a false positive when compared against "crypto_driver1". For such cases,
'strlen + 1' is done, so that the NULL termination also would be
considered for the comparison.

Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for crypto devices")

Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
Signed-off-by: Anoob Joseph <anoobj@marvell.com>
---
v2:
* Using strlen + 1, instead of RTE_CRYPTODEV_NAME_MAX_LEN for the comparison.
* Strcmp would not cause this issue. Touching only the places which
  would result in the issue.

 lib/librte_cryptodev/rte_cryptodev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Fiona Trahe March 11, 2019, 10:41 a.m. UTC | #1 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anoob Joseph
> Sent: Monday, March 11, 2019 5:56 AM
> To: Akhil Goyal <akhil.goyal@nxp.com>; Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch,
> Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Narayana Prasad Raju Athreya <pathreya@marvell.com>; Suheil
> Chandran <schandran@marvell.com>; dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] lib/cryptodev: fix driver name comparison
> 
> The string compare to the length of driver name might give false
> positives when there are drivers with similar names (one being the
> subset of another).
> 
> Following is such a naming which could result in false positive.
> 1. crypto_driver
> 2. crypto_driver1
> 
> When strncmp with len = strlen("crypto_driver") is done, it could give
> a false positive when compared against "crypto_driver1". For such cases,
> 'strlen + 1' is done, so that the NULL termination also would be
> considered for the comparison.
> 
> Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for crypto devices")
> 
> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>
Anoob Joseph March 19, 2019, 4:38 a.m. UTC | #2 
Hi Akhil, Declan, Pablo,

Can you review this patch and share your thoughts?

Thanks,
Anoob

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Monday, March 11, 2019 4:11 PM
> To: Anoob Joseph <anoobj@marvell.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Doherty, Declan <declan.doherty@intel.com>; De
> Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Ankur Dwivedi <adwivedi@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; Suheil Chandran <schandran@marvell.com>;
> dev@dpdk.org
> Subject: RE: [PATCH v2] lib/cryptodev: fix driver name comparison
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anoob Joseph
> > Sent: Monday, March 11, 2019 5:56 AM
> > To: Akhil Goyal <akhil.goyal@nxp.com>; Doherty, Declan
> > <declan.doherty@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>
> > Cc: Anoob Joseph <anoobj@marvell.com>; Ankur Dwivedi
> > <adwivedi@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> > <pathreya@marvell.com>; Suheil Chandran <schandran@marvell.com>;
> > dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] lib/cryptodev: fix driver name
> > comparison
> >
> > The string compare to the length of driver name might give false
> > positives when there are drivers with similar names (one being the
> > subset of another).
> >
> > Following is such a naming which could result in false positive.
> > 1. crypto_driver
> > 2. crypto_driver1
> >
> > When strncmp with len = strlen("crypto_driver") is done, it could give
> > a false positive when compared against "crypto_driver1". For such
> > cases, 'strlen + 1' is done, so that the NULL termination also would
> > be considered for the comparison.
> >
> > Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for
> > crypto devices")
> >
> > Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
Akhil Goyal March 19, 2019, 1:42 p.m. UTC | #3 
On 3/11/2019 11:25 AM, Anoob Joseph wrote:
> The string compare to the length of driver name might give false
> positives when there are drivers with similar names (one being the
> subset of another).
>
> Following is such a naming which could result in false positive.
> 1. crypto_driver
> 2. crypto_driver1
>
> When strncmp with len = strlen("crypto_driver") is done, it could give
> a false positive when compared against "crypto_driver1". For such cases,
> 'strlen + 1' is done, so that the NULL termination also would be
> considered for the comparison.
>
> Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for crypto devices")
>
> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> ---
> v2:
> * Using strlen + 1, instead of RTE_CRYPTODEV_NAME_MAX_LEN for the comparison.
> * Strcmp would not cause this issue. Touching only the places which
>    would result in the issue.
>
>   lib/librte_cryptodev/rte_cryptodev.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
> index 7009735..871d7dd 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -586,7 +586,7 @@ rte_cryptodev_devices_get(const char *driver_name, uint8_t *devices,
>   
>   			cmp = strncmp(devs[i].device->driver->name,
>   					driver_name,
> -					strlen(driver_name));
> +					strlen(driver_name) + 1);
>   
>   			if (cmp == 0)
>   				devices[count++] = devs[i].data->dev_id;
> @@ -1691,7 +1691,7 @@ rte_cryptodev_driver_id_get(const char *name)
>   
>   	TAILQ_FOREACH(driver, &cryptodev_driver_list, next) {
>   		driver_name = driver->driver->name;
> -		if (strncmp(driver_name, name, strlen(driver_name)) == 0)
> +		if (strncmp(driver_name, name, strlen(driver_name) + 1) == 0)
>   			return driver->id;
>   	}
>   	return -1;
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
Akhil Goyal March 19, 2019, 2:06 p.m. UTC | #4 
On 3/19/2019 7:12 PM, Akhil Goyal wrote:
>
> On 3/11/2019 11:25 AM, Anoob Joseph wrote:
>> The string compare to the length of driver name might give false
>> positives when there are drivers with similar names (one being the
>> subset of another).
>>
>> Following is such a naming which could result in false positive.
>> 1. crypto_driver
>> 2. crypto_driver1
>>
>> When strncmp with len = strlen("crypto_driver") is done, it could give
>> a false positive when compared against "crypto_driver1". For such cases,
>> 'strlen + 1' is done, so that the NULL termination also would be
>> considered for the comparison.
>>
>> Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for crypto devices")
>>
>> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
>> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
>> ---
>> v2:
>> * Using strlen + 1, instead of RTE_CRYPTODEV_NAME_MAX_LEN for the comparison.
>> * Strcmp would not cause this issue. Touching only the places which
>>     would result in the issue.
>>
>>    lib/librte_cryptodev/rte_cryptodev.c | 4 ++--
>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
>> index 7009735..871d7dd 100644
>> --- a/lib/librte_cryptodev/rte_cryptodev.c
>> +++ b/lib/librte_cryptodev/rte_cryptodev.c
>> @@ -586,7 +586,7 @@ rte_cryptodev_devices_get(const char *driver_name, uint8_t *devices,
>>    
>>    			cmp = strncmp(devs[i].device->driver->name,
>>    					driver_name,
>> -					strlen(driver_name));
>> +					strlen(driver_name) + 1);
>>    
>>    			if (cmp == 0)
>>    				devices[count++] = devs[i].data->dev_id;
>> @@ -1691,7 +1691,7 @@ rte_cryptodev_driver_id_get(const char *name)
>>    
>>    	TAILQ_FOREACH(driver, &cryptodev_driver_list, next) {
>>    		driver_name = driver->driver->name;
>> -		if (strncmp(driver_name, name, strlen(driver_name)) == 0)
>> +		if (strncmp(driver_name, name, strlen(driver_name) + 1) == 0)
>>    			return driver->id;
>>    	}
>>    	return -1;
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
Applied to dpdk-next-crypto

Thanks.
diff mbox series

Patch

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 7009735..871d7dd 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -586,7 +586,7 @@  rte_cryptodev_devices_get(const char *driver_name, uint8_t *devices,
 
 			cmp = strncmp(devs[i].device->driver->name,
 					driver_name,
-					strlen(driver_name));
+					strlen(driver_name) + 1);
 
 			if (cmp == 0)
 				devices[count++] = devs[i].data->dev_id;
@@ -1691,7 +1691,7 @@  rte_cryptodev_driver_id_get(const char *name)
 
 	TAILQ_FOREACH(driver, &cryptodev_driver_list, next) {
 		driver_name = driver->driver->name;
-		if (strncmp(driver_name, name, strlen(driver_name)) == 0)
+		if (strncmp(driver_name, name, strlen(driver_name) + 1) == 0)
 			return driver->id;
 	}
 	return -1;