vhost: crash under heavy traffic condition due to memory corruption (VPP-1016) 92/7092/21
authorSteven <sluong@cisco.com>
Sat, 10 Jun 2017 01:49:17 +0000 (18:49 -0700)
committerDamjan Marion <dmarion.lists@gmail.com>
Sat, 14 Oct 2017 09:11:19 +0000 (09:11 +0000)
commitd77275307b6ad6459ecba01912a302fb7dbf0f02
tree97404007d5458d887d57ef8403bb1ea2d3172186
parent781b99ddc9565a6c8bbe078ecb3258d91a2bbc06
vhost: crash under heavy traffic condition due to memory corruption (VPP-1016)

With heavy traffic, tx code path may crash due to memory corruption

Thread 5 "vpp_wk_2" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff3995c700 (LWP 2505)]
0x00007ffff73675e8 in vhost_user_if_input (vm=0x7fffb5f5bf9c,
    vum=0x7ffff7882a40 <vhost_user_main>, vui=0x7fffb65570c4, qid=0,
    node=0x7fffb6577dac, mode=VNET_HW_INTERFACE_RX_MODE_POLLING)
    at /home/sluong/vpp-master/vpp/build-data/../src/vnet/devices/virtio/vhost-user.c:1610
1610   bi_current = (vum->cpus[thread_index].rx_buffers)
                       [vum->cpus[thread_index].rx_buffers_len];
(gdb) p vum->cpus[thread_index].rx_buffers_len
$2 = 793212607
(gdb)

Apparently, some code accidentally wrote the bad value in rx_buffers_len.
rx_buffers_len should never be greater than 1024 since that is how many buffers
we request each time.

After debugging many hours, I discovered that the memory corruption happens
in the tx code path right here on line 2176.

  {
    vhost_copy_t *cpy = &vum->cpus[thread_index].copy[copy_len];
    copy_len++;
    cpy->len = bytes_left;
    cpy->len = (cpy->len > buffer_len) ? buffer_len : cpy->len;
    cpy->dst = buffer_map_addr;
    cpy->src = (uword) vlib_buffer_get_current (current_b0) +
      current_b0->current_length - bytes_left;

(gdb) p cpy
$3 = (vhost_copy_t *) 0x7fffb554077c
(gdb) p copy_len
$4 = 1025
(gdb) p &vum->cpus[3].rx_buffers_len
$8 = (u32 *) 0x7fffb5540784

copy_len is picking up the index entry 1024 before it was incremented. copy array has only
1024 members (0 - 1023 are valid).
The assignment here in cpy surely causes memory corruption. It is only discovered later
when the memory location that it corrupted is used.

The condition for the crash is to transmit jumbo frames under heavy volume. Since ring
size is 1024, with one packet taking up one index for frame size (less 2048), it does
not cause overflow. With jumbo frames, it requires multiple indices for one packet,
it can cause the overflow under heavy traffic.

The fix is to do copy out when we have 1000 entries in the array to avoid
overflow.

Change-Id: Iefbc739b8e80470f1cf13123113f8331ffcd0eb2
Signed-off-by: Steven <sluong@cisco.com>
src/vnet/devices/virtio/vhost-user.c