[dpdk-dev] i40e: fix the issue reported by klocwork
Commit Message
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
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;
> }
>
> -----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(-)
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
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
> > 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
@@ -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;
}