aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSteven <sluong@cisco.com>2017-06-09 18:49:17 -0700
committerNeale Ranns <nranns@cisco.com>2017-10-10 19:09:12 +0000
commitaa5df48cb233b377b5910694e2440a16e5973864 (patch)
treeb773e44e27b1a60397f2d610f2f5af8f099f1cb6
parentda1eadcf4e6f76562cc87f317fb96a02d3e54eef (diff)
vhost: crash under heavy traffic condition due to memory corruption
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>
-rw-r--r--src/vnet/devices/virtio/vhost-user.c35
1 files changed, 33 insertions, 2 deletions
diff --git a/src/vnet/devices/virtio/vhost-user.c b/src/vnet/devices/virtio/vhost-user.c
index 82f7653356d..4db5a23398d 100644
--- a/src/vnet/devices/virtio/vhost-user.c
+++ b/src/vnet/devices/virtio/vhost-user.c
@@ -86,6 +86,16 @@
* The value 64 was obtained by testing (48 and 128 were not as good).
*/
#define VHOST_USER_RX_COPY_THRESHOLD 64
+/*
+ * On the transmit side, we keep processing the buffers from vlib in the while
+ * loop and prepare the copy order to be executed later. However, the static
+ * array which we keep the copy order is limited to VHOST_USER_COPY_ARRAY_N
+ * entries. In order to not corrupt memory, we have to do the copy when the
+ * static array reaches the copy threshold. We subtract 40 in case the code
+ * goes into the inner loop for a maximum of 64k frames which may require
+ * more array entries.
+ */
+#define VHOST_USER_TX_COPY_THRESHOLD (VHOST_USER_COPY_ARRAY_N - 40)
#define UNIX_GET_FD(unixfd_idx) \
(unixfd_idx != ~0) ? \
@@ -2001,7 +2011,7 @@ vhost_user_tx (vlib_main_t * vm,
qid =
VHOST_VRING_IDX_RX (*vec_elt_at_index
- (vui->per_cpu_tx_qid, vlib_get_thread_index ()));
+ (vui->per_cpu_tx_qid, thread_index));
rxvq = &vui->vrings[qid];
if (PREDICT_FALSE (vui->use_tx_spinlock))
vhost_user_vring_lock (vui, qid);
@@ -2215,6 +2225,27 @@ retry:
}
n_left--; //At the end for error counting when 'goto done' is invoked
+
+ /*
+ * Do the copy periodically to prevent
+ * vum->cpus[thread_index].copy array overflow and corrupt memory
+ */
+ if (PREDICT_FALSE (copy_len >= VHOST_USER_TX_COPY_THRESHOLD))
+ {
+ if (PREDICT_FALSE
+ (vhost_user_tx_copy (vui, vum->cpus[thread_index].copy,
+ copy_len, &map_hint)))
+ {
+ vlib_error_count (vm, node->node_index,
+ VHOST_USER_TX_FUNC_ERROR_MMAP_FAIL, 1);
+ }
+ copy_len = 0;
+
+ /* give buffers back to driver */
+ CLIB_MEMORY_BARRIER ();
+ rxvq->used->idx = rxvq->last_used_idx;
+ vhost_user_log_dirty_ring (vui, rxvq, idx);
+ }
buffers++;
}
@@ -2269,7 +2300,7 @@ done3:
vlib_increment_simple_counter
(vnet_main.interface_main.sw_if_counters
+ VNET_INTERFACE_COUNTER_DROP,
- vlib_get_thread_index (), vui->sw_if_index, n_left);
+ thread_index, vui->sw_if_index, n_left);
}
vlib_buffer_free (vm, vlib_frame_args (frame), frame->n_vectors);