diff mbox series

[v2] net/bonding: fix socket id check

Message ID 1619515014-19181-1-git-send-email-humin29@huawei.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] net/bonding: fix socket id check |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

humin (Q) April 27, 2021, 9:16 a.m. UTC 
  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>
---
v2:
* fixed socket id type.
---
 drivers/net/bonding/rte_eth_bond_args.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Ferruh Yigit April 27, 2021, 10:47 a.m. UTC | #1 
On 4/27/2021 10:16 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>
> ---
> v2:
> * fixed socket id type.
> ---
>  drivers/net/bonding/rte_eth_bond_args.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 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);

Already provided some comment to v1, why it is better to do checks first and
cast later.

>  	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;

The provided 'extra_args' is 'uint8_t', we can't just cast it to "int *" and
assign to it.
diff mbox series

Patch

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;