From b5e55a27a46f166f466c7996675542de645eff66 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Thu, 10 Jan 2019 12:42:47 -0800 Subject: session: generate wrong thread errors instead of crashing Change-Id: I7e59ae718d2722c49d42b22a0874e1645a191e89 Signed-off-by: Florin Coras --- src/plugins/unittest/session_test.c | 3 ++- src/vnet/sctp/sctp_error.def | 3 ++- src/vnet/sctp/sctp_input.c | 10 +++++----- src/vnet/session/session_lookup.c | 34 ++++++++++++++++++++++++---------- src/vnet/session/session_lookup.h | 7 +++++++ src/vnet/tcp/tcp_error.def | 3 ++- src/vnet/tcp/tcp_input.c | 10 +++++----- 7 files changed, 47 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/plugins/unittest/session_test.c b/src/plugins/unittest/session_test.c index c504d8df15d..cc273321d32 100644 --- a/src/plugins/unittest/session_test.c +++ b/src/plugins/unittest/session_test.c @@ -1194,7 +1194,8 @@ session_test_rules (vlib_main_t * vm, unformat_input_t * input) &is_filtered); SESSION_TEST ((tc == 0), "lookup for 1.2.3.4/32 1234 5.6.7.8/16 4321 " "should fail (deny rule)"); - SESSION_TEST ((is_filtered == 1), "lookup should be filtered (deny)"); + SESSION_TEST ((is_filtered == SESSION_LOOKUP_RESULT_FILTERED), + "lookup should be filtered (deny)"); handle = session_lookup_local_endpoint (local_ns_index, &sep); SESSION_TEST ((handle == SESSION_DROP_HANDLE), "lookup for 1.2.3.4/32 1234 " diff --git a/src/vnet/sctp/sctp_error.def b/src/vnet/sctp/sctp_error.def index 7326301e862..b95b71a9417 100644 --- a/src/vnet/sctp/sctp_error.def +++ b/src/vnet/sctp/sctp_error.def @@ -13,6 +13,8 @@ * limitations under the License. */ sctp_error (NONE, "no error") +sctp_error (WRONG_WORKER, "Wrong worker thread") +sctp_error (FILTERED, "Packets filtered") sctp_error (PKTS_SENT, "Packets sent") sctp_error (INVALID_CONNECTION, "Invalid connection") sctp_error (INVALID_TAG, "Invalid verification tag") @@ -47,5 +49,4 @@ sctp_error (EVENT_FIFO_FULL, "Events not sent for lack of event fifo space") sctp_error (UNKNOWN_CHUNK, "Unrecognized / unknown chunk or chunk-state mismatch") sctp_error (BUNDLING_VIOLATION, "Bundling not allowed") sctp_error (PUNT, "Packets punted") -sctp_error (FILTERED, "Packets filtered") sctp_error (MAX_CONNECTIONS, "Reached max supported subconnection") diff --git a/src/vnet/sctp/sctp_input.c b/src/vnet/sctp/sctp_input.c index 4454f99e6bb..88e4eab754d 100644 --- a/src/vnet/sctp/sctp_input.c +++ b/src/vnet/sctp/sctp_input.c @@ -2114,7 +2114,7 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node, { u32 n_left_from, next_index, *from, *to_next; u32 my_thread_index = vm->thread_index; - u8 is_filtered; + u8 result; sctp_main_t *tm = vnet_get_sctp_main (); from = vlib_frame_vector_args (from_frame); @@ -2175,7 +2175,7 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node, sctp_hdr->src_port, TRANSPORT_PROTO_SCTP, my_thread_index, - &is_filtered); + &result); } else { @@ -2198,7 +2198,7 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node, sctp_hdr->src_port, TRANSPORT_PROTO_SCTP, my_thread_index, - &is_filtered); + &result); } /* Length check */ @@ -2256,10 +2256,10 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node, } else { - if (is_filtered) + if (result) { next0 = SCTP_INPUT_NEXT_DROP; - error0 = SCTP_ERROR_FILTERED; + error0 = SCTP_ERROR_NONE + result; } else if ((is_ip4 && tm->punt_unknown4) || (!is_ip4 && tm->punt_unknown6)) diff --git a/src/vnet/session/session_lookup.c b/src/vnet/session/session_lookup.c index c410dab121f..931c3d0f9e2 100644 --- a/src/vnet/session/session_lookup.c +++ b/src/vnet/session/session_lookup.c @@ -852,7 +852,7 @@ transport_connection_t * session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl, ip4_address_t * rmt, u16 lcl_port, u16 rmt_port, u8 proto, u32 thread_index, - u8 * is_filtered) + u8 * result) { session_table_t *st; session_kv4_t kv4; @@ -871,7 +871,11 @@ session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl, rv = clib_bihash_search_inline_16_8 (&st->v4_session_hash, &kv4); if (rv == 0) { - ASSERT ((u32) (kv4.value >> 32) == thread_index); + if (PREDICT_FALSE ((u32) (kv4.value >> 32) != thread_index)) + { + *result = SESSION_LOOKUP_RESULT_WRONG_THREAD; + return 0; + } s = session_get (kv4.value & 0xFFFFFFFFULL, thread_index); return tp_vfts[proto].get_connection (s->connection_index, thread_index); @@ -891,8 +895,11 @@ session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl, rmt, lcl_port, rmt_port); if (session_lookup_action_index_is_valid (action_index)) { - if ((*is_filtered = (action_index == SESSION_RULES_TABLE_ACTION_DROP))) - return 0; + if (action_index == SESSION_RULES_TABLE_ACTION_DROP) + { + *result = SESSION_LOOKUP_RESULT_FILTERED; + return 0; + } if ((s = session_lookup_action_to_session (action_index, FIB_PROTOCOL_IP4, proto))) return tp_vfts[proto].get_listener (s->connection_index); @@ -912,9 +919,8 @@ session_lookup_connection_wt4 (u32 fib_index, ip4_address_t * lcl, /** * Lookup connection with ip4 and transport layer information * - * Not optimized. This is used on the fast path so it needs to be fast. - * Thereby, duplication of code and 'hacks' allowed. Lookup logic is identical - * to that of @ref session_lookup_connection_wt4 + * Not optimized. Lookup logic is identical to that of + * @ref session_lookup_connection_wt4 * * @param fib_index index of the fib wherein the connection was received * @param lcl local ip4 address @@ -1070,7 +1076,7 @@ transport_connection_t * session_lookup_connection_wt6 (u32 fib_index, ip6_address_t * lcl, ip6_address_t * rmt, u16 lcl_port, u16 rmt_port, u8 proto, u32 thread_index, - u8 * is_filtered) + u8 * result) { session_table_t *st; stream_session_t *s; @@ -1087,6 +1093,11 @@ session_lookup_connection_wt6 (u32 fib_index, ip6_address_t * lcl, if (rv == 0) { ASSERT ((u32) (kv6.value >> 32) == thread_index); + if (PREDICT_FALSE ((u32) (kv6.value >> 32) != thread_index)) + { + *result = SESSION_LOOKUP_RESULT_WRONG_THREAD; + return 0; + } s = session_get (kv6.value & 0xFFFFFFFFULL, thread_index); return tp_vfts[proto].get_connection (s->connection_index, thread_index); @@ -1102,8 +1113,11 @@ session_lookup_connection_wt6 (u32 fib_index, ip6_address_t * lcl, rmt, lcl_port, rmt_port); if (session_lookup_action_index_is_valid (action_index)) { - if ((*is_filtered = (action_index == SESSION_RULES_TABLE_ACTION_DROP))) - return 0; + if (action_index == SESSION_RULES_TABLE_ACTION_DROP) + { + *result = SESSION_LOOKUP_RESULT_FILTERED; + return 0; + } if ((s = session_lookup_action_to_session (action_index, FIB_PROTOCOL_IP6, proto))) return tp_vfts[proto].get_listener (s->connection_index); diff --git a/src/vnet/session/session_lookup.h b/src/vnet/session/session_lookup.h index 1acf923831d..212a11833a3 100644 --- a/src/vnet/session/session_lookup.h +++ b/src/vnet/session/session_lookup.h @@ -21,6 +21,13 @@ #include #include +typedef enum session_lookup_result_ +{ + SESSION_LOOKUP_RESULT_NONE, + SESSION_LOOKUP_RESULT_WRONG_THREAD, + SESSION_LOOKUP_RESULT_FILTERED +} session_lookup_result_t; + stream_session_t *session_lookup_safe4 (u32 fib_index, ip4_address_t * lcl, ip4_address_t * rmt, u16 lcl_port, u16 rmt_port, u8 proto); diff --git a/src/vnet/tcp/tcp_error.def b/src/vnet/tcp/tcp_error.def index 141ca515995..0d6c44b5402 100644 --- a/src/vnet/tcp/tcp_error.def +++ b/src/vnet/tcp/tcp_error.def @@ -13,6 +13,8 @@ * limitations under the License. */ tcp_error (NONE, "no error") +tcp_error (WRONG_THREAD, "Wrong worker thread") +tcp_error (FILTERED, "Packets filtered") tcp_error (LENGTH, "inconsistent ip/tcp lengths") tcp_error (NO_LISTENER, "no listener for dst port") tcp_error (LOOKUP_DROPS, "lookup drops") @@ -40,7 +42,6 @@ tcp_error (INVALID_CONNECTION, "Invalid connection") tcp_error (CONNECTION_CLOSED, "Connection closed") tcp_error (CREATE_EXISTS, "Connection already exists") tcp_error (PUNT, "Packets punted") -tcp_error (FILTERED, "Packets filtered") tcp_error (OPTIONS, "Could not parse options") tcp_error (PAWS, "PAWS check failed") tcp_error (RCV_WND, "Segment not in receive window") diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c index 4358bd30d88..82ab817430f 100644 --- a/src/vnet/tcp/tcp_input.c +++ b/src/vnet/tcp/tcp_input.c @@ -3295,7 +3295,7 @@ tcp_input_trace_frame (vlib_main_t * vm, vlib_node_runtime_t * node, static void tcp_input_set_error_next (tcp_main_t * tm, u16 * next, u32 * error, u8 is_ip4) { - if (*error == TCP_ERROR_FILTERED) + if (*error == TCP_ERROR_FILTERED || *error == TCP_ERROR_WRONG_THREAD) { *next = TCP_INPUT_NEXT_DROP; } @@ -3319,7 +3319,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error, int n_advance_bytes, n_data_bytes; transport_connection_t *tc; tcp_header_t *tcp; - u8 is_filtered = 0; + u8 result = 0; if (is_ip4) { @@ -3345,7 +3345,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error, tc = session_lookup_connection_wt4 (fib_index, &ip4->dst_address, &ip4->src_address, tcp->dst_port, tcp->src_port, TRANSPORT_PROTO_TCP, - thread_index, &is_filtered); + thread_index, &result); } else { @@ -3371,7 +3371,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error, tc = session_lookup_connection_wt6 (fib_index, &ip6->dst_address, &ip6->src_address, tcp->dst_port, tcp->src_port, TRANSPORT_PROTO_TCP, - thread_index, &is_filtered); + thread_index, &result); } vnet_buffer (b)->tcp.seq_number = clib_net_to_host_u32 (tcp->seq_number); @@ -3382,7 +3382,7 @@ tcp_input_lookup_buffer (vlib_buffer_t * b, u8 thread_index, u32 * error, + n_data_bytes; vnet_buffer (b)->tcp.flags = 0; - *error = is_filtered ? TCP_ERROR_FILTERED : *error; + *error = result ? TCP_ERROR_NONE + result : *error; return tcp_get_connection_from_transport (tc); } -- cgit 1.2.3-korg