From: Chris Luke Date: Thu, 7 Sep 2017 11:40:13 +0000 (-0400) Subject: Fixes for issues reported by Coverity (VPP-972) X-Git-Tag: v17.10-rc1~113 X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=ab7b8d93cf1098970bc17fb4937376bb1ff33a21 Fixes for issues reported by Coverity (VPP-972) Change-Id: I25238debb7081b4467aec4620dfdef33fbef3295 Signed-off-by: Chris Luke --- diff --git a/src/svm/svm_fifo.c b/src/svm/svm_fifo.c index 8fe82f56abd..42eb1ee8f88 100644 --- a/src/svm/svm_fifo.c +++ b/src/svm/svm_fifo.c @@ -443,7 +443,7 @@ ooo_segment_try_collect (svm_fifo_t * f, u32 n_bytes_enqueued) } } - ASSERT (bytes >= 0 && bytes <= f->nitems); + ASSERT (bytes <= f->nitems); return bytes; } diff --git a/src/uri/sock_test_client.c b/src/uri/sock_test_client.c index 4319f01b5ab..ab8e5a0e4a7 100644 --- a/src/uri/sock_test_client.c +++ b/src/uri/sock_test_client.c @@ -895,6 +895,8 @@ main (int argc, char **argv) case 'w': fprintf (stderr, "ERROR: Option -%c requires an argument.\n", optopt); + break; + default: if (isprint (optopt)) fprintf (stderr, "ERROR: Unknown option `-%c'.\n", optopt); diff --git a/src/uri/sock_test_server.c b/src/uri/sock_test_server.c index f703b177385..29adea2574a 100644 --- a/src/uri/sock_test_server.c +++ b/src/uri/sock_test_server.c @@ -514,9 +514,15 @@ main (int argc, char **argv) continue; } - else if (strlen ((char *) conn->buf)) - printf ("\nSERVER (fd %d): RX (%d bytes) - '%s'\n", - conn->fd, rx_bytes, conn->buf); + else if (((char *) conn->buf)[0] != 0) + { + // If it looks vaguely like a string, make sure it's terminated + ((char *) conn->buf)[rx_bytes < + conn->buf_size ? rx_bytes : + conn->buf_size - 1] = 0; + printf ("\nSERVER (fd %d): RX (%d bytes) - '%s'\n", + conn->fd, rx_bytes, conn->buf); + } } else // rx_bytes < 0 { diff --git a/src/uri/vppcom.c b/src/uri/vppcom.c index aec1295f1b3..aa307f1d333 100644 --- a/src/uri/vppcom.c +++ b/src/uri/vppcom.c @@ -1369,23 +1369,18 @@ vppcom_cfg_heapsize (char *conf_fname) argc++; char **tmp = realloc (argv, argc * sizeof (char *)); if (tmp == NULL) - { - fclose (fp); - goto defaulted; - } + goto defaulted; argv = tmp; arg = strndup (p, 1024); if (arg == NULL) - { - fclose (fp); - goto defaulted; - } + goto defaulted; argv[argc - 1] = arg; p = strtok (NULL, " \t\n"); } } fclose (fp); + fp = NULL; char **tmp = realloc (argv, (argc + 1) * sizeof (char *)); if (tmp == NULL) @@ -1438,6 +1433,10 @@ vppcom_cfg_heapsize (char *conf_fname) } defaulted: + if (fp != NULL) + fclose (fp); + if (argv != NULL) + free (argv); if (!clib_mem_init (0, vcl_cfg->heapsize)) clib_warning ("[%d] vppcom heap allocation failure!", vcm->my_pid); else if (VPPCOM_DEBUG > 0) @@ -1687,7 +1686,7 @@ input_done: unformat_free (input); file_done: - if (fd > 0) + if (fd >= 0) close (fd); } diff --git a/src/vlib/unix/main.c b/src/vlib/unix/main.c index c90e1331374..3a92b2e3d12 100644 --- a/src/vlib/unix/main.c +++ b/src/vlib/unix/main.c @@ -434,6 +434,10 @@ unix_config (vlib_main_t * vm, unformat_input_t * input) vlib_default_runtime_dir, 0); } + error = setup_signal_handlers (um); + if (error) + return error; + if (um->pidfile) { if ((error = vlib_unix_validate_runtime_file (um, @@ -448,10 +452,6 @@ unix_config (vlib_main_t * vm, unformat_input_t * input) } } - error = setup_signal_handlers (um); - if (error) - return error; - if (!(um->flags & UNIX_FLAG_INTERACTIVE)) { openlog (vm->name, LOG_CONS | LOG_PERROR | LOG_PID, LOG_DAEMON); diff --git a/src/vnet/ipsec/ikev2.c b/src/vnet/ipsec/ikev2.c index 296654ecbac..a3dc7b872c5 100644 --- a/src/vnet/ipsec/ikev2.c +++ b/src/vnet/ipsec/ikev2.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -1595,8 +1596,16 @@ ikev2_create_tunnel_interface (vnet_main_t * vnm, ikev2_sa_t * sa, + sa->profile->lifetime; if (sa->profile->lifetime_jitter) { + // This is not much better than rand(3), which Coverity warns + // is unsuitable for security applications; random_u32 is + // however fast. If this perturbance to the expiration time + // needs to use a better RNG then we may need to use something + // like /dev/urandom which has significant overhead. + u32 rnd = (u32) (vlib_time_now (vnm->vlib_main) * 1e6); + rnd = random_u32 (&rnd); + child->time_to_expiration += - 1 + (rand () % sa->profile->lifetime_jitter); + 1 + (rnd % sa->profile->lifetime_jitter); } } diff --git a/src/vppinfra/socket.c b/src/vppinfra/socket.c index 7ade440c51c..37dcbbfd8c6 100644 --- a/src/vppinfra/socket.c +++ b/src/vppinfra/socket.c @@ -359,9 +359,21 @@ clib_socket_init (clib_socket_t * s) && s->flags & SOCKET_ALLOW_GROUP_WRITE) { struct stat st = { 0 }; - stat (((struct sockaddr_un *) &addr)->sun_path, &st); + if (stat (((struct sockaddr_un *) &addr)->sun_path, &st) < 0) + { + error = clib_error_return_unix (0, "stat (fd %d, '%s')", + s->fd, s->config); + goto done; + } st.st_mode |= S_IWGRP; - chmod (((struct sockaddr_un *) &addr)->sun_path, st.st_mode); + if (chmod (((struct sockaddr_un *) &addr)->sun_path, st.st_mode) < + 0) + { + error = + clib_error_return_unix (0, "chmod (fd %d, '%s', mode %o)", + s->fd, s->config, st.st_mode); + goto done; + } } } else