memif: fix coverity warnings as of 9/7

1. coverity complains about "buffer not null terminated" for strncpy because we
pass the size of the destination to the call which is equal to the true size
of the destination. We subtract 1 for the size to accommodate the null like all
other places are already doing it.

2. Add a check to tx_queues in memif_interface_tx_inline to avoid "divide by zero".

3. To avoid null pointer dereference in memif_create_if, change the goto done
rather than goto error and spit a more meaningful error rather than silent about it.

4. Shuffle a line to avoid "check after use" in vl_api_memif_delete_t_handler.

Change-Id: Icba7ecd5362c012a48ac35795d31aab356617420
Signed-off-by: Steven <sluong@cisco.com>

4 files changed, 29 insertions, 12 deletions

diff --git a/src/plugins/memif/device.c b/src/plugins/memif/device.c
index c7682534..aff18f2d 100644
--- a/src/plugins/memif/device.c
+++ b/src/plugins/memif/device.c
@@ -31,7 +31,8 @@
 #define foreach_memif_tx_func_error	       \
 _(NO_FREE_SLOTS, "no free tx slots")           \
 _(TRUNC_PACKET, "packet > buffer size -- truncated in tx ring") \
-_(PENDING_MSGS, "pending msgs in tx ring")
+_(PENDING_MSGS, "pending msgs in tx ring") \
+_(NO_TX_QUEUES, "no tx queues")
 
 typedef enum
 {
@@ -169,6 +170,13 @@ memif_interface_tx_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
   u8 tx_queues = vec_len (mif->tx_queues);
   memif_queue_t *mq;
 
+  if (PREDICT_FALSE (tx_queues == 0))
+    {
+      vlib_error_count (vm, node->node_index, MEMIF_TX_ERROR_NO_TX_QUEUES,
+			n_left);
+      goto error;
+    }
+
   if (tx_queues < vec_len (vlib_mains))
     {
       qid = thread_index % tx_queues;
@@ -249,7 +257,6 @@ memif_interface_tx_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 			n_left);
     }
 
-  vlib_buffer_free (vm, vlib_frame_args (frame), frame->n_vectors);
   if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0 && mq->int_fd > -1)
     {
       u64 b = 1;
@@ -257,6 +264,9 @@ memif_interface_tx_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
       mq->int_count++;
     }
 
+error:
+  vlib_buffer_free (vm, vlib_frame_args (frame), frame->n_vectors);
+
   return frame->n_vectors;
 }
 
diff --git a/src/plugins/memif/memif.c b/src/plugins/memif/memif.c
index af81faf2..7e2d947f 100644
--- a/src/plugins/memif/memif.c
+++ b/src/plugins/memif/memif.c
@@ -611,8 +611,11 @@ memif_create_if (vlib_main_t * vm, memif_create_if_args_t * args)
 	    }
 	  else
 	    {
-	      ret = VNET_API_ERROR_SYSCALL_ERROR_3;
-	      goto error;
+	      error = clib_error_return (0, "File exists for %s",
+					 socket_filename);
+	      clib_error_report (error);
+	      rv = VNET_API_ERROR_VALUE_EXIST;
+	      goto done;
 	    }
 	}
       pool_get (mm->socket_files, msf);
diff --git a/src/plugins/memif/memif_api.c b/src/plugins/memif/memif_api.c
index 2b7b792a..89afdb8d 100644
--- a/src/plugins/memif/memif_api.c
+++ b/src/plugins/memif/memif_api.c
@@ -201,13 +201,16 @@ vl_api_memif_delete_t_handler (vl_api_memif_delete_t * mp)
   vl_api_memif_delete_reply_t *rmp;
   vnet_hw_interface_t *hi =
     vnet_get_sup_hw_interface (vnm, ntohl (mp->sw_if_index));
-  memif_if_t *mif = pool_elt_at_index (mm->interfaces, hi->dev_instance);
+  memif_if_t *mif;
   int rv = 0;
 
   if (hi == NULL || memif_device_class.index != hi->dev_class_index)
     rv = VNET_API_ERROR_INVALID_SW_IF_INDEX;
   else
-    rv = memif_delete_if (vm, mif);
+    {
+      mif = pool_elt_at_index (mm->interfaces, hi->dev_instance);
+      rv = memif_delete_if (vm, mif);
+    }
 
   REPLY_MACRO (VL_API_MEMIF_DELETE_REPLY);
 }
diff --git a/src/plugins/memif/socket.c b/src/plugins/memif/socket.c
index a3c8d544..79ae07be 100644
--- a/src/plugins/memif/socket.c
+++ b/src/plugins/memif/socket.c
@@ -115,7 +115,7 @@ memif_msg_enq_hello (int fd)
   h->max_region = MEMIF_MAX_REGION;
   h->max_log2_ring_size = MEMIF_MAX_LOG2_RING_SIZE;
   s = format (0, "VPP %s%c", VPP_BUILD_VER, 0);
-  strncpy ((char *) h->name, (char *) s, sizeof (h->name));
+  strncpy ((char *) h->name, (char *) s, sizeof (h->name) - 1);
   vec_free (s);
   return memif_msg_send (fd, &msg, -1);
 }
@@ -134,9 +134,10 @@ memif_msg_enq_init (memif_if_t * mif)
   i->id = mif->id;
   i->mode = mif->mode;
   s = format (0, "VPP %s%c", VPP_BUILD_VER, 0);
-  strncpy ((char *) i->name, (char *) s, sizeof (i->name));
+  strncpy ((char *) i->name, (char *) s, sizeof (i->name) - 1);
   if (mif->secret)
-    strncpy ((char *) i->secret, (char *) mif->secret, sizeof (i->secret));
+    strncpy ((char *) i->secret, (char *) mif->secret,
+	     sizeof (i->secret) - 1);
   vec_free (s);
 }
 
@@ -189,7 +190,7 @@ memif_msg_enq_connect (memif_if_t * mif)
   e->msg.type = MEMIF_MSG_TYPE_CONNECT;
   e->fd = -1;
   s = format (0, "%U%c", format_memif_device_name, mif->dev_instance, 0);
-  strncpy ((char *) c->if_name, (char *) s, sizeof (c->if_name));
+  strncpy ((char *) c->if_name, (char *) s, sizeof (c->if_name) - 1);
   vec_free (s);
 }
 
@@ -204,7 +205,7 @@ memif_msg_enq_connected (memif_if_t * mif)
   e->msg.type = MEMIF_MSG_TYPE_CONNECTED;
   e->fd = -1;
   s = format (0, "%U%c", format_memif_device_name, mif->dev_instance, 0);
-  strncpy ((char *) c->if_name, (char *) s, sizeof (c->if_name));
+  strncpy ((char *) c->if_name, (char *) s, sizeof (c->if_name) - 1);
   vec_free (s);
 }
 
@@ -216,7 +217,7 @@ memif_msg_send_disconnect (memif_if_t * mif, clib_error_t * err)
   memif_msg_disconnect_t *d = &msg.disconnect;
 
   d->code = err->code;
-  strncpy ((char *) d->string, (char *) err->what, sizeof (d->string));
+  strncpy ((char *) d->string, (char *) err->what, sizeof (d->string) - 1);
 
   return memif_msg_send (mif->conn_fd, &msg, -1);
 }