Fixes for issues Coverity has reported (VPP-972)

177117: fstat() returns -1 on error; the code is
        checking for any positive value instead
175142: final return could never be reached; simple
        refactoring
175235,175236: Warning suppressed with an explicit
        cast to (void)
174817: Final return couldn't be reached; is
        is_in_order is 0 then 'rv' is already returned
        above
172095,172093: If is_is_set does not get set to 1,
        then return 0 has already been invoked
174405: Re-kill this (nothing sets rv)
171136: Looks like a cmd line flag to set test_bytes
        was missing; added it, and refactored the
        argc/argv processing to avoid two other
        potential segv's
176813: Add range checking for term width/height.
        First stab at a reasonable range is 1-512
        for both.
175350: Fix implicit casting in shift operation
174272: Not a c+p error; try using a coverity
        annotation to ignore it
174273,175320: Annotated FORWARD_NULL

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

diff --git a/src/uri/sock_test_client.c b/src/uri/sock_test_client.c
index ab8e5a0..151c90b 100644 (file)
--- a/src/uri/sock_test_client.c
+++ b/src/uri/sock_test_client.c
@@ -429,6 +429,7 @@ exit_client (void)
       tsock = &scm->test_socket[i];
       tsock->cfg.test = SOCK_TEST_TYPE_EXIT;
 
+      /* coverity[COPY_PASTE_ERROR] */
       if (ctrl->cfg.verbose)
        {
          printf ("\nCLIENT (fd %d): Sending exit cfg to server...\n",

diff --git a/src/uri/uri_socket_test.c b/src/uri/uri_socket_test.c
index 5f7084d..4469b03 100644 (file)
--- a/src/uri/uri_socket_test.c
+++ b/src/uri/uri_socket_test.c
@@ -36,8 +36,6 @@ main (int argc, char *argv[])
 
   if (argc >= 3)
     {
-      bytes = ((long) atoi (argv[4])) << 20;
-      no_echo = atoi (argv[3]);
       portno = atoi (argv[2]);
       server = gethostbyname (argv[1]);
       if (server == NULL)
@@ -45,6 +43,28 @@ main (int argc, char *argv[])
          clib_unix_warning ("gethostbyname");
          exit (1);
        }
+
+      argc -= 3;
+      argv += 3;
+
+      if (argc)
+       {
+         bytes = ((long) atoi (argv[0])) << 20;
+         argc--;
+         argv++;
+       }
+      if (argc)
+       {
+         no_echo = atoi (argv[0]);
+         argc--;
+         argv++;
+       }
+      if (argc)
+       {
+         test_bytes = atoi (argv[0]);
+         argc--;
+         argv++;
+       }
     }
   else
     {

diff --git a/src/uri/vppcom.c b/src/uri/vppcom.c
index 8a8a806..c7ae0ea 100644 (file)
--- a/src/uri/vppcom.c
+++ b/src/uri/vppcom.c
@@ -1478,7 +1478,7 @@ vppcom_cfg_read (char *conf_fname)
 
   while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
     {
-      unformat_user (input, unformat_line_input, line_input);
+      (void) unformat_user (input, unformat_line_input, line_input);
       unformat_skip_white_space (line_input);
 
       if (unformat (line_input, "vppcom {"))
@@ -2359,12 +2359,14 @@ vppcom_select (unsigned long n_bits, unsigned long *read_map,
               clib_bitmap_get (vcm->ex_bitmap, session_index) && (rv < 0))
             {
               // TBD: clib_warning
+              /* coverity[FORWARD_NULL] */
               clib_bitmap_set_no_check (except_map, session_index, 1);
               bits_set++;
             }
           else if (rv > 0)
             {
               // TBD: clib_warning
+              /* coverity[FORWARD_NULL] */
               clib_bitmap_set_no_check (read_map, session_index, 1);
               bits_set++;
             }
@@ -2387,9 +2389,10 @@ vppcom_select (unsigned long n_bits, unsigned long *read_map,
 
           rv = vppcom_session_write_ready (session, session_index);
           clib_spinlock_unlock (&vcm->sessions_lockp);
-          if (rv > 0)
+          if (rv > 0 )
             {
               // TBD: clib_warning
+              /* coverity[FORWARD_NULL] */
               clib_bitmap_set_no_check (write_map, session_index, 1);
               bits_set++;
             }
@@ -2415,6 +2418,7 @@ vppcom_select (unsigned long n_bits, unsigned long *read_map,
           if (rv < 0)
             {
               // TBD: clib_warning
+              /* coverity[FORWARD_NULL] */
               clib_bitmap_set_no_check (except_map, session_index, 1);
               bits_set++;
             }

diff --git a/src/vlib/linux/physmem.c b/src/vlib/linux/physmem.c
index 3cc42a0..6d3f7c5 100644 (file)
--- a/src/vlib/linux/physmem.c
+++ b/src/vlib/linux/physmem.c
@@ -157,7 +157,7 @@ unix_physmem_region_alloc (vlib_main_t * vm, char *name, u32 size,
   pr->mem = alloc.addr;
   pr->log2_page_size = alloc.log2_page_size;
   pr->n_pages = alloc.n_pages;
-  pr->size = pr->n_pages << pr->log2_page_size;
+  pr->size = (u64) pr->n_pages << (u64) pr->log2_page_size;
   pr->page_mask = (1 << pr->log2_page_size) - 1;
   pr->numa_node = numa_node;
   pr->name = format (0, "%s", name);

diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c
index 1567cc2..1624ce3 100644 (file)
--- a/src/vlib/unix/cli.c
+++ b/src/vlib/unix/cli.c
@@ -91,6 +91,15 @@
  * protocol message. This is a saftey measure. */
 #define UNIX_CLI_MAX_DEPTH_TELNET 24
 
+/** Minimum terminal width we will accept */
+#define UNIX_CLI_MIN_TERMINAL_WIDTH 1
+/** Maximum terminal width we will accept */
+#define UNIX_CLI_MAX_TERMINAL_WIDTH 512
+/** Minimum terminal height we will accept */
+#define UNIX_CLI_MIN_TERMINAL_HEIGHT 1
+/** Maximum terminal height we will accept */
+#define UNIX_CLI_MAX_TERMINAL_HEIGHT 512
+
 /** Unix standard in */
 #define UNIX_CLI_STDIN_FD 0
 
@@ -1164,10 +1173,21 @@ unix_cli_process_telnet (unix_main_t * um,
                    /* Window size */
                    if (i != 8) /* check message is correct size */
                      break;
+
                    cf->width =
                      clib_net_to_host_u16 (*((u16 *) (input_vector + 3)));
+                   if (cf->width > UNIX_CLI_MAX_TERMINAL_WIDTH)
+                     cf->width = UNIX_CLI_MAX_TERMINAL_WIDTH;
+                   if (cf->width < UNIX_CLI_MIN_TERMINAL_WIDTH)
+                     cf->width = UNIX_CLI_MIN_TERMINAL_WIDTH;
+
                    cf->height =
                      clib_net_to_host_u16 (*((u16 *) (input_vector + 5)));
+                   if (cf->height > UNIX_CLI_MAX_TERMINAL_HEIGHT)
+                     cf->height = UNIX_CLI_MAX_TERMINAL_HEIGHT;
+                   if (cf->height < UNIX_CLI_MIN_TERMINAL_HEIGHT)
+                     cf->height = UNIX_CLI_MIN_TERMINAL_HEIGHT;
+
                    /* reindex pager buffer */
                    unix_cli_pager_reindex (cf);
                    /* redraw page */
@@ -2539,8 +2559,18 @@ unix_cli_resize_interrupt (int signum)
       /* We can't trust ws.XXX... */
       return;
     }
+
   cf->width = ws.ws_col;
+  if (cf->width > UNIX_CLI_MAX_TERMINAL_WIDTH)
+    cf->width = UNIX_CLI_MAX_TERMINAL_WIDTH;
+  if (cf->width < UNIX_CLI_MIN_TERMINAL_WIDTH)
+    cf->width = UNIX_CLI_MIN_TERMINAL_WIDTH;
+
   cf->height = ws.ws_row;
+  if (cf->height > UNIX_CLI_MAX_TERMINAL_HEIGHT)
+    cf->height = UNIX_CLI_MAX_TERMINAL_HEIGHT;
+  if (cf->height < UNIX_CLI_MIN_TERMINAL_HEIGHT)
+    cf->height = UNIX_CLI_MIN_TERMINAL_HEIGHT;
 
   /* Reindex the pager buffer */
   unix_cli_pager_reindex (cf);

diff --git a/src/vnet/mpls/mpls_api.c b/src/vnet/mpls/mpls_api.c
index 988c2c9..762c40f 100644 (file)
--- a/src/vnet/mpls/mpls_api.c
+++ b/src/vnet/mpls/mpls_api.c
@@ -96,7 +96,7 @@ vl_api_mpls_table_add_del_t_handler (vl_api_mpls_table_add_del_t * mp)
   else
     mpls_table_delete (ntohl (mp->mt_table_id), 1);
 
-  rv = (rv == 0) ? vnm->api_errno : rv;
+  // NB: Nothing sets rv; none of the above returns an error
 
   REPLY_MACRO (VL_API_MPLS_TABLE_ADD_DEL_REPLY);
 }

diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c
index 4544f9a..792e661 100644 (file)
--- a/src/vnet/session/session.c
+++ b/src/vnet/session/session.c
@@ -267,10 +267,7 @@ stream_session_enqueue_data (transport_connection_t * tc, vlib_buffer_t * b,
        }
     }
 
-  if (is_in_order)
-    return enqueued;
-
-  return 0;
+  return enqueued;
 }
 
 /** Check if we have space in rx fifo to push more bytes */

diff --git a/src/vnet/session/session_cli.c b/src/vnet/session/session_cli.c
index d9f516b..8c30a1d 100755 (executable)
--- a/src/vnet/session/session_cli.c
+++ b/src/vnet/session/session_cli.c
@@ -127,13 +127,8 @@ unformat_stream_session_id (unformat_input_t * input, va_list * args)
       *is_ip4 = 0;
       tuple_is_set = 1;
     }
-  else
-    return 0;
-
-  if (tuple_is_set)
-    return 1;
 
-  return 0;
+  return tuple_is_set;
 }
 
 uword
@@ -144,21 +139,12 @@ unformat_stream_session (unformat_input_t * input, va_list * args)
   u8 proto = ~0;
   ip46_address_t lcl, rmt;
   u32 lcl_port = 0, rmt_port = 0;
-  u8 is_ip4 = 0, s_type = ~0, id_is_set = 0;
+  u8 is_ip4 = 0, s_type = ~0;
 
-  if (unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
-               &lcl_port, &rmt_port, &is_ip4))
-    {
-      id_is_set = 1;
-    }
-  else
+  if (!unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
+                &lcl_port, &rmt_port, &is_ip4))
     return 0;
 
-  if (!id_is_set)
-    {
-      return 0;
-    }
-
   s_type = session_type_from_proto_and_ip (proto, is_ip4);
   if (is_ip4)
     s = stream_session_lookup4 (&lcl.ip4, &rmt.ip4,
@@ -185,21 +171,12 @@ unformat_transport_connection (unformat_input_t * input, va_list * args)
   u8 proto = ~0;
   ip46_address_t lcl, rmt;
   u32 lcl_port = 0, rmt_port = 0;
-  u8 is_ip4 = 0, s_type = ~0, id_is_set = 0;
+  u8 is_ip4 = 0, s_type = ~0;
 
-  if (unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
-               &lcl_port, &rmt_port, &is_ip4))
-    {
-      id_is_set = 1;
-    }
-  else
+  if (!unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
+                &lcl_port, &rmt_port, &is_ip4))
     return 0;
 
-  if (!id_is_set)
-    {
-      return 0;
-    }
-
   proto = (proto == (u8) ~ 0) ? suggested_proto : proto;
   if (proto == (u8) ~ 0)
     return 0;

diff --git a/src/vnet/session/session_lookup.c b/src/vnet/session/session_lookup.c
index 0f9abf9..4487b1c 100644 (file)
--- a/src/vnet/session/session_lookup.c
+++ b/src/vnet/session/session_lookup.c
@@ -233,15 +233,15 @@ stream_session_half_open_table_add (transport_connection_t * tc, u64 value)
     {
       make_v4_ss_kv_from_tc (&kv4, tc);
       kv4.value = value;
-      clib_bihash_add_del_16_8 (&sl->v4_half_open_hash, &kv4,
-                               1 /* is_add */ );
+      (void) clib_bihash_add_del_16_8 (&sl->v4_half_open_hash, &kv4,
+                                      1 /* is_add */ );
     }
   else
     {
       make_v6_ss_kv_from_tc (&kv6, tc);
       kv6.value = value;
-      clib_bihash_add_del_48_8 (&sl->v6_half_open_hash, &kv6,
-                               1 /* is_add */ );
+      (void) clib_bihash_add_del_48_8 (&sl->v6_half_open_hash, &kv6,
+                                      1 /* is_add */ );
     }
 }
 

diff --git a/src/vppinfra/linux/mem.c b/src/vppinfra/linux/mem.c
index 665ddf6..df46763 100644 (file)
--- a/src/vppinfra/linux/mem.c
+++ b/src/vppinfra/linux/mem.c
@@ -49,7 +49,7 @@ int
 clib_mem_vm_get_log2_page_size (int fd)
 {
   struct stat st = { 0 };
-  if (fstat (fd, &st))
+  if (fstat (fd, &st) == -1)
     return 0;
   return min_log2 (st.st_blksize);
 }