* [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue
@ 2020-05-07 3:09 Chenxu Di
2020-05-07 5:15 ` Ye Xiaolong
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Chenxu Di @ 2020-05-07 3:09 UTC (permalink / raw)
To: dev; +Cc: beilei.xing, Chenxu Di
This patch fixes (out-of-bounds read) coverity issue.
Coverity issue: 357699
Coverity issue: 357694
Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
Signed-off-by: Chenxu Di <chenxux.di@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 749d85f54..6c295ac5a 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -13180,7 +13180,7 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
}
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
- j < I40E_FILTER_PCTYPE_MAX; j++) {
+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
i40e_write_global_rx_ctl(hw,
I40E_GLQF_HSYM(j),
@@ -13312,7 +13312,7 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
}
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
- j < I40E_FILTER_PCTYPE_MAX; j++) {
+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
i40e_write_global_rx_ctl(hw,
I40E_GLQF_HSYM(j),
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue
2020-05-07 3:09 [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue Chenxu Di
@ 2020-05-07 5:15 ` Ye Xiaolong
2020-05-07 5:55 ` Di, ChenxuX
2020-05-07 9:49 ` [dpdk-dev] [PATCH v2] " Chenxu Di
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Ye Xiaolong @ 2020-05-07 5:15 UTC (permalink / raw)
To: Chenxu Di; +Cc: dev, beilei.xing
On 05/07, Chenxu Di wrote:
>This patch fixes (out-of-bounds read) coverity issue.
>
>Coverity issue: 357699
>Coverity issue: 357694
>Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
>
>Signed-off-by: Chenxu Di <chenxux.di@intel.com>
>---
> drivers/net/i40e/i40e_ethdev.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>index 749d85f54..6c295ac5a 100644
>--- a/drivers/net/i40e/i40e_ethdev.c
>+++ b/drivers/net/i40e/i40e_ethdev.c
>@@ -13180,7 +13180,7 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
> }
>
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>- j < I40E_FILTER_PCTYPE_MAX; j++) {
>+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
I see i is defined as uint32_t, why compare it to UINT64_BIT here?
And could you specify where is the out of bounds read before the fix?
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> i40e_write_global_rx_ctl(hw,
> I40E_GLQF_HSYM(j),
>@@ -13312,7 +13312,7 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> }
>
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>- j < I40E_FILTER_PCTYPE_MAX; j++) {
>+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> i40e_write_global_rx_ctl(hw,
> I40E_GLQF_HSYM(j),
>--
>2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue
2020-05-07 5:15 ` Ye Xiaolong
@ 2020-05-07 5:55 ` Di, ChenxuX
2020-05-07 6:30 ` Ye Xiaolong
0 siblings, 1 reply; 16+ messages in thread
From: Di, ChenxuX @ 2020-05-07 5:55 UTC (permalink / raw)
To: Ye, Xiaolong; +Cc: dev, Xing, Beilei
Hi, xiaolong
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Thursday, May 7, 2020 1:15 PM
> To: Di, ChenxuX <chenxux.di@intel.com>
> Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue
>
> On 05/07, Chenxu Di wrote:
> >This patch fixes (out-of-bounds read) coverity issue.
> >
> >Coverity issue: 357699
> >Coverity issue: 357694
> >Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> >flow")
> >
> >Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> >---
> > drivers/net/i40e/i40e_ethdev.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/net/i40e/i40e_ethdev.c
> >b/drivers/net/i40e/i40e_ethdev.c index 749d85f54..6c295ac5a 100644
> >--- a/drivers/net/i40e/i40e_ethdev.c
> >+++ b/drivers/net/i40e/i40e_ethdev.c
> >@@ -13180,7 +13180,7 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
> > }
> >
> > for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> >- j < I40E_FILTER_PCTYPE_MAX; j++) {
> >+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
>
> I see i is defined as uint32_t, why compare it to UINT64_BIT here?
> And could you specify where is the out of bounds read before the fix?
The UINT64_BIT is the define of 64. And i is just used as the index of pctypes_tbl[].
And the code is just copy the function i40e_set_hash_filter_global_config(),
So I don't why he use the define UINT64_BIT as the value 64.
>
> > if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
the out of bounds read is the pctypes_tbl[i]. the above code is that :
for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT64_BIT; i++) {
if (mask0 & (1UL << i))
break;
}
If the loop doesn't break; the value of i will be 64 while the length of pctypes_tbl[] is 64.
> > i40e_write_global_rx_ctl(hw,
> > I40E_GLQF_HSYM(j),
> >@@ -13312,7 +13312,7 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> > }
> >
> > for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> >- j < I40E_FILTER_PCTYPE_MAX; j++) {
> >+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
> > if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> > i40e_write_global_rx_ctl(hw,
> > I40E_GLQF_HSYM(j),
> >--
> >2.17.1
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue
2020-05-07 5:55 ` Di, ChenxuX
@ 2020-05-07 6:30 ` Ye Xiaolong
0 siblings, 0 replies; 16+ messages in thread
From: Ye Xiaolong @ 2020-05-07 6:30 UTC (permalink / raw)
To: Di, ChenxuX; +Cc: dev, Xing, Beilei
On 05/07, Di, ChenxuX wrote:
>Hi, xiaolong
>
>> -----Original Message-----
>> From: Ye, Xiaolong
>> Sent: Thursday, May 7, 2020 1:15 PM
>> To: Di, ChenxuX <chenxux.di@intel.com>
>> Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue
>>
>> On 05/07, Chenxu Di wrote:
>> >This patch fixes (out-of-bounds read) coverity issue.
>> >
>> >Coverity issue: 357699
>> >Coverity issue: 357694
>> >Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
>> >flow")
>> >
>> >Signed-off-by: Chenxu Di <chenxux.di@intel.com>
>> >---
>> > drivers/net/i40e/i40e_ethdev.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/drivers/net/i40e/i40e_ethdev.c
>> >b/drivers/net/i40e/i40e_ethdev.c index 749d85f54..6c295ac5a 100644
>> >--- a/drivers/net/i40e/i40e_ethdev.c
>> >+++ b/drivers/net/i40e/i40e_ethdev.c
>> >@@ -13180,7 +13180,7 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
>> > }
>> >
>> > for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>> >- j < I40E_FILTER_PCTYPE_MAX; j++) {
>> >+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
>>
>> I see i is defined as uint32_t, why compare it to UINT64_BIT here?
>> And could you specify where is the out of bounds read before the fix?
>
>The UINT64_BIT is the define of 64. And i is just used as the index of pctypes_tbl[].
>And the code is just copy the function i40e_set_hash_filter_global_config(),
>So I don't why he use the define UINT64_BIT as the value 64.
>
>>
>> > if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
>
>the out of bounds read is the pctypes_tbl[i]. the above code is that :
>
> for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT64_BIT; i++) {
> if (mask0 & (1UL << i))
> break;
> }
>If the loop doesn't break; the value of i will be 64 while the length of pctypes_tbl[] is 64.
Got it, can you move the i < UINT64_BIT check before the new for loop, so it doesn't
need to check it everytime?
Thanks,
Xiaolong
>
>> > i40e_write_global_rx_ctl(hw,
>> > I40E_GLQF_HSYM(j),
>> >@@ -13312,7 +13312,7 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
>> > }
>> >
>> > for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>> >- j < I40E_FILTER_PCTYPE_MAX; j++) {
>> >+ j < I40E_FILTER_PCTYPE_MAX && i < UINT64_BIT; j++) {
>> > if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
>> > i40e_write_global_rx_ctl(hw,
>> > I40E_GLQF_HSYM(j),
>> >--
>> >2.17.1
>> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
2020-05-07 3:09 [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue Chenxu Di
2020-05-07 5:15 ` Ye Xiaolong
@ 2020-05-07 9:49 ` Chenxu Di
2020-05-08 2:26 ` Yang, Qiming
2020-05-13 2:26 ` [dpdk-dev] [PATCH v3] " Chenxu Di
2020-05-14 7:07 ` [dpdk-dev] [PATCH v4] " Chenxu Di
3 siblings, 1 reply; 16+ messages in thread
From: Chenxu Di @ 2020-05-07 9:49 UTC (permalink / raw)
To: dev; +Cc: Chenxu Di
This patch fixes (out-of-bounds read) coverity issue.
Coverity issue: 357699
Coverity issue: 357694
Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
Signed-off-by: Chenxu Di <chenxux.di@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 749d85f54..c2d5c6835 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
break;
}
+ if (i == UINT64_BIT)
+ return 0;
+
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
j < I40E_FILTER_PCTYPE_MAX; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
@@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
break;
}
+ if (i == UINT64_BIT)
+ return 0;
+
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
j < I40E_FILTER_PCTYPE_MAX; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
2020-05-07 9:49 ` [dpdk-dev] [PATCH v2] " Chenxu Di
@ 2020-05-08 2:26 ` Yang, Qiming
2020-05-08 2:36 ` Ye Xiaolong
0 siblings, 1 reply; 16+ messages in thread
From: Yang, Qiming @ 2020-05-08 2:26 UTC (permalink / raw)
To: Di, ChenxuX, dev; +Cc: Di, ChenxuX
Nacked by: Qiming Yang <Qiming.yang@intel.com>
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Chenxu Di
> Sent: Thursday, May 7, 2020 17:50
> To: dev@dpdk.org
> Cc: Di, ChenxuX <chenxux.di@intel.com>
> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
>
> This patch fixes (out-of-bounds read) coverity issue.
>
> Coverity issue: 357699
> Coverity issue: 357694
> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
>
> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 749d85f54..c2d5c6835 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf
> *pf,
> break;
> }
>
> + if (i == UINT64_BIT)
> + return 0;
> +
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j)) @@ -
> 13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> break;
> }
>
> + if (i == UINT64_BIT)
> + return 0;
> +
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
2020-05-08 2:26 ` Yang, Qiming
@ 2020-05-08 2:36 ` Ye Xiaolong
2020-05-08 2:54 ` Yang, Qiming
0 siblings, 1 reply; 16+ messages in thread
From: Ye Xiaolong @ 2020-05-08 2:36 UTC (permalink / raw)
To: Yang, Qiming; +Cc: Di, ChenxuX, dev
Hi, Qiming
On 05/08, Yang, Qiming wrote:
>Nacked by: Qiming Yang <Qiming.yang@intel.com>
Could you post the reason why you nack this patch?
Thanks,
Xiaolong
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Chenxu Di
>> Sent: Thursday, May 7, 2020 17:50
>> To: dev@dpdk.org
>> Cc: Di, ChenxuX <chenxux.di@intel.com>
>> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
>>
>> This patch fixes (out-of-bounds read) coverity issue.
>>
>> Coverity issue: 357699
>> Coverity issue: 357694
>> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
>>
>> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
>> ---
>> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index 749d85f54..c2d5c6835 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf
>> *pf,
>> break;
>> }
>>
>> + if (i == UINT64_BIT)
>> + return 0;
>> +
>
>
>
>> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>> j < I40E_FILTER_PCTYPE_MAX; j++) {
>> if (pf->adapter->pctypes_tbl[i] & (1ULL << j)) @@ -
>> 13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
>> break;
>> }
>>
>> + if (i == UINT64_BIT)
>> + return 0;
>> +
>> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>> j < I40E_FILTER_PCTYPE_MAX; j++) {
>> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
>> --
>> 2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
2020-05-08 2:36 ` Ye Xiaolong
@ 2020-05-08 2:54 ` Yang, Qiming
0 siblings, 0 replies; 16+ messages in thread
From: Yang, Qiming @ 2020-05-08 2:54 UTC (permalink / raw)
To: Ye, Xiaolong; +Cc: Di, ChenxuX, dev
Hi, Xiaolong
I reviewed that function, it need rework, not only this coverity issue, the logical was wrong.
Qiming
> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Friday, May 8, 2020 10:36
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: Di, ChenxuX <chenxux.di@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
>
> Hi, Qiming
>
> On 05/08, Yang, Qiming wrote:
> >Nacked by: Qiming Yang <Qiming.yang@intel.com>
>
> Could you post the reason why you nack this patch?
>
> Thanks,
> Xiaolong
>
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Chenxu Di
> >> Sent: Thursday, May 7, 2020 17:50
> >> To: dev@dpdk.org
> >> Cc: Di, ChenxuX <chenxux.di@intel.com>
> >> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix out of bounds read issue
> >>
> >> This patch fixes (out-of-bounds read) coverity issue.
> >>
> >> Coverity issue: 357699
> >> Coverity issue: 357694
> >> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> >> flow")
> >>
> >> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> >> ---
> >> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >> b/drivers/net/i40e/i40e_ethdev.c index 749d85f54..c2d5c6835 100644
> >> --- a/drivers/net/i40e/i40e_ethdev.c
> >> +++ b/drivers/net/i40e/i40e_ethdev.c
> >> @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf
> >> *pf,
> >> break;
> >> }
> >>
> >> + if (i == UINT64_BIT)
> >> + return 0;
> >> +
> >
> >
> >
> >> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> >> j < I40E_FILTER_PCTYPE_MAX; j++) {
> >> if (pf->adapter->pctypes_tbl[i] & (1ULL << j)) @@ -
> >> 13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> >> break;
> >> }
> >>
> >> + if (i == UINT64_BIT)
> >> + return 0;
> >> +
> >> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> >> j < I40E_FILTER_PCTYPE_MAX; j++) {
> >> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> >> --
> >> 2.17.1
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v3] net/i40e: fix out of bounds read issue
2020-05-07 3:09 [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue Chenxu Di
2020-05-07 5:15 ` Ye Xiaolong
2020-05-07 9:49 ` [dpdk-dev] [PATCH v2] " Chenxu Di
@ 2020-05-13 2:26 ` Chenxu Di
2020-05-13 6:51 ` Jeff Guo
2020-05-14 7:07 ` [dpdk-dev] [PATCH v4] " Chenxu Di
3 siblings, 1 reply; 16+ messages in thread
From: Chenxu Di @ 2020-05-13 2:26 UTC (permalink / raw)
To: dev; +Cc: beilei.xing, jia.guo, Chenxu Di
This patch fixes (out-of-bounds read) coverity issue.
Coverity issue: 357699
Coverity issue: 357694
Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
Signed-off-by: Chenxu Di <chenxux.di@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 749d85f54..c2d5c6835 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
break;
}
+ if (i == UINT64_BIT)
+ return 0;
+
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
j < I40E_FILTER_PCTYPE_MAX; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
@@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
break;
}
+ if (i == UINT64_BIT)
+ return 0;
+
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
j < I40E_FILTER_PCTYPE_MAX; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: fix out of bounds read issue
2020-05-13 2:26 ` [dpdk-dev] [PATCH v3] " Chenxu Di
@ 2020-05-13 6:51 ` Jeff Guo
2020-05-14 1:16 ` Di, ChenxuX
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Guo @ 2020-05-13 6:51 UTC (permalink / raw)
To: Chenxu Di, dev; +Cc: beilei.xing
hi, chenxu
On 5/13/2020 10:26 AM, Chenxu Di wrote:
> This patch fixes (out-of-bounds read) coverity issue.
>
> Coverity issue: 357699
> Coverity issue: 357694
> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
>
> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 749d85f54..c2d5c6835 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
> break;
> }
>
> + if (i == UINT64_BIT)
> + return 0;
> +
Should this function need to return none zero value and show error info
out, or said should considerate "out of bound" as a configure failed.
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> @@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> break;
> }
>
> + if (i == UINT64_BIT)
> + return 0;
> +
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: fix out of bounds read issue
2020-05-13 6:51 ` Jeff Guo
@ 2020-05-14 1:16 ` Di, ChenxuX
2020-05-14 6:17 ` Jeff Guo
0 siblings, 1 reply; 16+ messages in thread
From: Di, ChenxuX @ 2020-05-14 1:16 UTC (permalink / raw)
To: Guo, Jia, dev; +Cc: Xing, Beilei
Hi,
> -----Original Message-----
> From: Guo, Jia
> Sent: Wednesday, May 13, 2020 2:51 PM
> To: Di, ChenxuX <chenxux.di@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v3] net/i40e: fix out of bounds read issue
>
> hi, chenxu
>
> On 5/13/2020 10:26 AM, Chenxu Di wrote:
> > This patch fixes (out-of-bounds read) coverity issue.
> >
> > Coverity issue: 357699
> > Coverity issue: 357694
> > Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> > flow")
> >
> > Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 749d85f54..c2d5c6835 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
> > break;
> > }
> >
> > + if (i == UINT64_BIT)
> > + return 0;
> > +
>
>
> Should this function need to return none zero value and show error info out, or
> said should considerate "out of bound" as a configure failed.
>
It will just find the flow type and do the things. If not find the flow type, it just do nothing and return.
>
> > for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> > j < I40E_FILTER_PCTYPE_MAX; j++) {
> > if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> > @@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> > break;
> > }
> >
> > + if (i == UINT64_BIT)
> > + return 0;
> > +
> > for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> > j < I40E_FILTER_PCTYPE_MAX; j++) {
> > if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: fix out of bounds read issue
2020-05-14 1:16 ` Di, ChenxuX
@ 2020-05-14 6:17 ` Jeff Guo
2020-05-14 6:41 ` Di, ChenxuX
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Guo @ 2020-05-14 6:17 UTC (permalink / raw)
To: Di, ChenxuX, dev; +Cc: Xing, Beilei
On 5/14/2020 9:16 AM, Di, ChenxuX wrote:
> Hi,
>
>
>
>> -----Original Message-----
>> From: Guo, Jia
>> Sent: Wednesday, May 13, 2020 2:51 PM
>> To: Di, ChenxuX <chenxux.di@intel.com>; dev@dpdk.org
>> Cc: Xing, Beilei <beilei.xing@intel.com>
>> Subject: Re: [PATCH v3] net/i40e: fix out of bounds read issue
>>
>> hi, chenxu
>>
>> On 5/13/2020 10:26 AM, Chenxu Di wrote:
>>> This patch fixes (out-of-bounds read) coverity issue.
>>>
>>> Coverity issue: 357699
>>> Coverity issue: 357694
>>> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
>>> flow")
>>>
>>> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
>>> ---
>>> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/net/i40e/i40e_ethdev.c
>>> b/drivers/net/i40e/i40e_ethdev.c index 749d85f54..c2d5c6835 100644
>>> --- a/drivers/net/i40e/i40e_ethdev.c
>>> +++ b/drivers/net/i40e/i40e_ethdev.c
>>> @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
>>> break;
>>> }
>>>
>>> + if (i == UINT64_BIT)
>>> + return 0;
>>> +
>>
>> Should this function need to return none zero value and show error info out, or
>> said should considerate "out of bound" as a configure failed.
>>
> It will just find the flow type and do the things. If not find the flow type, it just do nothing and return.
i40e_rss_config_hash_function is return int value right, should it
always return 0? Should the case of not finding be considerate as
none-success config?
>>> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>>> j < I40E_FILTER_PCTYPE_MAX; j++) {
>>> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
>>> @@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
>>> break;
>>> }
>>>
>>> + if (i == UINT64_BIT)
>>> + return 0;
>>> +
>>> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
>>> j < I40E_FILTER_PCTYPE_MAX; j++) {
>>> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: fix out of bounds read issue
2020-05-14 6:17 ` Jeff Guo
@ 2020-05-14 6:41 ` Di, ChenxuX
0 siblings, 0 replies; 16+ messages in thread
From: Di, ChenxuX @ 2020-05-14 6:41 UTC (permalink / raw)
To: Guo, Jia, dev; +Cc: Xing, Beilei
Hi
> -----Original Message-----
> From: Guo, Jia
> Sent: Thursday, May 14, 2020 2:17 PM
> To: Di, ChenxuX <chenxux.di@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v3] net/i40e: fix out of bounds read issue
>
>
> On 5/14/2020 9:16 AM, Di, ChenxuX wrote:
> > Hi,
> >
> >
> >
> >> -----Original Message-----
> >> From: Guo, Jia
> >> Sent: Wednesday, May 13, 2020 2:51 PM
> >> To: Di, ChenxuX <chenxux.di@intel.com>; dev@dpdk.org
> >> Cc: Xing, Beilei <beilei.xing@intel.com>
> >> Subject: Re: [PATCH v3] net/i40e: fix out of bounds read issue
> >>
> >> hi, chenxu
> >>
> >> On 5/13/2020 10:26 AM, Chenxu Di wrote:
> >>> This patch fixes (out-of-bounds read) coverity issue.
> >>>
> >>> Coverity issue: 357699
> >>> Coverity issue: 357694
> >>> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> >>> flow")
> >>>
> >>> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> >>> ---
> >>> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >>> b/drivers/net/i40e/i40e_ethdev.c index 749d85f54..c2d5c6835 100644
> >>> --- a/drivers/net/i40e/i40e_ethdev.c
> >>> +++ b/drivers/net/i40e/i40e_ethdev.c
> >>> @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf
> *pf,
> >>> break;
> >>> }
> >>>
> >>> + if (i == UINT64_BIT)
> >>> + return 0;
> >>> +
> >>
> >> Should this function need to return none zero value and show error
> >> info out, or said should considerate "out of bound" as a configure failed.
> >>
> > It will just find the flow type and do the things. If not find the flow type, it just
> do nothing and return.
>
>
> i40e_rss_config_hash_function is return int value right, should it always return 0?
> Should the case of not finding be considerate as none-success config?
>
The original code has too many tabs (for->if->for->if ), it is over 90 words per line.
So just break and move it out .
After break the loop,
if the first loop go to the end without break, the value i will be 64 while the length of pf->adapter->pctypes_tbl is 64.
That is the reason of the out of bound which the original code will not cause.
Actually it will not happen the case of not finding. It is just the code fix the out of bound caused by breaking the loop .
>
> >>> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> >>> j < I40E_FILTER_PCTYPE_MAX; j++) {
> >>> if (pf->adapter->pctypes_tbl[i] & (1ULL << j)) @@ -
> 13311,6
> >>> +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> >>> break;
> >>> }
> >>>
> >>> + if (i == UINT64_BIT)
> >>> + return 0;
> >>> +
> >>> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> >>> j < I40E_FILTER_PCTYPE_MAX; j++) {
> >>> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v4] net/i40e: fix out of bounds read issue
2020-05-07 3:09 [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue Chenxu Di
` (2 preceding siblings ...)
2020-05-13 2:26 ` [dpdk-dev] [PATCH v3] " Chenxu Di
@ 2020-05-14 7:07 ` Chenxu Di
2020-05-14 9:07 ` Jeff Guo
2020-05-15 3:22 ` Ye Xiaolong
3 siblings, 2 replies; 16+ messages in thread
From: Chenxu Di @ 2020-05-14 7:07 UTC (permalink / raw)
To: dev; +Cc: beilei.xing, jia.guo, Chenxu Di
This patch fixes (out-of-bounds read) coverity issue.
Coverity issue: 357699
Coverity issue: 357694
Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
Signed-off-by: Chenxu Di <chenxux.di@intel.com>
---
v4:
-Updated the return value.
---
drivers/net/i40e/i40e_ethdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 749d85f54..91dcd0ebf 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
break;
}
+ if (i == UINT64_BIT)
+ return -EINVAL;
+
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
j < I40E_FILTER_PCTYPE_MAX; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
@@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
break;
}
+ if (i == UINT64_BIT)
+ return -EINVAL;
+
for (j = I40E_FILTER_PCTYPE_INVALID + 1;
j < I40E_FILTER_PCTYPE_MAX; j++) {
if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v4] net/i40e: fix out of bounds read issue
2020-05-14 7:07 ` [dpdk-dev] [PATCH v4] " Chenxu Di
@ 2020-05-14 9:07 ` Jeff Guo
2020-05-15 3:22 ` Ye Xiaolong
1 sibling, 0 replies; 16+ messages in thread
From: Jeff Guo @ 2020-05-14 9:07 UTC (permalink / raw)
To: Chenxu Di, dev; +Cc: beilei.xing
On 5/14/2020 3:07 PM, Chenxu Di wrote:
> This patch fixes (out-of-bounds read) coverity issue.
>
> Coverity issue: 357699
> Coverity issue: 357694
> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
>
> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> ---
> v4:
> -Updated the return value.
> ---
> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 749d85f54..91dcd0ebf 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
> break;
> }
>
> + if (i == UINT64_BIT)
> + return -EINVAL;
> +
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
> @@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> break;
> }
>
> + if (i == UINT64_BIT)
> + return -EINVAL;
> +
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
Reviewed-by: Jeff Guo <jia.guo@intel.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v4] net/i40e: fix out of bounds read issue
2020-05-14 7:07 ` [dpdk-dev] [PATCH v4] " Chenxu Di
2020-05-14 9:07 ` Jeff Guo
@ 2020-05-15 3:22 ` Ye Xiaolong
1 sibling, 0 replies; 16+ messages in thread
From: Ye Xiaolong @ 2020-05-15 3:22 UTC (permalink / raw)
To: Chenxu Di; +Cc: dev, beilei.xing, jia.guo
Minor nit: 'issue' doesn't need to be mentioned in the commit title, since the
'fix' implies something is broken.
On 05/14, Chenxu Di wrote:
>This patch fixes (out-of-bounds read) coverity issue.
>
>Coverity issue: 357699
>Coverity issue: 357694
>Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
>
>Signed-off-by: Chenxu Di <chenxux.di@intel.com>
>---
>v4:
>-Updated the return value.
>---
> drivers/net/i40e/i40e_ethdev.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>index 749d85f54..91dcd0ebf 100644
>--- a/drivers/net/i40e/i40e_ethdev.c
>+++ b/drivers/net/i40e/i40e_ethdev.c
>@@ -13179,6 +13179,9 @@ i40e_rss_config_hash_function(struct i40e_pf *pf,
> break;
> }
>
>+ if (i == UINT64_BIT)
>+ return -EINVAL;
>+
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
>@@ -13311,6 +13314,9 @@ i40e_rss_clear_hash_function(struct i40e_pf *pf,
> break;
> }
>
>+ if (i == UINT64_BIT)
>+ return -EINVAL;
>+
> for (j = I40E_FILTER_PCTYPE_INVALID + 1;
> j < I40E_FILTER_PCTYPE_MAX; j++) {
> if (pf->adapter->pctypes_tbl[i] & (1ULL << j))
>--
>2.17.1
>
Applied to dpdk-next-net-intel, Thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-05-15 3:30 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 3:09 [dpdk-dev] [PATCH] net/i40e: fix out of bounds read issue Chenxu Di
2020-05-07 5:15 ` Ye Xiaolong
2020-05-07 5:55 ` Di, ChenxuX
2020-05-07 6:30 ` Ye Xiaolong
2020-05-07 9:49 ` [dpdk-dev] [PATCH v2] " Chenxu Di
2020-05-08 2:26 ` Yang, Qiming
2020-05-08 2:36 ` Ye Xiaolong
2020-05-08 2:54 ` Yang, Qiming
2020-05-13 2:26 ` [dpdk-dev] [PATCH v3] " Chenxu Di
2020-05-13 6:51 ` Jeff Guo
2020-05-14 1:16 ` Di, ChenxuX
2020-05-14 6:17 ` Jeff Guo
2020-05-14 6:41 ` Di, ChenxuX
2020-05-14 7:07 ` [dpdk-dev] [PATCH v4] " Chenxu Di
2020-05-14 9:07 ` Jeff Guo
2020-05-15 3:22 ` Ye Xiaolong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).