Re: [dpdk-stable] [PATCH 18.11] cryptodev: fix missing device id range checking

[Kevin Traynor <ktraynor@redhat.com>]

On 24/02/2020 13:08, Adam Dybkowski wrote:
> This patch adds range-checking of the device id passed from
> the user app code. It prevents out-of-range array accesses
> which in some situations resulted in an
> application crash (segfault).
> 
> Fixes: 8db57afd7ab9 ("cryptodev: fix checks related to device id")
> 
> Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>

Thanks for the backport Adam. Will take when the equivalent patch is in
dpdk master and I will add the reference to master commit.

> ---
>  lib/librte_cryptodev/rte_cryptodev.c | 41 +++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
> index f7566fc30..180260a3b 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -509,7 +509,8 @@ rte_cryptodev_pmd_get_named_dev(const char *name)
>  static inline uint8_t
>  rte_cryptodev_is_valid_device_data(uint8_t dev_id)
>  {
> -	if (rte_crypto_devices[dev_id].data == NULL)
> +	if (dev_id >= RTE_CRYPTO_MAX_DEVS ||
> +			rte_crypto_devices[dev_id].data == NULL)
>  		return 0;
>  
>  	return 1;
> @@ -601,8 +602,9 @@ rte_cryptodev_devices_get(const char *driver_name, uint8_t *devices,
>  void *
>  rte_cryptodev_get_sec_ctx(uint8_t dev_id)
>  {
> -	if (rte_crypto_devices[dev_id].feature_flags &
> -			RTE_CRYPTODEV_FF_SECURITY)
> +	if (dev_id < RTE_CRYPTO_MAX_DEVS &&
> +			(rte_crypto_devices[dev_id].feature_flags &
> +			RTE_CRYPTODEV_FF_SECURITY))
>  		return rte_crypto_devices[dev_id].security_ctx;
>  
>  	return NULL;
> @@ -741,6 +743,11 @@ rte_cryptodev_queue_pair_count(uint8_t dev_id)
>  {
>  	struct rte_cryptodev *dev;
>  
> +	if (!rte_cryptodev_is_valid_device_data(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return 0;
> +	}
> +
>  	dev = &rte_crypto_devices[dev_id];
>  	return dev->data->nb_queue_pairs;
>  }
> @@ -1169,6 +1176,11 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>  	uint8_t index;
>  	int ret;
>  
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return -EINVAL;
> +	}
> +
>  	dev = rte_cryptodev_pmd_get_dev(dev_id);
>  
>  	if (sess == NULL || xforms == NULL || dev == NULL)
> @@ -1202,6 +1214,11 @@ rte_cryptodev_asym_session_init(uint8_t dev_id,
>  	uint8_t index;
>  	int ret;
>  
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return -EINVAL;
> +	}
> +
>  	dev = rte_cryptodev_pmd_get_dev(dev_id);
>  
>  	if (sess == NULL || xforms == NULL || dev == NULL)
> @@ -1271,6 +1288,11 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
>  {
>  	struct rte_cryptodev *dev;
>  
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return -EINVAL;
> +	}
> +
>  	dev = rte_cryptodev_pmd_get_dev(dev_id);
>  
>  	if (dev == NULL || sess == NULL)
> @@ -1289,6 +1311,11 @@ rte_cryptodev_asym_session_clear(uint8_t dev_id,
>  {
>  	struct rte_cryptodev *dev;
>  
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return -EINVAL;
> +	}
> +
>  	dev = rte_cryptodev_pmd_get_dev(dev_id);
>  
>  	if (dev == NULL || sess == NULL)
> @@ -1594,8 +1621,14 @@ rte_cryptodev_driver_id_get(const char *name)
>  const char *
>  rte_cryptodev_name_get(uint8_t dev_id)
>  {
> -	struct rte_cryptodev *dev = rte_cryptodev_pmd_get_dev(dev_id);
> +	struct rte_cryptodev *dev;
>  
> +	if (!rte_cryptodev_is_valid_device_data(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return NULL;
> +	}
> +
> +	dev = rte_cryptodev_pmd_get_dev(dev_id);
>  	if (dev == NULL)
>  		return NULL;
>  
>