[dpdk-dev] [PATCH v3] net/bonding: fix socket id check

[dpdk-dev] [PATCH v3] net/bonding: fix socket id check

[Min Hu (Connor)]

From: Chengchang Tang <tangchengchang@huawei.com>

The socket ID entered by user is cast to an unsigned integer. However,
the value may be an illegal negative value, which may cause some
problems. In this case, an error should be returned.

In addition, the socket ID may be an invalid positive number, which is
also processed in this patch.

Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v3:
* changed type of socket id.

v2:
* fixed socket id type.
---
 drivers/net/bonding/rte_eth_bond_args.c | 6 +++---
 drivers/net/bonding/rte_eth_bond_pmd.c  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
index 8c5f90d..977f3fe 100644
--- a/drivers/net/bonding/rte_eth_bond_args.c
+++ b/drivers/net/bonding/rte_eth_bond_args.c
@@ -207,13 +207,13 @@ bond_ethdev_parse_socket_id_kvarg(const char *key __rte_unused,
 		return -1;
 
 	errno = 0;
-	socket_id = (uint8_t)strtol(value, &endptr, 10);
+	socket_id = (int)strtol(value, &endptr, 10);
 	if (*endptr != 0 || errno != 0)
 		return -1;
 
 	/* validate socket id value */
-	if (socket_id >= 0) {
-		*(uint8_t *)extra_args = (uint8_t)socket_id;
+	if (socket_id >= 0 && socket_id < RTE_MAX_NUMA_NODES) {
+		*(int *)extra_args = socket_id;
 		return 0;
 	}
 	return -1;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 2e9cea5..4fad563 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3333,8 +3333,8 @@ bond_probe(struct rte_vdev_device *dev)
 	const char *name;
 	struct bond_dev_private *internals;
 	struct rte_kvargs *kvlist;
-	uint8_t bonding_mode, socket_id/*, agg_mode*/;
-	int  arg_count, port_id;
+	uint8_t bonding_mode /*, agg_mode*/;
+	int  arg_count, port_id, socket_id;
 	uint8_t agg_mode;
 	struct rte_eth_dev *eth_dev;
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3] net/bonding: fix socket id check

[Ferruh Yigit]

On 4/27/2021 11:54 AM, Min Hu (Connor) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> The socket ID entered by user is cast to an unsigned integer. However,
> the value may be an illegal negative value, which may cause some
> problems. In this case, an error should be returned.
> 
> In addition, the socket ID may be an invalid positive number, which is
> also processed in this patch.
> 
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v3:
> * changed type of socket id.
> 
> v2:
> * fixed socket id type.
> ---
>  drivers/net/bonding/rte_eth_bond_args.c | 6 +++---
>  drivers/net/bonding/rte_eth_bond_pmd.c  | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
> index 8c5f90d..977f3fe 100644
> --- a/drivers/net/bonding/rte_eth_bond_args.c
> +++ b/drivers/net/bonding/rte_eth_bond_args.c
> @@ -207,13 +207,13 @@ bond_ethdev_parse_socket_id_kvarg(const char *key __rte_unused,
>  		return -1;
>  
>  	errno = 0;
> -	socket_id = (uint8_t)strtol(value, &endptr, 10);
> +	socket_id = (int)strtol(value, &endptr, 10);
>  	if (*endptr != 0 || errno != 0)
>  		return -1;
>  
>  	/* validate socket id value */
> -	if (socket_id >= 0) {
> -		*(uint8_t *)extra_args = (uint8_t)socket_id;
> +	if (socket_id >= 0 && socket_id < RTE_MAX_NUMA_NODES) {
> +		*(int *)extra_args = socket_id;

What do you think to keep this local 'socket_id' variable as "long int" as
reasons commented on previous versions.

>  		return 0;
>  	}
>  	return -1;
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 2e9cea5..4fad563 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -3333,8 +3333,8 @@ bond_probe(struct rte_vdev_device *dev)
>  	const char *name;
>  	struct bond_dev_private *internals;
>  	struct rte_kvargs *kvlist;
> -	uint8_t bonding_mode, socket_id/*, agg_mode*/;
> -	int  arg_count, port_id;
> +	uint8_t bonding_mode /*, agg_mode*/;

Can you please cleanup the 'agg_mode' while you are touching this.

> +	int  arg_count, port_id, socket_id;

Can you add it into new line, instead of appending?

>  	uint8_t agg_mode;
>  	struct rte_eth_dev *eth_dev;
>  
>