Devices: Set interface rx-mode may cause SIGSEGV with nonexistent queue

When I type in set interface rx-mode with a nonexistent queue, I got a crash with the following
traceback. It looks like the vm is NULL when vlib_node_get_runtime is called.

DBGvpp# sh int rx
Thread 0 (vpp_main):
  node dpdk-input:
    TenGigabitEthernet5/0/0 queue 0 (polling)
    TenGigabitEthernet5/0/1 queue 0 (polling)
    TenGigabitEthernet7/0/0 queue 0 (polling)
    TenGigabitEthernet7/0/1 queue 0 (polling)
  node vhost-user-input:
    VirtualEthernet0/0/2 queue 0 (adaptive)
DBGvpp# set interface rx-mode VirtualEthernet0/0/2  queue 1 polling

Thread 1 "vpp_main" received signal SIGSEGV, Segmentation fault.
0x00007ffff6d4e0bc in vlib_node_get_runtime (vm=0x0, node_index=125)
    at /home/sluong/vpp/build-data/../src/vlib/node_funcs.h:92
92	  vlib_node_t *n = vec_elt (nm->nodes, node_index);
(gdb) where
    at /home/sluong/vpp/build-data/../src/vlib/node_funcs.h:92
    at /home/sluong/vpp/build-data/../src/vlib/node_funcs.h:112
    vnm=0x6f0fa0 <vnet_main>, hw_if_index=7, queue_id=1, mode=0x7fffb62099e8)
    at /home/sluong/vpp/build-data/../src/vnet/devices/devices.c:307
    hw_if_index=7, queue_id=1, mode=VNET_HW_INTERFACE_RX_MODE_POLLING)
    at /home/sluong/vpp/build-data/../src/vnet/interface_cli.c:1192
    vm=0x7ffff7b9d440 <vlib_global_main>, input=0x7fffb6209ef0,
    cmd=0x7fffb61d5d14)
    at /home/sluong/vpp/build-data/../src/vnet/interface_cli.c:1288
    vm=0x7ffff7b9d440 <vlib_global_main>,
    cm=0x7ffff7b9d630 <vlib_global_main+496>, input=0x7fffb6209ef0,
    parent_command_index=18)
    at /home/sluong/vpp/build-data/../src/vlib/cli.c:588
    vm=0x7ffff7b9d440 <vlib_global_main>,
    cm=0x7ffff7b9d630 <vlib_global_main+496>, input=0x7fffb6209ef0,
    parent_command_index=12)

The fix is to add a check for vec_len(hw->input_node_thread_index_by_queue)
and vec_len (hw->rx_mode_by_queue) to reject the command if the queue_id is
out of bound. While at it, I notice inputting queue_id=-1 is being interpreted
as all queues. An easy fix is to not overload the queue_id variable with -1 to
mean something else.

Change-Id: Id70ec3e7d06ccc67635e6d28ef53420bdac4a988
Signed-off-by: Steven <[email protected]>

diff --git a/src/vnet/api_errno.h b/src/vnet/api_errno.h
index 22522f3..c0deb1d 100644 (file)
--- a/src/vnet/api_errno.h
+++ b/src/vnet/api_errno.h
@@ -114,7 +114,8 @@ _(BD_NOT_MODIFIABLE, -121, "Bridge domain 0 can't be deleted/modified") \
 _(BD_ID_EXCEED_MAX, -122, "Bridge domain ID exceed 16M limit")         \
 _(SUBIF_DOESNT_EXIST, -123, "Subinterface doesn't exist")               \
 _(L2_MACS_EVENT_CLINET_PRESENT, -124, "Client already exist for L2 MACs events") \
-_(UNSUPPORTED, -125, "Unsupported")
+_(INVALID_QUEUE, -125, "Invalid queue")                                \
+_(UNSUPPORTED, -126, "Unsupported")
 
 typedef enum
 {

diff --git a/src/vnet/devices/devices.c b/src/vnet/devices/devices.c
index 2eb8e30..a38ecd2 100644 (file)
--- a/src/vnet/devices/devices.c
+++ b/src/vnet/devices/devices.c
@@ -264,6 +264,10 @@ vnet_hw_interface_set_rx_mode (vnet_main_t * vnm, u32 hw_if_index,
       (hw->flags & VNET_HW_INTERFACE_FLAG_SUPPORTS_INT_MODE) == 0)
     return VNET_API_ERROR_UNSUPPORTED;
 
+  if ((vec_len (hw->input_node_thread_index_by_queue) < queue_id + 1) ||
+      (vec_len (hw->rx_mode_by_queue) < queue_id + 1))
+    return VNET_API_ERROR_INVALID_QUEUE;
+
   hw->rx_mode_by_queue[queue_id] = mode;
   thread_index = hw->input_node_thread_index_by_queue[queue_id];
   vm = vlib_mains[thread_index];
@@ -307,6 +311,10 @@ vnet_hw_interface_get_rx_mode (vnet_main_t * vnm, u32 hw_if_index,
   if (hw->input_node_thread_index_by_queue == 0)
     return VNET_API_ERROR_INVALID_INTERFACE;
 
+  if ((vec_len (hw->input_node_thread_index_by_queue) < queue_id + 1) ||
+      (vec_len (hw->rx_mode_by_queue) < queue_id + 1))
+    return VNET_API_ERROR_INVALID_QUEUE;
+
   thread_index = hw->input_node_thread_index_by_queue[queue_id];
   vm = vlib_mains[thread_index];
 

diff --git a/src/vnet/interface_cli.c b/src/vnet/interface_cli.c
index f37f139..a6680c5 100644 (file)
--- a/src/vnet/interface_cli.c
+++ b/src/vnet/interface_cli.c
@@ -1313,6 +1313,8 @@ set_hw_interface_rx_mode (vnet_main_t * vnm, u32 hw_if_index,
       break;
     case VNET_API_ERROR_INVALID_INTERFACE:
       return clib_error_return (0, "invalid interface");
+    case VNET_API_ERROR_INVALID_QUEUE:
+      return clib_error_return (0, "invalid queue");
     default:
       return clib_error_return (0, "unknown error");
     }
@@ -1334,6 +1336,8 @@ set_hw_interface_rx_mode (vnet_main_t * vnm, u32 hw_if_index,
       return clib_error_return (0, "unsupported");
     case VNET_API_ERROR_INVALID_INTERFACE:
       return clib_error_return (0, "invalid interface");
+    case VNET_API_ERROR_INVALID_QUEUE:
+      return clib_error_return (0, "invalid queue");
     default:
       return clib_error_return (0, "unknown error");
     }
@@ -1353,6 +1357,7 @@ set_interface_rx_mode (vlib_main_t * vm, unformat_input_t * input,
   u32 queue_id = (u32) ~ 0;
   vnet_hw_interface_rx_mode mode = VNET_HW_INTERFACE_RX_MODE_UNKNOWN;
   int i;
+  u8 input_queue_id = 0;
 
   if (!unformat_user (input, unformat_line_input, line_input))
     return 0;
@@ -1363,7 +1368,7 @@ set_interface_rx_mode (vlib_main_t * vm, unformat_input_t * input,
          (line_input, "%U", unformat_vnet_hw_interface, vnm, &hw_if_index))
        ;
       else if (unformat (line_input, "queue %d", &queue_id))
-       ;
+       input_queue_id = 1;
       else if (unformat (line_input, "polling"))
        mode = VNET_HW_INTERFACE_RX_MODE_POLLING;
       else if (unformat (line_input, "interrupt"))
@@ -1389,7 +1394,7 @@ set_interface_rx_mode (vlib_main_t * vm, unformat_input_t * input,
 
   hw = vnet_get_hw_interface (vnm, hw_if_index);
 
-  if (queue_id == ~0)
+  if (input_queue_id == 0)
     {
       for (i = 0; i < vec_len (hw->dq_runtime_index_by_queue); i++)
        {