Fixes for issues reported by Coverity (VPP-972)

Change-Id: I25238debb7081b4467aec4620dfdef33fbef3295
Signed-off-by: Chris Luke <chrisy@flirble.org>

diff --git a/src/svm/svm_fifo.c b/src/svm/svm_fifo.c
index 8fe82f5..42eb1ee 100644 (file)
--- 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 4319f01..ab8e5a0 100644 (file)
--- 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 f703b17..29adea2 100644 (file)
--- 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 aec1295..aa307f1 100644 (file)
--- 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 c90e133..3a92b2e 100644 (file)
--- 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 296654e..a3dc7b8 100644 (file)
--- a/src/vnet/ipsec/ikev2.c
+++ b/src/vnet/ipsec/ikev2.c
@@ -17,6 +17,7 @@
 #include <vnet/vnet.h>
 #include <vnet/pg/pg.h>
 #include <vppinfra/error.h>
+#include <vppinfra/random.h>
 #include <vnet/udp/udp.h>
 #include <vnet/ipsec/ipsec.h>
 #include <vnet/ipsec/ikev2.h>
@@ -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 7ade440..37dcbbf 100644 (file)
--- 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