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>

diff --git a/src/plugins/memif/device.c b/src/plugins/memif/device.c
index c768253..aff18f2 100644 (file)
--- 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 af81faf..7e2d947 100644 (file)
--- 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 2b7b792..89afdb8 100644 (file)
--- 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 a3c8d54..79ae07b 100644 (file)
--- 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);
 }