[10/10] net/bonding: fix configuration assignment overflow

Message ID 1618839289-33224-11-git-send-email-humin29@huawei.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series fixes for clean code |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot fail travis build: failed
ci/github-robot fail github build: failed
ci/iol-testing fail Testing issues
ci/Intel-compilation fail Compilation issues
ci/iol-abi-testing success Testing PASS

Commit Message

humin (Q) April 19, 2021, 1:34 p.m. UTC
From: Chengchang Tang <tangchengchang@huawei.com>

The expression may cause an overflow.

This patch fix the codeDEX static check warning "INTEGER_OVERFLOW".

Fixes: 46fb43683679 ("bond: add mode 4")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Stephen Hemminger June 30, 2023, 6:02 p.m. UTC | #1
On Mon, 19 Apr 2021 21:34:49 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> The expression may cause an overflow.
> 
> This patch fix the codeDEX static check warning "INTEGER_OVERFLOW".
> 
> Fixes: 46fb43683679 ("bond: add mode 4")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
Is the codeDEX static checker publicly available?
Would be good to add to CI infrastructure.


Acked-by: Stephen Hemminger <stephen@networkplumber.org>
  
Stephen Hemminger Oct. 3, 2024, 4:03 p.m. UTC | #2
On Mon, 19 Apr 2021 21:34:49 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> The expression may cause an overflow.
> 
> This patch fix the codeDEX static check warning "INTEGER_OVERFLOW".
> 
> Fixes: 46fb43683679 ("bond: add mode 4")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Several of these patches in this set have been applied.
But the rest need to be resubmitted and the CI test failures need to be fixed.
  
Huisong Li Oct. 8, 2024, 2:19 a.m. UTC | #3
Hi Stephen,

Thanks for your review this series again.


在 2024/10/4 0:03, Stephen Hemminger 写道:
> On Mon, 19 Apr 2021 21:34:49 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
>
>> From: Chengchang Tang <tangchengchang@huawei.com>
>>
>> The expression may cause an overflow.
>>
>> This patch fix the codeDEX static check warning "INTEGER_OVERFLOW".
>>
>> Fixes: 46fb43683679 ("bond: add mode 4")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Several of these patches in this set have been applied.
> But the rest need to be resubmitted and the CI test failures need to be fixed.

Can you past a link or log about the CI test failures so as to fix it?

> .
  
Stephen Hemminger Oct. 10, 2024, 2:53 a.m. UTC | #4
On Mon, 19 Apr 2021 21:34:49 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> The expression may cause an overflow.
> 
> This patch fix the codeDEX static check warning "INTEGER_OVERFLOW".
> 
> Fixes: 46fb43683679 ("bond: add mode 4")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/bonding/rte_eth_bond_8023ad.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
> index 128754f..483f082 100644
> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -1237,7 +1237,7 @@ bond_mode_8023ad_conf_assign(struct mode8023ad_private *mode4,
>  	mode4->aggregate_wait_timeout = conf->aggregate_wait_timeout_ms * ms_ticks;
>  	mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
>  	mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
> -	mode4->update_timeout_us = conf->update_timeout_ms * 1000;
> +	mode4->update_timeout_us = (uint64_t)conf->update_timeout_ms * 1000;

It could overflow, but that would only happen if the timeout_ms was greater than 2^32 / 1000
which is 4295 seconds!  The default is 100 ms. 
	
The driver should do some more validation in bond_8023ad_setup_validate().
It does check that update_timeout_ms is non zero, but it has no upper bound.
  

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index 128754f..483f082 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1237,7 +1237,7 @@  bond_mode_8023ad_conf_assign(struct mode8023ad_private *mode4,
 	mode4->aggregate_wait_timeout = conf->aggregate_wait_timeout_ms * ms_ticks;
 	mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
 	mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
-	mode4->update_timeout_us = conf->update_timeout_ms * 1000;
+	mode4->update_timeout_us = (uint64_t)conf->update_timeout_ms * 1000;
 
 	mode4->dedicated_queues.enabled = 0;
 	mode4->dedicated_queues.rx_qid = UINT16_MAX;