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; } -- 1.9.3
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(-)
Tested-by: Min Cao <min.cao@intel.com> Patch name: [dpdk-dev] [PATCH] i40e: fix the issue reported by klocwork Test Flag: Tested-by Tester name: min.cao@intel.com Result summary: total 2 cases, 2passed, 0 failed Test Case 1: Name: ipv4 fwd Environment: OS: Fedora20 3.11.10-301.fc20.x86_64 gcc (GCC) 4.8.2 CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz NIC: Fortville eagle Test result: PASSED Detail: ipv4 fwd Test Case 2: Name: ipv6 fwd Environment: OS: Fedora20 3.11.10-301.fc20.x86_64 gcc (GCC) 4.8.2 CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz NIC: Fortville eagle Test result: PASSED Detail: ipv6 fwd -----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> --- 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; } -- 1.9.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
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