DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 00/10] fixes for clean code
@ 2021-04-19 13:34 Min Hu (Connor)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 01/10] net/pfe: check return value Min Hu (Connor)
                   ` (9 more replies)
  0 siblings, 10 replies; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

This patchset fix some bugs, to make the code more clean.

Chengchang Tang (1):
  net/bonding: fix configuration assignment overflow

HongBo Zheng (5):
  app/test: add NULL pointer check of memory allocation
  lib/librte_pipeline: fix the use of unsafe strcpy
  examples/l3fwd: add function return value check
  crypto/virtio: fix return values check error
  net/e1000: add function return value check

Min Hu (Connor) (4):
  net/pfe: check return value
  common/sfc_efx/base: delete redundant handling
  bus/dpaa: fix management command init calling
  app/regex: fix division by zero

 app/test-regex/main.c                     | 8 +++++---
 app/test/test_crc.c                       | 2 ++
 drivers/bus/dpaa/base/qbman/bman.c        | 6 ++----
 drivers/common/sfc_efx/base/rhead_nic.c   | 3 +--
 drivers/crypto/virtio/virtio_rxtx.c       | 4 ++--
 drivers/net/bonding/rte_eth_bond_8023ad.c | 2 +-
 drivers/net/e1000/base/e1000_ich8lan.c    | 2 ++
 drivers/net/pfe/pfe_ethdev.c              | 6 ++++++
 examples/l3fwd/l3fwd_event.c              | 6 +++++-
 lib/librte_pipeline/rte_swx_pipeline.c    | 4 ++--
 10 files changed, 28 insertions(+), 15 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 01/10] net/pfe: check return value
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2023-06-30 17:59   ` Stephen Hemminger
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling Min Hu (Connor)
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

Variable 'fd', which may receive negative value when open
"/dev/mem" file.

This patch added checking return value process.

Fixes: 67fc3ff97c39 ("net/pfe: introduce basic functions")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/pfe/pfe_ethdev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/pfe/pfe_ethdev.c b/drivers/net/pfe/pfe_ethdev.c
index 8cf59e2..b4f5591 100644
--- a/drivers/net/pfe/pfe_ethdev.c
+++ b/drivers/net/pfe/pfe_ethdev.c
@@ -1049,6 +1049,12 @@ pmd_pfe_probe(struct rte_vdev_device *vdev)
 	g_pfe->cbus_size = cbus_size;
 
 	fd = open("/dev/mem", O_RDWR);
+	if (fd < 0) {
+		PFE_PMD_ERR("Can not open /dev/mem");
+		rc = -EIO;
+		goto err;
+	}
+
 	g_pfe->cbus_baseaddr = mmap(NULL, cbus_size, PROT_READ | PROT_WRITE,
 					MAP_SHARED, fd, cbus_addr);
 	close(fd);
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 01/10] net/pfe: check return value Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2021-04-20  9:33   ` Andrew Rybchenko
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling Min Hu (Connor)
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

the default case in 'rhead_nic_get_bar_region' is unreachable.

This patch fixed that.

Fixes: 3c1c5cc4a786 ("common/sfc_efx/base: add Riverhead support to NIC module")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/common/sfc_efx/base/rhead_nic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/common/sfc_efx/base/rhead_nic.c b/drivers/common/sfc_efx/base/rhead_nic.c
index f2c18c1..b9af348 100644
--- a/drivers/common/sfc_efx/base/rhead_nic.c
+++ b/drivers/common/sfc_efx/base/rhead_nic.c
@@ -483,8 +483,7 @@ rhead_nic_get_bar_region(
 		break;
 
 	default:
-		rc = EINVAL;
-		goto fail1;
+		break;
 	}
 
 	return (0);
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 01/10] net/pfe: check return value Min Hu (Connor)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2021-04-20  9:35   ` Andrew Rybchenko
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 04/10] app/regex: fix division by zero Min Hu (Connor)
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

'bm_mc_init' only return 0, but the function whicl calls int
check the negative ret, and this is redundant.

This patch fixed it by not checking the return value.

Fixes: f38f61e982f8 ("bus/dpaa: add BMAN hardware interfaces")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/bus/dpaa/base/qbman/bman.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/dpaa/base/qbman/bman.c b/drivers/bus/dpaa/base/qbman/bman.c
index 8a62907..e1ba2a8 100644
--- a/drivers/bus/dpaa/base/qbman/bman.c
+++ b/drivers/bus/dpaa/base/qbman/bman.c
@@ -70,10 +70,8 @@ struct bman_portal *bman_create_portal(struct bman_portal *portal,
 		pr_err("Bman RCR initialisation failed\n");
 		return NULL;
 	}
-	if (bm_mc_init(p)) {
-		pr_err("Bman MC initialisation failed\n");
-		goto fail_mc;
-	}
+	(void)bm_mc_init(p);
+
 	portal->pools = kmalloc(2 * sizeof(*pools), GFP_KERNEL);
 	if (!portal->pools)
 		goto fail_pools;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 04/10] app/regex: fix division by zero
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
                   ` (2 preceding siblings ...)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2021-04-19 17:48   ` Ori Kam
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 05/10] app/test: add null pointer check of memory allocation Min Hu (Connor)
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

Variable nb_jobs, which may be zero, is used as a denominator.

This patch fixed it.

Fixes: f5cffb7eb7fb ("app/regex: read data file once at startup")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test-regex/main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/test-regex/main.c b/app/test-regex/main.c
index 8e665df..b49fa88 100644
--- a/app/test-regex/main.c
+++ b/app/test-regex/main.c
@@ -725,9 +725,11 @@ main(int argc, char **argv)
 	if (data_len <= 0)
 		rte_exit(EXIT_FAILURE, "Error, can't read file, or file is empty.\n");
 
-	job_len = data_len / nb_jobs;
-	if (job_len == 0)
-		rte_exit(EXIT_FAILURE, "Error, To many jobs, for the given input.\n");
+	if (!nb_jobs) {
+		job_len = data_len / nb_jobs;
+		if (job_len == 0)
+			rte_exit(EXIT_FAILURE, "Error, To many jobs, for the given input.\n");
+	}
 
 	if (job_len > nb_max_payload)
 		rte_exit(EXIT_FAILURE, "Error, not enough jobs to cover input.\n");
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 05/10] app/test: add null pointer check of memory allocation
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
                   ` (3 preceding siblings ...)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 04/10] app/regex: fix division by zero Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2022-06-26 17:48   ` Thomas Monjalon
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy Min Hu (Connor)
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

From: HongBo Zheng <zhenghongbo3@huawei.com>

The rte_zmalloc is called in test_crc_calc without null pointer
check. This patch adds null pointer checks on return value of
rte_zmalloc.

Fixes: 9c77b848b1c1 ("test: add CRC computation")
Cc: stable@dpdk.org

Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test/test_crc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test_crc.c b/app/test/test_crc.c
index bf1d344..8231f81 100644
--- a/app/test/test_crc.c
+++ b/app/test/test_crc.c
@@ -80,6 +80,8 @@ test_crc_calc(void)
 
 	/* 32-bit ethernet CRC: Test 2 */
 	test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
+	if (test_data == NULL)
+		return -7;
 
 	for (i = 0; i < CRC32_VEC_LEN1; i += 12)
 		rte_memcpy(&test_data[i], crc32_vec1, 12);
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
                   ` (4 preceding siblings ...)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 05/10] app/test: add null pointer check of memory allocation Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2021-04-20  9:36   ` Andrew Rybchenko
  2023-06-30 18:08   ` Stephen Hemminger
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 07/10] examples/l3fwd: add function return value check Min Hu (Connor)
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

From: HongBo Zheng <zhenghongbo3@huawei.com>

'strcpy' is called in rte_swx_ctl_table_info_get, this function
is unsafe, use 'strncpy' instead.

Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
Cc: stable@dpdk.org

Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
index 4455d91..d4db4dd 100644
--- a/lib/librte_pipeline/rte_swx_pipeline.c
+++ b/lib/librte_pipeline/rte_swx_pipeline.c
@@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline *p,
 	if (!t)
 		return -EINVAL;
 
-	strcpy(table->name, t->name);
-	strcpy(table->args, t->args);
+	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
+	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);
 	table->n_match_fields = t->n_fields;
 	table->n_actions = t->n_actions;
 	table->default_action_is_const = t->default_action_is_const;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 07/10] examples/l3fwd: add function return value check
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
                   ` (5 preceding siblings ...)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2023-06-30 18:15   ` Stephen Hemminger
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 08/10] crypto/virtio: fix return values check error Min Hu (Connor)
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

From: HongBo Zheng <zhenghongbo3@huawei.com>

Return value of a function 'rte_eth_macaddr_get' called at
l3fwd_eth_dev_port_setup is not checked, but it is usually
checked for this function.

This patch fix this problem.

Fixes: a65bf3d724df ("examples/l3fwd: add ethdev setup based on eventdev")
Cc: stable@dpdk.org

Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 examples/l3fwd/l3fwd_event.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/examples/l3fwd/l3fwd_event.c b/examples/l3fwd/l3fwd_event.c
index 4d31593..7f704f9 100644
--- a/examples/l3fwd/l3fwd_event.c
+++ b/examples/l3fwd/l3fwd_event.c
@@ -105,7 +105,11 @@ l3fwd_eth_dev_port_setup(struct rte_eth_conf *port_conf)
 				 "Cannot adjust number of descriptors: err=%d, "
 				 "port=%d\n", ret, port_id);
 
-		rte_eth_macaddr_get(port_id, &ports_eth_addr[port_id]);
+		ret = rte_eth_macaddr_get(port_id, &ports_eth_addr[port_id]);
+		if (ret < 0)
+			rte_exit(EXIT_FAILURE,
+				 "Cannot get MAC address: err=%d, port=%d\n",
+				 ret, port_id);
 		print_ethaddr(" Address:", &ports_eth_addr[port_id]);
 		printf(", ");
 		print_ethaddr("Destination:",
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 08/10] crypto/virtio: fix return values check error
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
                   ` (6 preceding siblings ...)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 07/10] examples/l3fwd: add function return value check Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2023-06-30 18:14   ` Stephen Hemminger
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 09/10] net/e1000: add function return value check Min Hu (Connor)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 10/10] net/bonding: fix configuration assignment overflow Min Hu (Connor)
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

From: HongBo Zheng <zhenghongbo3@huawei.com>

In virtio_crypto_pkt_tx_burst, we check the return values of
virtqueue_crypto_enqueue_xmit, which may returns -ENOSPC/-EMSGSIZE,
but we only check ENOSPC/EMSGSIZE, and cause the result of checks
is always false.

This patch fix this problem.

Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
Cc: stable@dpdk.org

Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/crypto/virtio/virtio_rxtx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_rxtx.c b/drivers/crypto/virtio/virtio_rxtx.c
index e1cb4ad..a35a5b0 100644
--- a/drivers/crypto/virtio/virtio_rxtx.c
+++ b/drivers/crypto/virtio/virtio_rxtx.c
@@ -500,10 +500,10 @@ virtio_crypto_pkt_tx_burst(void *tx_queue, struct rte_crypto_op **tx_pkts,
 		/* Enqueue Packet buffers */
 		error = virtqueue_crypto_enqueue_xmit(txvq, tx_pkts[nb_tx]);
 		if (unlikely(error)) {
-			if (error == ENOSPC)
+			if (error == -ENOSPC)
 				VIRTIO_CRYPTO_TX_LOG_ERR(
 					"virtqueue_enqueue Free count = 0");
-			else if (error == EMSGSIZE)
+			else if (error == -EMSGSIZE)
 				VIRTIO_CRYPTO_TX_LOG_ERR(
 					"virtqueue_enqueue Free count < 1");
 			else
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 09/10] net/e1000: add function return value check
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
                   ` (7 preceding siblings ...)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 08/10] crypto/virtio: fix return values check error Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 10/10] net/bonding: fix configuration assignment overflow Min Hu (Connor)
  9 siblings, 0 replies; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

From: HongBo Zheng <zhenghongbo3@huawei.com>

Return value of a function 'e1000_phy_has_link_generic'
called at e1000_kmrn_lock_loss_workaround_ich8lan is not
checked, but it is usually checked for this function.

This patch fix this problem.

Fixes: 5a32a257f957 ("e1000: more NICs in base driver")
Cc: stable@dpdk.org

Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/e1000/base/e1000_ich8lan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/e1000/base/e1000_ich8lan.c b/drivers/net/e1000/base/e1000_ich8lan.c
index 14f86b7..67adc46 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -5406,6 +5406,8 @@ STATIC s32 e1000_kmrn_lock_loss_workaround_ich8lan(struct e1000_hw *hw)
 	 * stability
 	 */
 	ret_val = e1000_phy_has_link_generic(hw, 1, 0, &link);
+	if (ret_val)
+		return ret_val;
 	if (!link)
 		return E1000_SUCCESS;
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [dpdk-dev] [PATCH 10/10] net/bonding: fix configuration assignment overflow
  2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
                   ` (8 preceding siblings ...)
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 09/10] net/e1000: add function return value check Min Hu (Connor)
@ 2021-04-19 13:34 ` Min Hu (Connor)
  2023-06-30 18:02   ` Stephen Hemminger
  9 siblings, 1 reply; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-19 13:34 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal, orika

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;
 
 	mode4->dedicated_queues.enabled = 0;
 	mode4->dedicated_queues.rx_qid = UINT16_MAX;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 04/10] app/regex: fix division by zero
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 04/10] app/regex: fix division by zero Min Hu (Connor)
@ 2021-04-19 17:48   ` Ori Kam
  0 siblings, 0 replies; 24+ messages in thread
From: Ori Kam @ 2021-04-19 17:48 UTC (permalink / raw)
  To: Min Hu (Connor), dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, andrew.rybchenko, hemant.agrawal

Hi Min,

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Monday, April 19, 2021 4:35 PM
> Subject: [PATCH 04/10] app/regex: fix division by zero
> 
> Variable nb_jobs, which may be zero, is used as a denominator.
> 
> This patch fixed it.
> 
> Fixes: f5cffb7eb7fb ("app/regex: read data file once at startup")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  app/test-regex/main.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-regex/main.c b/app/test-regex/main.c
> index 8e665df..b49fa88 100644
> --- a/app/test-regex/main.c
> +++ b/app/test-regex/main.c
> @@ -725,9 +725,11 @@ main(int argc, char **argv)
>  	if (data_len <= 0)
>  		rte_exit(EXIT_FAILURE, "Error, can't read file, or file is
> empty.\n");
> 
> -	job_len = data_len / nb_jobs;
> -	if (job_len == 0)
> -		rte_exit(EXIT_FAILURE, "Error, To many jobs, for the given
> input.\n");
> +	if (!nb_jobs) {
> +		job_len = data_len / nb_jobs;
> +		if (job_len == 0)
> +			rte_exit(EXIT_FAILURE, "Error, To many jobs, for the
> given input.\n");
> +	}
> 
>  	if (job_len > nb_max_payload)
>  		rte_exit(EXIT_FAILURE, "Error, not enough jobs to cover
> input.\n");
> --
> 2.7.4

Acked-by: Ori Kam <orika@nvidia.com>
Thanks,
Ori

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling Min Hu (Connor)
@ 2021-04-20  9:33   ` Andrew Rybchenko
  2021-04-20  9:42     ` Min Hu (Connor)
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Rybchenko @ 2021-04-20  9:33 UTC (permalink / raw)
  To: Min Hu (Connor), dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, hemant.agrawal, orika

On 4/19/21 4:34 PM, Min Hu (Connor) wrote:
> the default case in 'rhead_nic_get_bar_region' is unreachable.

Why? May be it is true right now, but default case is required
to handle future changes in enum and missing update here.

> 
> This patch fixed that.
> 
> Fixes: 3c1c5cc4a786 ("common/sfc_efx/base: add Riverhead support to NIC module")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Nack

> ---
>  drivers/common/sfc_efx/base/rhead_nic.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/common/sfc_efx/base/rhead_nic.c b/drivers/common/sfc_efx/base/rhead_nic.c
> index f2c18c1..b9af348 100644
> --- a/drivers/common/sfc_efx/base/rhead_nic.c
> +++ b/drivers/common/sfc_efx/base/rhead_nic.c
> @@ -483,8 +483,7 @@ rhead_nic_get_bar_region(
>  		break;
>  
>  	default:
> -		rc = EINVAL;
> -		goto fail1;
> +		break;
>  	}
>  
>  	return (0);
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling Min Hu (Connor)
@ 2021-04-20  9:35   ` Andrew Rybchenko
  2021-04-20  9:54     ` Min Hu (Connor)
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Rybchenko @ 2021-04-20  9:35 UTC (permalink / raw)
  To: Min Hu (Connor), dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, hemant.agrawal, orika

On 4/19/21 4:34 PM, Min Hu (Connor) wrote:
> 'bm_mc_init' only return 0, but the function whicl calls int
> check the negative ret, and this is redundant.
> 
> This patch fixed it by not checking the return value.
> 
> Fixes: f38f61e982f8 ("bus/dpaa: add BMAN hardware interfaces")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/bus/dpaa/base/qbman/bman.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/dpaa/base/qbman/bman.c b/drivers/bus/dpaa/base/qbman/bman.c
> index 8a62907..e1ba2a8 100644
> --- a/drivers/bus/dpaa/base/qbman/bman.c
> +++ b/drivers/bus/dpaa/base/qbman/bman.c
> @@ -70,10 +70,8 @@ struct bman_portal *bman_create_portal(struct bman_portal *portal,
>  		pr_err("Bman RCR initialisation failed\n");
>  		return NULL;
>  	}
> -	if (bm_mc_init(p)) {
> -		pr_err("Bman MC initialisation failed\n");
> -		goto fail_mc;
> -	}
> +	(void)bm_mc_init(p);
> +

As I understand compiler can do it for you and there is no
point to break the code in the case of future changes if
bm_mc_init() starts to return errors.

>  	portal->pools = kmalloc(2 * sizeof(*pools), GFP_KERNEL);
>  	if (!portal->pools)
>  		goto fail_pools;
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy Min Hu (Connor)
@ 2021-04-20  9:36   ` Andrew Rybchenko
  2023-06-30 18:08   ` Stephen Hemminger
  1 sibling, 0 replies; 24+ messages in thread
From: Andrew Rybchenko @ 2021-04-20  9:36 UTC (permalink / raw)
  To: Min Hu (Connor), dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, hemant.agrawal, orika

On 4/19/21 4:34 PM, Min Hu (Connor) wrote:
> From: HongBo Zheng <zhenghongbo3@huawei.com>
> 
> 'strcpy' is called in rte_swx_ctl_table_info_get, this function
> is unsafe, use 'strncpy' instead.
> 
> Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
> index 4455d91..d4db4dd 100644
> --- a/lib/librte_pipeline/rte_swx_pipeline.c
> +++ b/lib/librte_pipeline/rte_swx_pipeline.c
> @@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline *p,
>  	if (!t)
>  		return -EINVAL;
>  
> -	strcpy(table->name, t->name);
> -	strcpy(table->args, t->args);
> +	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
> +	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);

strlcpy() should be used in fact, since strncpy() has problems
as well.

>  	table->n_match_fields = t->n_fields;
>  	table->n_actions = t->n_actions;
>  	table->default_action_is_const = t->default_action_is_const;
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling
  2021-04-20  9:33   ` Andrew Rybchenko
@ 2021-04-20  9:42     ` Min Hu (Connor)
  0 siblings, 0 replies; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-20  9:42 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, hemant.agrawal, orika



在 2021/4/20 17:33, Andrew Rybchenko 写道:
> On 4/19/21 4:34 PM, Min Hu (Connor) wrote:
>> the default case in 'rhead_nic_get_bar_region' is unreachable.
> 
> Why? May be it is true right now, but default case is required
> to handle future changes in enum and missing update here.
> 
Well, agreed, this patch can be abandoned.
>>
>> This patch fixed that.
>>
>> Fixes: 3c1c5cc4a786 ("common/sfc_efx/base: add Riverhead support to NIC module")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> 
> Nack
> 
>> ---
>>   drivers/common/sfc_efx/base/rhead_nic.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/common/sfc_efx/base/rhead_nic.c b/drivers/common/sfc_efx/base/rhead_nic.c
>> index f2c18c1..b9af348 100644
>> --- a/drivers/common/sfc_efx/base/rhead_nic.c
>> +++ b/drivers/common/sfc_efx/base/rhead_nic.c
>> @@ -483,8 +483,7 @@ rhead_nic_get_bar_region(
>>   		break;
>>   
>>   	default:
>> -		rc = EINVAL;
>> -		goto fail1;
>> +		break;
>>   	}
>>   
>>   	return (0);
>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling
  2021-04-20  9:35   ` Andrew Rybchenko
@ 2021-04-20  9:54     ` Min Hu (Connor)
  0 siblings, 0 replies; 24+ messages in thread
From: Min Hu (Connor) @ 2021-04-20  9:54 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou, jia.guo,
	g.singh, hemant.agrawal, orika



在 2021/4/20 17:35, Andrew Rybchenko 写道:
> On 4/19/21 4:34 PM, Min Hu (Connor) wrote:
>> 'bm_mc_init' only return 0, but the function whicl calls int
>> check the negative ret, and this is redundant.
>>
>> This patch fixed it by not checking the return value.
>>
>> Fixes: f38f61e982f8 ("bus/dpaa: add BMAN hardware interfaces")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   drivers/bus/dpaa/base/qbman/bman.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/bus/dpaa/base/qbman/bman.c b/drivers/bus/dpaa/base/qbman/bman.c
>> index 8a62907..e1ba2a8 100644
>> --- a/drivers/bus/dpaa/base/qbman/bman.c
>> +++ b/drivers/bus/dpaa/base/qbman/bman.c
>> @@ -70,10 +70,8 @@ struct bman_portal *bman_create_portal(struct bman_portal *portal,
>>   		pr_err("Bman RCR initialisation failed\n");
>>   		return NULL;
>>   	}
>> -	if (bm_mc_init(p)) {
>> -		pr_err("Bman MC initialisation failed\n");
>> -		goto fail_mc;
>> -	}
>> +	(void)bm_mc_init(p);
>> +
> 
> As I understand compiler can do it for you and there is no
> point to break the code in the case of future changes if
> bm_mc_init() starts to return errors.
> 
Agreed, this patch can be abandoned.
>>   	portal->pools = kmalloc(2 * sizeof(*pools), GFP_KERNEL);
>>   	if (!portal->pools)
>>   		goto fail_pools;
>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 05/10] app/test: add null pointer check of memory allocation
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 05/10] app/test: add null pointer check of memory allocation Min Hu (Connor)
@ 2022-06-26 17:48   ` Thomas Monjalon
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Monjalon @ 2022-06-26 17:48 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: dev, ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou,
	jia.guo, g.singh, andrew.rybchenko, hemant.agrawal, orika

19/04/2021 15:34, Min Hu (Connor):
> From: HongBo Zheng <zhenghongbo3@huawei.com>
> 
> The rte_zmalloc is called in test_crc_calc without null pointer
> check. This patch adds null pointer checks on return value of
> rte_zmalloc.
> 
> Fixes: 9c77b848b1c1 ("test: add CRC computation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Applied only this patch, thanks.




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 01/10] net/pfe: check return value
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 01/10] net/pfe: check return value Min Hu (Connor)
@ 2023-06-30 17:59   ` Stephen Hemminger
  0 siblings, 0 replies; 24+ messages in thread
From: Stephen Hemminger @ 2023-06-30 17:59 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: dev, ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou,
	jia.guo, g.singh, andrew.rybchenko, hemant.agrawal, orika

On Mon, 19 Apr 2021 21:34:40 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

>  
>  	fd = open("/dev/mem", O_RDWR);
> +	if (fd < 0) {
> +		PFE_PMD_ERR("Can not open /dev/mem");
> +		rc = -EIO;
> +		goto err;
> +	}
> +

This patch makes sense and should be applied.
But the errno is most like EPERM so maybe:
	rc = -errno;

PS: /dev/mem is a bad idea, Linux kernel config often disables
it in distributions. Not sure why this driver can't use UIO or VFIO
like normal devices. This should have been caught during code review.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 10/10] net/bonding: fix configuration assignment overflow
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 10/10] net/bonding: fix configuration assignment overflow Min Hu (Connor)
@ 2023-06-30 18:02   ` Stephen Hemminger
  0 siblings, 0 replies; 24+ messages in thread
From: Stephen Hemminger @ 2023-06-30 18:02 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: dev, ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou,
	jia.guo, g.singh, andrew.rybchenko, hemant.agrawal, orika

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>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy Min Hu (Connor)
  2021-04-20  9:36   ` Andrew Rybchenko
@ 2023-06-30 18:08   ` Stephen Hemminger
  2023-07-03 10:57     ` Dumitrescu, Cristian
  1 sibling, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2023-06-30 18:08 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: dev, ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou,
	jia.guo, g.singh, andrew.rybchenko, hemant.agrawal, orika

On Mon, 19 Apr 2021 21:34:45 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> From: HongBo Zheng <zhenghongbo3@huawei.com>
> 
> 'strcpy' is called in rte_swx_ctl_table_info_get, this function
> is unsafe, use 'strncpy' instead.
> 
> Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
> index 4455d91..d4db4dd 100644
> --- a/lib/librte_pipeline/rte_swx_pipeline.c
> +++ b/lib/librte_pipeline/rte_swx_pipeline.c
> @@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline *p,
>  	if (!t)
>  		return -EINVAL;
>  
> -	strcpy(table->name, t->name);
> -	strcpy(table->args, t->args);
> +	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
> +	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);
>  	table->n_match_fields = t->n_fields;
>  	table->n_actions = t->n_actions;
>  	table->default_action_is_const = t->default_action_is_const;

This patch is unnecessary.
Both structures declare the same size for the name and args.
Therefore the strcpy is always safe as long as the table structure
is correctly setup with null terminated string. If not there are worse bugs.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 08/10] crypto/virtio: fix return values check error
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 08/10] crypto/virtio: fix return values check error Min Hu (Connor)
@ 2023-06-30 18:14   ` Stephen Hemminger
  0 siblings, 0 replies; 24+ messages in thread
From: Stephen Hemminger @ 2023-06-30 18:14 UTC (permalink / raw)
  To: Min Hu (Connor), jianjay.zhou
  Cc: dev, ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou,
	jia.guo, g.singh, andrew.rybchenko, hemant.agrawal, orika

On Mon, 19 Apr 2021 21:34:47 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> From: HongBo Zheng <zhenghongbo3@huawei.com>
> 
> In virtio_crypto_pkt_tx_burst, we check the return values of
> virtqueue_crypto_enqueue_xmit, which may returns -ENOSPC/-EMSGSIZE,
> but we only check ENOSPC/EMSGSIZE, and cause the result of checks
> is always false.
> 
> This patch fix this problem.
> 
> Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

This patch looks correct.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dpdk-dev] [PATCH 07/10] examples/l3fwd: add function return value check
  2021-04-19 13:34 ` [dpdk-dev] [PATCH 07/10] examples/l3fwd: add function return value check Min Hu (Connor)
@ 2023-06-30 18:15   ` Stephen Hemminger
  0 siblings, 0 replies; 24+ messages in thread
From: Stephen Hemminger @ 2023-06-30 18:15 UTC (permalink / raw)
  To: Min Hu (Connor), jerinj
  Cc: dev, ferruh.yigit, cristian.dumitrescu, jerinj, jianjay.zhou,
	jia.guo, g.singh, andrew.rybchenko, hemant.agrawal, orika

On Mon, 19 Apr 2021 21:34:46 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> From: HongBo Zheng <zhenghongbo3@huawei.com>
> 
> Return value of a function 'rte_eth_macaddr_get' called at
> l3fwd_eth_dev_port_setup is not checked, but it is usually
> checked for this function.
> 
> This patch fix this problem.
> 
> Fixes: a65bf3d724df ("examples/l3fwd: add ethdev setup based on eventdev")
> Cc: stable@dpdk.org
> 
> Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

This looks correct, but only a buggy driver would never set macaddr.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy
  2023-06-30 18:08   ` Stephen Hemminger
@ 2023-07-03 10:57     ` Dumitrescu, Cristian
  0 siblings, 0 replies; 24+ messages in thread
From: Dumitrescu, Cristian @ 2023-07-03 10:57 UTC (permalink / raw)
  To: Stephen Hemminger, Min Hu (Connor)
  Cc: dev, ferruh.yigit, jerinj, jianjay.zhou, jia.guo, g.singh,
	andrew.rybchenko, hemant.agrawal, orika



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, June 30, 2023 7:09 PM
> To: Min Hu (Connor) <humin29@huawei.com>
> Cc: dev@dpdk.org; ferruh.yigit@intel.com; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; jerinj@marvell.com;
> jianjay.zhou@huawei.com; jia.guo@intel.com; g.singh@nxp.com;
> andrew.rybchenko@oktetlabs.ru; hemant.agrawal@nxp.com; orika@nvidia.com
> Subject: Re: [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe
> strcpy
> 
> On Mon, 19 Apr 2021 21:34:45 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
> > From: HongBo Zheng <zhenghongbo3@huawei.com>
> >
> > 'strcpy' is called in rte_swx_ctl_table_info_get, this function
> > is unsafe, use 'strncpy' instead.
> >
> > Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> > Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> > ---
> >  lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_pipeline/rte_swx_pipeline.c
> b/lib/librte_pipeline/rte_swx_pipeline.c
> > index 4455d91..d4db4dd 100644
> > --- a/lib/librte_pipeline/rte_swx_pipeline.c
> > +++ b/lib/librte_pipeline/rte_swx_pipeline.c
> > @@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline
> *p,
> >  	if (!t)
> >  		return -EINVAL;
> >
> > -	strcpy(table->name, t->name);
> > -	strcpy(table->args, t->args);
> > +	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
> > +	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);
> >  	table->n_match_fields = t->n_fields;
> >  	table->n_actions = t->n_actions;
> >  	table->default_action_is_const = t->default_action_is_const;
> 
> This patch is unnecessary.
> Both structures declare the same size for the name and args.
> Therefore the strcpy is always safe as long as the table structure
> is correctly setup with null terminated string. If not there are worse bugs.


+1
Agree with Steve, this is not necessary here.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-07-03 10:57 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 13:34 [dpdk-dev] [PATCH 00/10] fixes for clean code Min Hu (Connor)
2021-04-19 13:34 ` [dpdk-dev] [PATCH 01/10] net/pfe: check return value Min Hu (Connor)
2023-06-30 17:59   ` Stephen Hemminger
2021-04-19 13:34 ` [dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling Min Hu (Connor)
2021-04-20  9:33   ` Andrew Rybchenko
2021-04-20  9:42     ` Min Hu (Connor)
2021-04-19 13:34 ` [dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling Min Hu (Connor)
2021-04-20  9:35   ` Andrew Rybchenko
2021-04-20  9:54     ` Min Hu (Connor)
2021-04-19 13:34 ` [dpdk-dev] [PATCH 04/10] app/regex: fix division by zero Min Hu (Connor)
2021-04-19 17:48   ` Ori Kam
2021-04-19 13:34 ` [dpdk-dev] [PATCH 05/10] app/test: add null pointer check of memory allocation Min Hu (Connor)
2022-06-26 17:48   ` Thomas Monjalon
2021-04-19 13:34 ` [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy Min Hu (Connor)
2021-04-20  9:36   ` Andrew Rybchenko
2023-06-30 18:08   ` Stephen Hemminger
2023-07-03 10:57     ` Dumitrescu, Cristian
2021-04-19 13:34 ` [dpdk-dev] [PATCH 07/10] examples/l3fwd: add function return value check Min Hu (Connor)
2023-06-30 18:15   ` Stephen Hemminger
2021-04-19 13:34 ` [dpdk-dev] [PATCH 08/10] crypto/virtio: fix return values check error Min Hu (Connor)
2023-06-30 18:14   ` Stephen Hemminger
2021-04-19 13:34 ` [dpdk-dev] [PATCH 09/10] net/e1000: add function return value check Min Hu (Connor)
2021-04-19 13:34 ` [dpdk-dev] [PATCH 10/10] net/bonding: fix configuration assignment overflow Min Hu (Connor)
2023-06-30 18:02   ` Stephen Hemminger

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).