[v4,2/3] net/ice: refactor raw pattern parsing function

Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern.

Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
 drivers/net/ice/ice_hash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
  

On Mon, Jul 22, 2024 at 10:59:49AM +0000, Vladimir Medvedkin wrote:
> Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern.
> 
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
>  drivers/net/ice/ice_hash.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
> index 6b3095e2c5..506ea261e8 100644
> --- a/drivers/net/ice/ice_hash.c
> +++ b/drivers/net/ice/ice_hash.c
> @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad,
>  	raw_spec = item->spec;
>  	raw_mask = item->mask;
>  
> -	spec_len = strlen((char *)(uintptr_t)raw_spec->pattern);
> -	if (strlen((char *)(uintptr_t)raw_mask->pattern) !=
> -		spec_len)
> +	spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length);
> +	if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) !=
> +			spec_len)

Are we missing something by not checking the return values from the length
calls for overflow? If spec_len == raw_spec->length, then we have an
overflow, and if raw_mask similarly overflows the comparison would still
pass and not flag an error.

/Bruce

>  		return -rte_errno;
>  
>  	pkt_len = spec_len / 2;
> -- 
> 2.34.1
>
  

-----Original Message-----
From: Richardson, Bruce <bruce.richardson@intel.com> 
Sent: Monday, July 22, 2024 12:25 PM
To: Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v4 2/3] net/ice: refactor raw pattern parsing function

On Mon, Jul 22, 2024 at 10:59:49AM +0000, Vladimir Medvedkin wrote:
> Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern.
> 
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
>  drivers/net/ice/ice_hash.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c 
> index 6b3095e2c5..506ea261e8 100644
> --- a/drivers/net/ice/ice_hash.c
> +++ b/drivers/net/ice/ice_hash.c
> @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad,
>  	raw_spec = item->spec;
>  	raw_mask = item->mask;
>  
> -	spec_len = strlen((char *)(uintptr_t)raw_spec->pattern);
> -	if (strlen((char *)(uintptr_t)raw_mask->pattern) !=
> -		spec_len)
> +	spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length);
> +	if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) !=
> +			spec_len)

Are we missing something by not checking the return values from the length calls for overflow? If spec_len == raw_spec->length, then we have an overflow, and if raw_mask similarly overflows the comparison would still pass and not flag an error.

Fixed in v5

/Bruce

>  		return -rte_errno;
>  
>  	pkt_len = spec_len / 2;
> --
> 2.34.1
>