[PATCH 0/6] Coverity related fixes

[PATCH 0/6] Coverity related fixes

[Stephen Hemminger]

Some small stuff that can be picked up after 24.03

Stephen Hemminger (6):
  test/bpf: make sure mbuf is initialized
  net/tap: log if netlink ext ack not possible
  examples/l2fwd-keepalive: use rte_drand_max
  net/qede: replace use of rand()
  pipeline: replace use of rand()
  baseband/fpga_5gnr: don't use rand()

 app/test/test_bpf.c                                | 1 +
 drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 3 ++-
 drivers/net/qede/qede_ethdev.c                     | 4 ++--
 drivers/net/tap/tap_netlink.c                      | 3 ++-
 examples/l2fwd-keepalive/main.c                    | 3 ++-
 lib/pipeline/rte_swx_ipsec.c                       | 3 ++-
 6 files changed, 11 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH 1/6] test/bpf: make sure mbuf is initialized

[Stephen Hemminger]

The BPF filter test was not initializing off load flags.

Coverity issue: 375844
Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_bpf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index f83e72a9adfd..53e3a3112371 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -3341,6 +3341,7 @@ test_bpf_filter_sanity(pcap_t *pcap)
 		struct rte_ipv4_hdr ip_hdr;
 	} *hdr;
 
+	memset(&mb, 0, sizeof(mb));
 	dummy_mbuf_prep(&mb, tbuf, sizeof(tbuf), plen);
 	m = &mb;
 
-- 
2.43.0

Re: [PATCH 1/6] test/bpf: make sure mbuf is initialized

[Tyler Retzlaff]

On Fri, Mar 01, 2024 at 09:57:06AM -0800, Stephen Hemminger wrote:
> The BPF filter test was not initializing off load flags.
> 
> Coverity issue: 375844
> Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Reviewed-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

[PATCH 2/6] net/tap: log if netlink ext ack not possible

[Stephen Hemminger]

If kernel doesn't support external ack, log that to aide
in any problem diagnosis.

Coverity issue: 362848
Fixes: 647909bcf34b ("net/tap: use netlink extended ack support")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/tap_netlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/tap_netlink.c b/drivers/net/tap/tap_netlink.c
index 75af3404b035..d9c260127d52 100644
--- a/drivers/net/tap/tap_netlink.c
+++ b/drivers/net/tap/tap_netlink.c
@@ -72,7 +72,8 @@ tap_nl_init(uint32_t nl_groups)
 
 #ifdef NETLINK_EXT_ACK
 	/* Ask for extended ACK response. on older kernel will ignore request. */
-	setsockopt(fd, SOL_NETLINK, NETLINK_EXT_ACK, &one, sizeof(one));
+	if (setsockopt(fd, SOL_NETLINK, NETLINK_EXT_ACK, &one, sizeof(one)) < 0)
+		TAP_LOG(NOTICE, "Unable to request netlink error information");
 #endif
 
 	if (bind(fd, (struct sockaddr *)&local, sizeof(local)) < 0) {
-- 
2.43.0

[PATCH 3/6] examples/l2fwd-keepalive: use rte_drand_max

[Stephen Hemminger]

There is a better way than using rand() to compute a random interval.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l2fwd-keepalive/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/examples/l2fwd-keepalive/main.c b/examples/l2fwd-keepalive/main.c
index 7e0f99a361bc..c077e7f3d4f0 100644
--- a/examples/l2fwd-keepalive/main.c
+++ b/examples/l2fwd-keepalive/main.c
@@ -26,6 +26,7 @@
 #include <rte_launch.h>
 #include <rte_cycles.h>
 #include <rte_prefetch.h>
+#include <rte_random.h>
 #include <rte_lcore.h>
 #include <rte_per_lcore.h>
 #include <rte_branch_prediction.h>
@@ -220,7 +221,7 @@ l2fwd_main_loop(void)
 	}
 
 	uint64_t tsc_initial = rte_rdtsc();
-	uint64_t tsc_lifetime = (rand()&0x07) * rte_get_tsc_hz();
+	uint64_t tsc_lifetime = rte_rand_max(8 * rte_get_tsc_hz());
 
 	while (!terminate_signal_received) {
 		/* Keepalive heartbeat. 8< */
-- 
2.43.0

[PATCH 4/6] net/qede: replace use of rand()

[Stephen Hemminger]

The function of rand() is a weak random number generator.
Use the DPDK rte_rand() instead.

Note: this doesn't matter for real security, since most drivers
use a fixed RSS default key.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/qede/qede_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 254548d40638..bee9fa4f60a1 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -7,6 +7,7 @@
 #include "qede_ethdev.h"
 #include <rte_string_fns.h>
 #include <rte_alarm.h>
+#include <rte_random.h>
 #include <rte_kvargs.h>
 
 static const struct qed_eth_ops *qed_ops;
@@ -1040,9 +1041,8 @@ static void qede_prandom_bytes(uint32_t *buff)
 {
 	uint8_t i;
 
-	srand((unsigned int)time(NULL));
 	for (i = 0; i < ECORE_RSS_KEY_SIZE; i++)
-		buff[i] = rand();
+		buff[i] = rte_rand();
 }
 
 int qede_config_rss(struct rte_eth_dev *eth_dev)
-- 
2.43.0

Re: [PATCH 4/6] net/qede: replace use of rand()

[Tyler Retzlaff]

On Fri, Mar 01, 2024 at 09:57:09AM -0800, Stephen Hemminger wrote:
> The function of rand() is a weak random number generator.
> Use the DPDK rte_rand() instead.
> 
> Note: this doesn't matter for real security, since most drivers
> use a fixed RSS default key.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Reviewed-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

[PATCH 5/6] pipeline: replace use of rand()

[Stephen Hemminger]

The rand() function is weak and using it for salt might be a future
security issue. Use rte_rand() which has a bigger period and more
secure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pipeline/rte_swx_ipsec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/pipeline/rte_swx_ipsec.c b/lib/pipeline/rte_swx_ipsec.c
index 28576c2a4812..eb97b9eb9106 100644
--- a/lib/pipeline/rte_swx_ipsec.c
+++ b/lib/pipeline/rte_swx_ipsec.c
@@ -7,6 +7,7 @@
 #include <arpa/inet.h>
 
 #include <rte_common.h>
+#include <rte_random.h>
 #include <rte_ip.h>
 #include <rte_tailq.h>
 #include <rte_eal_memconfig.h>
@@ -1453,7 +1454,7 @@ crypto_xform_get(struct rte_swx_ipsec_sa_params *p,
 		switch (p->crypto.cipher_auth.cipher.alg) {
 		case RTE_CRYPTO_CIPHER_AES_CBC:
 		case RTE_CRYPTO_CIPHER_3DES_CBC:
-			salt = (uint32_t)rand();
+			salt = rte_rand();
 			break;
 
 		case RTE_CRYPTO_CIPHER_AES_CTR:
-- 
2.43.0

Re: [PATCH 5/6] pipeline: replace use of rand()

[Tyler Retzlaff]

On Fri, Mar 01, 2024 at 09:57:10AM -0800, Stephen Hemminger wrote:
> The rand() function is weak and using it for salt might be a future
> security issue. Use rte_rand() which has a bigger period and more
> secure.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Reviewed-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

[PATCH 6/6] baseband/fpga_5gnr: don't use rand()

[Stephen Hemminger]

The function rand is very weak and should not be used.
Use the DPDK rte_rand() which is seeded from entropy instead.

Coverity issue: 414987
Fixes: b3d326e438f1 ("baseband/fpga_5gnr_fec: add FPGA mutex")
Cc: hernan.vargas@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
index efc1d3a7725b..9b253cde280d 100644
--- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
+++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
@@ -14,6 +14,7 @@
 #include <bus_pci_driver.h>
 #include <rte_byteorder.h>
 #include <rte_cycles.h>
+#include <rte_random.h>
 
 #include <rte_bbdev.h>
 #include <rte_bbdev_pmd.h>
@@ -1990,7 +1991,7 @@ fpga_5gnr_mutex_acquisition(struct fpga_5gnr_queue *q)
 {
 	uint32_t mutex_ctrl, mutex_read, cnt = 0;
 	/* Assign a unique id for the duration of the DDR access */
-	q->ddr_mutex_uuid = rand();
+	q->ddr_mutex_uuid = rte_rand();
 	/* Request and wait for acquisition of the mutex */
 	mutex_ctrl = (q->ddr_mutex_uuid << 16) + 1;
 	do {
-- 
2.43.0

Re: [PATCH 6/6] baseband/fpga_5gnr: don't use rand()

[Tyler Retzlaff]

On Fri, Mar 01, 2024 at 09:57:11AM -0800, Stephen Hemminger wrote:
> The function rand is very weak and should not be used.
> Use the DPDK rte_rand() which is seeded from entropy instead.
> 
> Coverity issue: 414987
> Fixes: b3d326e438f1 ("baseband/fpga_5gnr_fec: add FPGA mutex")
> Cc: hernan.vargas@intel.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Reviewed-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

RE: [PATCH 6/6] baseband/fpga_5gnr: don't use rand()

[Chautru, Nicolas]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, March 1, 2024 9:57 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Vargas, Hernan
> <hernan.vargas@intel.com>; Chautru, Nicolas <nicolas.chautru@intel.com>
> Subject: [PATCH 6/6] baseband/fpga_5gnr: don't use rand()
> 
> The function rand is very weak and should not be used.
> Use the DPDK rte_rand() which is seeded from entropy instead.
> 
> Coverity issue: 414987
> Fixes: b3d326e438f1 ("baseband/fpga_5gnr_fec: add FPGA mutex")
> Cc: hernan.vargas@intel.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Nicolas Chautru <nicolas.chautru@intel.com>

> ---
>  drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> index efc1d3a7725b..9b253cde280d 100644
> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> @@ -14,6 +14,7 @@
>  #include <bus_pci_driver.h>
>  #include <rte_byteorder.h>
>  #include <rte_cycles.h>
> +#include <rte_random.h>
> 
>  #include <rte_bbdev.h>
>  #include <rte_bbdev_pmd.h>
> @@ -1990,7 +1991,7 @@ fpga_5gnr_mutex_acquisition(struct
> fpga_5gnr_queue *q)  {
>  	uint32_t mutex_ctrl, mutex_read, cnt = 0;
>  	/* Assign a unique id for the duration of the DDR access */
> -	q->ddr_mutex_uuid = rand();
> +	q->ddr_mutex_uuid = rte_rand();
>  	/* Request and wait for acquisition of the mutex */
>  	mutex_ctrl = (q->ddr_mutex_uuid << 16) + 1;
>  	do {
> --
> 2.43.0

Re: [PATCH 0/6] Coverity related fixes

[David Marchand]

On Fri, Mar 1, 2024 at 6:59 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Some small stuff that can be picked up after 24.03
>
> Stephen Hemminger (6):
>   test/bpf: make sure mbuf is initialized
>   net/tap: log if netlink ext ack not possible
>   examples/l2fwd-keepalive: use rte_drand_max
>   net/qede: replace use of rand()
>   pipeline: replace use of rand()
>   baseband/fpga_5gnr: don't use rand()
>
>  app/test/test_bpf.c                                | 1 +
>  drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 3 ++-
>  drivers/net/qede/qede_ethdev.c                     | 4 ++--
>  drivers/net/tap/tap_netlink.c                      | 3 ++-
>  examples/l2fwd-keepalive/main.c                    | 3 ++-
>  lib/pipeline/rte_swx_ipsec.c                       | 3 ++-
>  6 files changed, 11 insertions(+), 6 deletions(-)

Series applied, thanks.


-- 
David Marchand