diff mbox

[dpdk-dev] i40e: fix the issue reported by klocwork

Message ID 1423740143-29708-1-git-send-email-jingjing.wu@intel.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Wu, Jingjing Feb. 12, 2015, 11:22 a.m. UTC
Klocwork reports array 'src_offset' may use index 16.
In function i40e_srcoff_to_flx_pit, index j + 1 can reach I40E_FDIR_MAX_FLEX_LEN.
This patch fixes this issue to avoid array bound.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 lib/librte_pmd_i40e/i40e_fdir.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Comments

Thomas Monjalon March 30, 2015, 8:14 p.m. UTC | #1
Helin, is this patch valid and important?

2015-02-12 19:22, Jingjing Wu:
> Klocwork reports array 'src_offset' may use index 16.
> In function i40e_srcoff_to_flx_pit, index j + 1 can reach I40E_FDIR_MAX_FLEX_LEN.
> This patch fixes this issue to avoid array bound.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  lib/librte_pmd_i40e/i40e_fdir.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/librte_pmd_i40e/i40e_fdir.c b/lib/librte_pmd_i40e/i40e_fdir.c
> index 68511c8..bc36d8e 100644
> --- a/lib/librte_pmd_i40e/i40e_fdir.c
> +++ b/lib/librte_pmd_i40e/i40e_fdir.c
> @@ -402,28 +402,27 @@ i40e_srcoff_to_flx_pit(const uint16_t *src_offset,
>  
>  	while (j < I40E_FDIR_MAX_FLEX_LEN) {
>  		size = 1;
> -		for (; j < I40E_FDIR_MAX_FLEX_LEN; j++) {
> +		for (; j < I40E_FDIR_MAX_FLEX_LEN - 1; j++) {
>  			if (src_offset[j + 1] == src_offset[j] + 1)
>  				size++;
> -			else {
> -				src_tmp = src_offset[j] + 1 - size;
> -				/* the flex_pit need to be sort by scr_offset */
> -				for (i = 0; i < num; i++) {
> -					if (src_tmp < flex_pit[i].src_offset)
> -						break;
> -				}
> -				/* if insert required, move backward */
> -				for (k = num; k > i; k--)
> -					flex_pit[k] = flex_pit[k - 1];
> -				/* insert */
> -				flex_pit[i].dst_offset = j + 1 - size;
> -				flex_pit[i].src_offset = src_tmp;
> -				flex_pit[i].size = size;
> -				j++;
> -				num++;
> +			else
> +				break;
> +		}
> +		src_tmp = src_offset[j] + 1 - size;
> +		/* the flex_pit need to be sort by src_offset */
> +		for (i = 0; i < num; i++) {
> +			if (src_tmp < flex_pit[i].src_offset)
>  				break;
> -			}
>  		}
> +		/* if insert required, move backward */
> +		for (k = num; k > i; k--)
> +			flex_pit[k] = flex_pit[k - 1];
> +		/* insert */
> +		flex_pit[i].dst_offset = j + 1 - size;
> +		flex_pit[i].src_offset = src_tmp;
> +		flex_pit[i].size = size;
> +		j++;
> +		num++;
>  	}
>  	return num;
>  }
>
Helin Zhang March 31, 2015, 6:11 a.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> Sent: Thursday, February 12, 2015 7:22 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] i40e: fix the issue reported by klocwork
> 
> Klocwork reports array 'src_offset' may use index 16.
> In function i40e_srcoff_to_flx_pit, index j + 1 can reach
> I40E_FDIR_MAX_FLEX_LEN.
> This patch fixes this issue to avoid array bound.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

> ---
>  lib/librte_pmd_i40e/i40e_fdir.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
Thomas Monjalon March 31, 2015, 10:27 a.m. UTC | #3
Hi Helin,

> > Klocwork reports array 'src_offset' may use index 16.
> > In function i40e_srcoff_to_flx_pit, index j + 1 can reach
> > I40E_FDIR_MAX_FLEX_LEN.
> > This patch fixes this issue to avoid array bound.
> > 
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>

Please confirm it's a real bug which needs to be fixed in 2.0,
and/or you are sure this patch won't bring a new problem.

Thanks
Helin Zhang April 1, 2015, 1:26 a.m. UTC | #4
Hi Thomas

Actually it is a bug fix. It would be better to be put in R2.0.
It may not crash, as it just possibly read something out of range. I am waiting the test report from our validation team, and then I will merge that. Thanks for your patience!

Regards,
Helin

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, March 31, 2015 6:28 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org; Wu, Jingjing
> Subject: Re: [dpdk-dev] [PATCH] i40e: fix the issue reported by klocwork
> 
> Hi Helin,
> 
> > > Klocwork reports array 'src_offset' may use index 16.
> > > In function i40e_srcoff_to_flx_pit, index j + 1 can reach
> > > I40E_FDIR_MAX_FLEX_LEN.
> > > This patch fixes this issue to avoid array bound.
> > >
> > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > Acked-by: Helin Zhang <helin.zhang@intel.com>
> 
> Please confirm it's a real bug which needs to be fixed in 2.0, and/or you are sure
> this patch won't bring a new problem.
> 
> Thanks
Thomas Monjalon April 1, 2015, 7:47 p.m. UTC | #5
> > Klocwork reports array 'src_offset' may use index 16.
> > In function i40e_srcoff_to_flx_pit, index j + 1 can reach
> > I40E_FDIR_MAX_FLEX_LEN.
> > This patch fixes this issue to avoid array bound.
> > 
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>

Fixes: d8b90c4eabe9 ("i40e: take flow director flexible payload configuration")

Applied, thanks
diff mbox

Patch

diff --git a/lib/librte_pmd_i40e/i40e_fdir.c b/lib/librte_pmd_i40e/i40e_fdir.c
index 68511c8..bc36d8e 100644
--- a/lib/librte_pmd_i40e/i40e_fdir.c
+++ b/lib/librte_pmd_i40e/i40e_fdir.c
@@ -402,28 +402,27 @@  i40e_srcoff_to_flx_pit(const uint16_t *src_offset,
 
 	while (j < I40E_FDIR_MAX_FLEX_LEN) {
 		size = 1;
-		for (; j < I40E_FDIR_MAX_FLEX_LEN; j++) {
+		for (; j < I40E_FDIR_MAX_FLEX_LEN - 1; j++) {
 			if (src_offset[j + 1] == src_offset[j] + 1)
 				size++;
-			else {
-				src_tmp = src_offset[j] + 1 - size;
-				/* the flex_pit need to be sort by scr_offset */
-				for (i = 0; i < num; i++) {
-					if (src_tmp < flex_pit[i].src_offset)
-						break;
-				}
-				/* if insert required, move backward */
-				for (k = num; k > i; k--)
-					flex_pit[k] = flex_pit[k - 1];
-				/* insert */
-				flex_pit[i].dst_offset = j + 1 - size;
-				flex_pit[i].src_offset = src_tmp;
-				flex_pit[i].size = size;
-				j++;
-				num++;
+			else
+				break;
+		}
+		src_tmp = src_offset[j] + 1 - size;
+		/* the flex_pit need to be sort by src_offset */
+		for (i = 0; i < num; i++) {
+			if (src_tmp < flex_pit[i].src_offset)
 				break;
-			}
 		}
+		/* if insert required, move backward */
+		for (k = num; k > i; k--)
+			flex_pit[k] = flex_pit[k - 1];
+		/* insert */
+		flex_pit[i].dst_offset = j + 1 - size;
+		flex_pit[i].src_offset = src_tmp;
+		flex_pit[i].size = size;
+		j++;
+		num++;
 	}
 	return num;
 }