[dpdk-dev] New Defects reported by Coverity Scan for DPDK Data Plane Development Kit

[dpdk-dev] New Defects reported by Coverity Scan for DPDK Data Plane Development Kit

[Mcnamara, John]

Hi, 

The following is forwarded from latest Coverity scan on the DPDK head. As usual, I will send out semi-automated emails to the authors of the new defects.

In the meantime you can then review the defects online at:

    http://scan.coverity.com/projects/dpdk-data-plane-development-kit

You can register as "Contributor/Member" for the DPDK Coverity here:

    http://scan.coverity.com/users/sign_up

John.


Hi,

Please find the latest report on new defect(s) introduced to DPDK Data Plane Development Kit found with Coverity Scan.

9 new defect(s) introduced to DPDK Data Plane Development Kit found with Coverity Scan.
3 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan Showing 9 of 9 defect(s)


** CID 124575:    (STRING_OVERFLOW)
/examples/l2fwd-crypto/main.c: 1005 in l2fwd_crypto_parse_args_long_options()
/examples/l2fwd-crypto/main.c: 982 in l2fwd_crypto_parse_args_long_options()


________________________________________________________________________________________________________
*** CID 124575:    (STRING_OVERFLOW)
/examples/l2fwd-crypto/main.c: 1005 in l2fwd_crypto_parse_args_long_options()
999     
1000     	/* Authentication options */
1001     	else if (strcmp(lgopts[option_index].name, "auth_algo") == 0) {
1002     		retval = parse_auth_algo(&options->auth_xform.auth.algo,
1003     				optarg);
1004     		if (retval == 0)
>>>     CID 124575:    (STRING_OVERFLOW)
>>>     You might overrun the 32 byte fixed-size string "options->string_auth_algo" by copying "optarg" without checking the length.
1005     			strcpy(options->string_auth_algo, optarg);
1006     		return retval;
1007     	}
1008     
1009     	else if (strcmp(lgopts[option_index].name, "auth_op") == 0)
1010     		return parse_auth_op(&options->auth_xform.auth.op,
/examples/l2fwd-crypto/main.c: 982 in l2fwd_crypto_parse_args_long_options()
976     
977     	/* Cipher options */
978     	else if (strcmp(lgopts[option_index].name, "cipher_algo") == 0) {
979     		retval = parse_cipher_algo(&options->cipher_xform.cipher.algo,
980     				optarg);
981     		if (retval == 0)
>>>     CID 124575:    (STRING_OVERFLOW)
>>>     You might overrun the 32 byte fixed-size string "options->string_cipher_algo" by copying "optarg" without checking the length.
982     			strcpy(options->string_cipher_algo, optarg);
983     		return retval;
984     	}
985     
986     	else if (strcmp(lgopts[option_index].name, "cipher_op") == 0)
987     		return parse_cipher_op(&options->cipher_xform.cipher.op,

** CID 124567:  Memory - corruptions  (OVERRUN)
/examples/ip_pipeline/init.c: 246 in app_init_eal()


________________________________________________________________________________________________________
*** CID 124567:  Memory - corruptions  (OVERRUN)
/examples/ip_pipeline/init.c: 246 in app_init_eal()
240     
241     	if (p->socket_mem) {
242     		snprintf(buffer,
243     			sizeof(buffer),
244     			"--socket-mem=%s",
245     			p->socket_mem);
>>>     CID 124567:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "app->eal_argv" of 33 8-byte elements at element index 33 (byte offset 264) using index "n_args++" (which evaluates to 33).
246     		app->eal_argv[n_args++] = strdup(buffer);
247     	}
248     
249     	if (p->huge_dir) {
250     		snprintf(buffer, sizeof(buffer), "--huge-dir=%s", p->huge_dir);
251     		app->eal_argv[n_args++] = strdup(buffer);

** CID 124565:  Null pointer dereferences  (NULL_RETURNS)
/lib/librte_vhost/virtio-net.c: 296 in vhost_destroy_device()


________________________________________________________________________________________________________
*** CID 124565:  Null pointer dereferences  (NULL_RETURNS)
/lib/librte_vhost/virtio-net.c: 296 in vhost_destroy_device()
290      */
291     void
292     vhost_destroy_device(struct vhost_device_ctx ctx)
293     {
294     	struct virtio_net *dev = get_device(ctx);
295     
>>>     CID 124565:  Null pointer dereferences  (NULL_RETURNS)
>>>     Dereferencing a null pointer "dev".
296     	if (dev->flags & VIRTIO_DEV_RUNNING)
297     		notify_ops->destroy_device(dev);
298     
299     	cleanup_device(dev, 1);
300     	free_device(dev);
301     

** CID 124564:  Control flow issues  (MISSING_BREAK)
/app/test-pmd/cmdline.c: 8219 in cmd_flow_director_filter_parsed()


________________________________________________________________________________________________________
*** CID 124564:  Control flow issues  (MISSING_BREAK)
/app/test-pmd/cmdline.c: 8219 in cmd_flow_director_filter_parsed()
8213     	}
8214     
8215     	switch (entry.input.flow_type) {
8216     	case RTE_ETH_FLOW_FRAG_IPV4:
8217     	case RTE_ETH_FLOW_NONFRAG_IPV4_OTHER:
8218     		entry.input.flow.ip4_flow.proto = res->proto_value;
>>>     CID 124564:  Control flow issues  (MISSING_BREAK)
>>>     The above case falls through to this one.
8219     	case RTE_ETH_FLOW_NONFRAG_IPV4_UDP:
8220     	case RTE_ETH_FLOW_NONFRAG_IPV4_TCP:
8221     		IPV4_ADDR_TO_UINT(res->ip_dst,
8222     			entry.input.flow.ip4_flow.dst_ip);
8223     		IPV4_ADDR_TO_UINT(res->ip_src,
8224     			entry.input.flow.ip4_flow.src_ip);

** CID 124563:  Null pointer dereferences  (FORWARD_NULL)
/drivers/net/vmxnet3/vmxnet3_rxtx.c: 734 in vmxnet3_recv_pkts()


________________________________________________________________________________________________________
*** CID 124563:  Null pointer dereferences  (FORWARD_NULL)
/drivers/net/vmxnet3/vmxnet3_rxtx.c: 734 in vmxnet3_recv_pkts()
728     		} else {
729     			struct rte_mbuf *start = rxq->start_seg;
730     
731     			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
732     			VMXNET3_ASSERT(start != NULL);
733     
>>>     CID 124563:  Null pointer dereferences  (FORWARD_NULL)
>>>     Dereferencing null pointer "start".
734     			start->pkt_len += rxm->data_len;
735     			start->nb_segs++;
736     
737     			rxq->last_seg->next = rxm;
738     		}
739     		rxq->last_seg = rxm;

** CID 124562:  Null pointer dereferences  (FORWARD_NULL)
/lib/librte_ether/rte_ethdev.c: 1518 in rte_eth_xstats_get()


________________________________________________________________________________________________________
*** CID 124562:  Null pointer dereferences  (FORWARD_NULL)
/lib/librte_ether/rte_ethdev.c: 1518 in rte_eth_xstats_get()
1512     
1513     	/* global stats */
1514     	for (i = 0; i < RTE_NB_STATS; i++) {
1515     		stats_ptr = RTE_PTR_ADD(&eth_stats,
1516     					rte_stats_strings[i].offset);
1517     		val = *stats_ptr;
>>>     CID 124562:  Null pointer dereferences  (FORWARD_NULL)
>>>     Dereferencing null pointer "xstats".
1518     		snprintf(xstats[count].name, sizeof(xstats[count].name),
1519     			"%s", rte_stats_strings[i].name);
1520     		xstats[count++].value = val;
1521     	}
1522     
1523     	/* per-rxq stats */

** CID 124558:  Security best practices violations  (DC.WEAK_CRYPTO)
/examples/ipsec-secgw/esp.c: 66 in random_iv_u64()


________________________________________________________________________________________________________
*** CID 124558:  Security best practices violations  (DC.WEAK_CRYPTO)
/examples/ipsec-secgw/esp.c: 66 in random_iv_u64()
60     	IPSEC_ASSERT((n & 0x3) == 0);
61     
62     	for (i = 0; i < (n >> 3); i++)
63     		buf[i] = rte_rand();
64     
65     	if (left)
>>>     CID 124558:  Security best practices violations  (DC.WEAK_CRYPTO)
>>>     "lrand48" should not be used for security related applications, as linear congruential algorithms are too easy to break.
66     		*((uint32_t *)&buf[i]) = (uint32_t)lrand48();
67     }
68     
69     /* IPv4 Tunnel */
70     int
71     esp4_tunnel_inbound_pre_crypto(struct rte_mbuf *m, struct ipsec_sa *sa,

** CID 124557:  Error handling issues  (CHECKED_RETURN)
/lib/librte_ether/rte_ethdev.c: 1317 in rte_eth_tx_buffer_init()


________________________________________________________________________________________________________
*** CID 124557:  Error handling issues  (CHECKED_RETURN)
/lib/librte_ether/rte_ethdev.c: 1317 in rte_eth_tx_buffer_init()
1311     {
1312     	if (buffer == NULL)
1313     		return -EINVAL;
1314     
1315     	buffer->size = size;
1316     	if (buffer->error_callback == NULL)
>>>     CID 124557:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "rte_eth_tx_buffer_set_err_callback" without checking return value (as is done elsewhere 6 out of 7 times).
1317     		rte_eth_tx_buffer_set_err_callback(buffer,
1318     				rte_eth_tx_buffer_drop_callback, NULL);
1319     
1320     	return 0;
1321     }
1322     

** CID 124556:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
/lib/librte_vhost/virtio-net.c: 319 in vhost_set_ifname()


________________________________________________________________________________________________________
*** CID 124556:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
/lib/librte_vhost/virtio-net.c: 319 in vhost_set_ifname()
313     	if (dev == NULL)
314     		return;
315     
316     	len = if_len > sizeof(dev->ifname) ?
317     		sizeof(dev->ifname) : if_len;
318     
>>>     CID 124556:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
>>>     Calling strncpy with a maximum size argument of 4096 bytes on destination array "dev->ifname" of size 4096 bytes might leave the destination string unterminated.
319     	strncpy(dev->ifname, if_name, len);
320     }
321     
322     
323     /*
324      * Called from CUSE IOCTL: VHOST_SET_OWNER


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://scan.coverity.com/projects/dpdk-data-plane-development-kit?tab=overview

To manage Coverity Scan email notifications for "john.mcnamara@intel.com", click https://scan.coverity.com/subscriptions/edit?email=john.mcnamara%40intel.com&token=4b4350458ddd299564fa85d2b53fbd6c

[dpdk-dev] [PATCH] xstats: fix behavior when a null array is provided

[Olivier Matz]

Coverity reports an issue in ethdev:

  *** CID 124562:  Null pointer dereferences  (FORWARD_NULL)
  /lib/librte_ether/rte_ethdev.c: 1518 in rte_eth_xstats_get()
  1512
  1513		/* global stats */
  1514     	for (i = 0; i < RTE_NB_STATS; i++) {
  1515     	    stats_ptr = RTE_PTR_ADD(&eth_stats,
  1516
  rte_stats_strings[i].offset);
  1517			val = *stats_ptr;
  >>>     CID 124562:  Null pointer dereferences  (FORWARD_NULL)
  >>>     Dereferencing null pointer "xstats".
  1518     	      	   snprintf(xstats[count].name,
  sizeof(xstats[count].name),
  1519				"%s", rte_stats_strings[i].name);
  1520     			      xstats[count++].value = val;
  1521     			      }
  1522
  1523		/* per-rxq stats */

If a user calls rte_eth_xstats_get(portid, NULL, n) with n != 0,
it may result in a crash. Although the API documentation says that
n is the size of the table and xstats can be NULL if n == 0, we
can add an additional check here to make Coverity happy.

In that case, the return value is the same than when n == 0 is
passed, it returns the number of statistics.

Fixes: ce757f5c9a ("ethdev: new method to retrieve extended statistics")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_ether/rte_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 76a30fd..60d2573 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1503,7 +1503,7 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats,
 			return xcount;
 	}
 
-	if (n < count + xcount)
+	if (n < count + xcount || xstats == NULL)
 		return count + xcount;
 
 	/* now fill the xstats structure */
-- 
2.1.4

Re: [dpdk-dev] [PATCH] xstats: fix behavior when a null array is provided

[Van Haaren, Harry]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Subject: [dpdk-dev] [PATCH] xstats: fix behavior when a null array is provided
> 
> Coverity reports an issue in ethdev:
> 
>   *** CID 124562:  Null pointer dereferences  (FORWARD_NULL)
>   /lib/librte_ether/rte_ethdev.c: 1518 in rte_eth_xstats_get()
>   1512
>   1513		/* global stats */
>   1514     	for (i = 0; i < RTE_NB_STATS; i++) {
>   1515     	    stats_ptr = RTE_PTR_ADD(&eth_stats,
>   1516
>   rte_stats_strings[i].offset);
>   1517			val = *stats_ptr;
>   >>>     CID 124562:  Null pointer dereferences  (FORWARD_NULL)
>   >>>     Dereferencing null pointer "xstats".
>   1518     	      	   snprintf(xstats[count].name,
>   sizeof(xstats[count].name),
>   1519				"%s", rte_stats_strings[i].name);
>   1520     			      xstats[count++].value = val;
>   1521     			      }
>   1522
>   1523		/* per-rxq stats */
> 
> If a user calls rte_eth_xstats_get(portid, NULL, n) with n != 0,
> it may result in a crash. Although the API documentation says that
> n is the size of the table and xstats can be NULL if n == 0, we
> can add an additional check here to make Coverity happy.
> 
> In that case, the return value is the same than when n == 0 is
> passed, it returns the number of statistics.
> 
> Fixes: ce757f5c9a ("ethdev: new method to retrieve extended statistics")
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

I'm unsure on how verbose commit messages are ideal,
but there's certainly enough description here :)

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Re: [dpdk-dev] [PATCH] xstats: fix behavior when a null array is provided

[Thomas Monjalon]

2016-04-05 11:06, Van Haaren, Harry:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > Subject: [dpdk-dev] [PATCH] xstats: fix behavior when a null array is provided
> > 
> > Coverity reports an issue in ethdev:
> > 
> >   *** CID 124562:  Null pointer dereferences  (FORWARD_NULL)
> >   /lib/librte_ether/rte_ethdev.c: 1518 in rte_eth_xstats_get()
> >   1512
> >   1513		/* global stats */
> >   1514     	for (i = 0; i < RTE_NB_STATS; i++) {
> >   1515     	    stats_ptr = RTE_PTR_ADD(&eth_stats,
> >   1516
> >   rte_stats_strings[i].offset);
> >   1517			val = *stats_ptr;
> >   >>>     CID 124562:  Null pointer dereferences  (FORWARD_NULL)
> >   >>>     Dereferencing null pointer "xstats".
> >   1518     	      	   snprintf(xstats[count].name,
> >   sizeof(xstats[count].name),
> >   1519				"%s", rte_stats_strings[i].name);
> >   1520     			      xstats[count++].value = val;
> >   1521     			      }
> >   1522
> >   1523		/* per-rxq stats */
> > 
> > If a user calls rte_eth_xstats_get(portid, NULL, n) with n != 0,
> > it may result in a crash. Although the API documentation says that
> > n is the size of the table and xstats can be NULL if n == 0, we
> > can add an additional check here to make Coverity happy.
> > 
> > In that case, the return value is the same than when n == 0 is
> > passed, it returns the number of statistics.
> > 
> > Fixes: ce757f5c9a ("ethdev: new method to retrieve extended statistics")
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> I'm unsure on how verbose commit messages are ideal,
> but there's certainly enough description here :)
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Applied, thanks