Re: [dpdk-dev] [PATCH] lib/cryptodev: fix driver name comparison

[Anoob Joseph <anoobj@marvell.com>]

Hi Akhil, Declan, Pablo,

Can you review this patch and share your thoughts?

Thanks,
Anoob

> -----Original Message-----
> From: Anoob Joseph
> Sent: Monday, February 4, 2019 4:56 PM
> To: Akhil Goyal <akhil.goyal@nxp.com>; Declan Doherty
> <declan.doherty@intel.com>; Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; dev@dpdk.org; Ankur Dwivedi
> <adwivedi@marvell.com>
> Subject: [PATCH] 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".
> 
> 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>
> ---
>  lib/librte_cryptodev/rte_cryptodev.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> b/lib/librte_cryptodev/rte_cryptodev.c
> index 7009735..b743c60 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -510,7 +510,8 @@ rte_cryptodev_pmd_get_named_dev(const char *name)
>  		dev = &cryptodev_globals.devs[i];
> 
>  		if ((dev->attached == RTE_CRYPTODEV_ATTACHED) &&
> -				(strcmp(dev->data->name, name) == 0))
> +				(strncmp(dev->data->name, name,
> +					 RTE_CRYPTODEV_NAME_MAX_LEN)
> == 0))
>  			return dev;
>  	}
> 
> @@ -542,8 +543,8 @@ rte_cryptodev_get_dev_id(const char *name)
>  		return -1;
> 
>  	for (i = 0; i < cryptodev_globals.nb_devs; i++)
> -		if ((strcmp(cryptodev_globals.devs[i].data->name, name)
> -				== 0) &&
> +		if ((strncmp(cryptodev_globals.devs[i].data->name, name,
> +				RTE_CRYPTODEV_NAME_MAX_LEN) == 0) &&
>  				(cryptodev_globals.devs[i].attached ==
>  						RTE_CRYPTODEV_ATTACHED))
>  			return i;
> @@ -586,7 +587,7 @@ rte_cryptodev_devices_get(const char *driver_name,
> uint8_t *devices,
> 
>  			cmp = strncmp(devs[i].device->driver->name,
>  					driver_name,
> -					strlen(driver_name));
> +					RTE_CRYPTODEV_NAME_MAX_LEN);
> 
>  			if (cmp == 0)
>  				devices[count++] = devs[i].data->dev_id; @@ -
> 1691,7 +1692,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,
> RTE_CRYPTODEV_NAME_MAX_LEN) == 0)
>  			return driver->id;
>  	}
>  	return -1;
> --
> 2.7.4