From ef91534e665cf343af2389df11d46559a1f83d78 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Sat, 29 Sep 2018 10:23:06 -0700 Subject: [PATCH] tls: fix disconnects for sessions with pending data TLS can enqueue events to itself when app session queue cannot be entirely drained. If a pending disconnect is handled before any such event, session layer may try to dequeue data on deallocated sessions. Change-Id: I5bfc4d53ce95bc16b6a01e1b0e644aafa1ca311b Signed-off-by: Florin Coras --- src/plugins/tlsmbedtls/tls_mbedtls.c | 16 ++++----- src/plugins/tlsopenssl/tls_openssl.c | 24 ++++++------- src/vnet/session-apps/echo_client.c | 1 + src/vnet/session/session.c | 5 +-- src/vnet/session/session.h | 3 +- src/vnet/session/session_node.c | 9 ++++- src/vnet/tls/tls.c | 67 +++++++++++++++++++++++++++++------- src/vnet/tls/tls.h | 5 ++- 8 files changed, 93 insertions(+), 37 deletions(-) diff --git a/src/plugins/tlsmbedtls/tls_mbedtls.c b/src/plugins/tlsmbedtls/tls_mbedtls.c index 57a6e486c3f..1010e370880 100644 --- a/src/plugins/tlsmbedtls/tls_mbedtls.c +++ b/src/plugins/tlsmbedtls/tls_mbedtls.c @@ -169,7 +169,7 @@ tls_net_send (void *ctx_indexp, const unsigned char *buf, size_t len) rv = svm_fifo_enqueue_nowait (tls_session->server_tx_fifo, len, buf); if (rv < 0) return MBEDTLS_ERR_SSL_WANT_WRITE; - tls_add_vpp_q_evt (tls_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_tx_evt (tls_session); return rv; } @@ -448,7 +448,7 @@ mbedtls_ctx_write (tls_ctx_t * ctx, stream_session_t * app_session) if (PREDICT_FALSE (enq_max == 0)) { - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); return 0; } @@ -459,16 +459,16 @@ mbedtls_ctx_write (tls_ctx_t * ctx, stream_session_t * app_session) wrote = mbedtls_ssl_write (&mc->ssl, mm->tx_bufs[thread_index], deq_now); if (wrote <= 0) { - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); return 0; } svm_fifo_dequeue_drop (app_session->server_tx_fifo, wrote); vec_reset_length (mm->tx_bufs[thread_index]); - tls_add_vpp_q_evt (tls_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_tx_evt (tls_session); if (deq_now < deq_max) - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); return 0; } @@ -499,7 +499,7 @@ mbedtls_ctx_read (tls_ctx_t * ctx, stream_session_t * tls_session) if (PREDICT_FALSE (enq_now == 0)) { - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); return 0; } @@ -507,7 +507,7 @@ mbedtls_ctx_read (tls_ctx_t * ctx, stream_session_t * tls_session) read = mbedtls_ssl_read (&mc->ssl, mm->rx_bufs[thread_index], enq_now); if (read <= 0) { - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); return 0; } @@ -517,7 +517,7 @@ mbedtls_ctx_read (tls_ctx_t * ctx, stream_session_t * tls_session) vec_reset_length (mm->rx_bufs[thread_index]); if (svm_fifo_max_dequeue (tls_session->server_rx_fifo)) - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); if (enq > 0) tls_notify_app_enqueue (ctx, app_session); diff --git a/src/plugins/tlsopenssl/tls_openssl.c b/src/plugins/tlsopenssl/tls_openssl.c index 744a07a254e..b8757a9ffb7 100644 --- a/src/plugins/tlsopenssl/tls_openssl.c +++ b/src/plugins/tlsopenssl/tls_openssl.c @@ -158,7 +158,7 @@ openssl_try_handshake_write (openssl_ctx_t * oc, return 0; svm_fifo_enqueue_nocopy (f, read); - tls_add_vpp_q_evt (f, FIFO_EVENT_APP_TX); + tls_add_vpp_q_tx_evt (tls_session); if (read < enq_max) { @@ -318,7 +318,7 @@ openssl_ctx_write (tls_ctx_t * ctx, stream_session_t * app_session) wrote = SSL_write (oc->ssl, svm_fifo_head (f), to_write); if (wrote <= 0) { - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); goto check_tls_fifo; } svm_fifo_dequeue_drop (app_session->server_tx_fifo, wrote); @@ -334,7 +334,7 @@ openssl_ctx_write (tls_ctx_t * ctx, stream_session_t * app_session) } if (wrote < deq_max) - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); check_tls_fifo: @@ -346,7 +346,7 @@ check_tls_fifo: enq_max = svm_fifo_max_enqueue (f); if (!enq_max) { - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); return wrote; } @@ -354,12 +354,12 @@ check_tls_fifo: read = BIO_read (oc->rbio, svm_fifo_tail (f), deq_now); if (read <= 0) { - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); return wrote; } svm_fifo_enqueue_nocopy (f, read); - tls_add_vpp_q_evt (f, FIFO_EVENT_APP_TX); + tls_add_vpp_q_tx_evt (tls_session); if (read < enq_max && BIO_ctrl_pending (oc->rbio) > 0) { @@ -370,7 +370,7 @@ check_tls_fifo: } if (BIO_ctrl_pending (oc->rbio) > 0) - tls_add_vpp_q_evt (app_session->server_tx_fifo, FIFO_EVENT_APP_TX); + tls_add_vpp_q_builtin_tx_evt (app_session); return wrote; } @@ -402,7 +402,7 @@ openssl_ctx_read (tls_ctx_t * ctx, stream_session_t * tls_session) wrote = BIO_write (oc->wbio, svm_fifo_head (f), to_read); if (wrote <= 0) { - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); goto check_app_fifo; } svm_fifo_dequeue_drop (f, wrote); @@ -417,7 +417,7 @@ openssl_ctx_read (tls_ctx_t * ctx, stream_session_t * tls_session) } } if (svm_fifo_max_dequeue (f)) - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); check_app_fifo: @@ -429,7 +429,7 @@ check_app_fifo: enq_max = svm_fifo_max_enqueue (f); if (!enq_max) { - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); return wrote; } @@ -437,7 +437,7 @@ check_app_fifo: read = SSL_read (oc->ssl, svm_fifo_tail (f), deq_now); if (read <= 0) { - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); return wrote; } svm_fifo_enqueue_nocopy (f, read); @@ -451,7 +451,7 @@ check_app_fifo: tls_notify_app_enqueue (ctx, app_session); if (BIO_ctrl_pending (oc->wbio) > 0) - tls_add_vpp_q_evt (tls_session->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + tls_add_vpp_q_builtin_rx_evt (tls_session); return wrote; } diff --git a/src/vnet/session-apps/echo_client.c b/src/vnet/session-apps/echo_client.c index e67de959a28..b47dcf21a4b 100644 --- a/src/vnet/session-apps/echo_client.c +++ b/src/vnet/session-apps/echo_client.c @@ -826,6 +826,7 @@ echo_clients_command_fn (vlib_main_t * vm, cleanup: ecm->run_test = 0; + vlib_process_wait_for_event_or_clock (vm, 10e-3); for (i = 0; i < vec_len (ecm->connection_index_by_thread); i++) { vec_reset_length (ecm->connection_index_by_thread[i]); diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c index 3eaf9846709..372a6f93a95 100644 --- a/src/vnet/session/session.c +++ b/src/vnet/session/session.c @@ -69,6 +69,7 @@ session_send_evt_to_thread (void *data, void *args, u32 thread_index, case FIFO_EVENT_BUILTIN_RX: evt->fifo = data; break; + case FIFO_EVENT_BUILTIN_TX: case FIFO_EVENT_DISCONNECT: evt->session_handle = session_handle ((stream_session_t *) data); break; @@ -89,10 +90,10 @@ session_send_io_evt_to_thread (svm_fifo_t * f, session_evt_type_t evt_type) } int -session_send_io_evt_to_thread_custom (svm_fifo_t * f, u32 thread_index, +session_send_io_evt_to_thread_custom (void *data, u32 thread_index, session_evt_type_t evt_type) { - return session_send_evt_to_thread (f, 0, thread_index, evt_type); + return session_send_evt_to_thread (data, 0, thread_index, evt_type); } int diff --git a/src/vnet/session/session.h b/src/vnet/session/session.h index aa827556d45..1e08cccb6f7 100644 --- a/src/vnet/session/session.h +++ b/src/vnet/session/session.h @@ -38,6 +38,7 @@ typedef enum SESSION_IO_EVT_CT_TX, FIFO_EVENT_DISCONNECT, FIFO_EVENT_BUILTIN_RX, + FIFO_EVENT_BUILTIN_TX, FIFO_EVENT_RPC, SESSION_CTRL_EVT_BOUND, SESSION_CTRL_EVT_ACCEPTED, @@ -557,7 +558,7 @@ void stream_session_disconnect_transport (stream_session_t * s); void stream_session_cleanup (stream_session_t * s); int session_send_io_evt_to_thread (svm_fifo_t * f, session_evt_type_t evt_type); -int session_send_io_evt_to_thread_custom (svm_fifo_t * f, u32 thread_index, +int session_send_io_evt_to_thread_custom (void *data, u32 thread_index, session_evt_type_t evt_type); void session_send_rpc_evt_to_thread (u32 thread_index, void *fp, void *rpc_args); diff --git a/src/vnet/session/session_node.c b/src/vnet/session/session_node.c index e2a6f4c67c6..9a954bb127b 100644 --- a/src/vnet/session/session_node.c +++ b/src/vnet/session/session_node.c @@ -730,6 +730,8 @@ session_tx_fifo_dequeue_internal (vlib_main_t * vm, stream_session_t * s, int *n_tx_pkts) { application_t *app; + if (PREDICT_FALSE (s->session_state == SESSION_STATE_CLOSED)) + return 0; app = application_get (s->t_app_index); svm_fifo_unset_event (s->server_tx_fifo); return app->cb_fns.builtin_app_tx_callback (s); @@ -878,13 +880,18 @@ skip_dequeue: break; case FIFO_EVENT_BUILTIN_RX: s = session_event_get_session (e, thread_index); - if (PREDICT_FALSE (!s)) + if (PREDICT_FALSE (!s || s->session_state >= SESSION_STATE_CLOSING)) continue; svm_fifo_unset_event (s->server_rx_fifo); app_wrk = app_worker_get (s->app_wrk_index); app = application_get (app_wrk->app_index); app->cb_fns.builtin_app_rx_callback (s); break; + case FIFO_EVENT_BUILTIN_TX: + s = session_get_from_handle_if_valid (e->session_handle); + if (PREDICT_TRUE (s != 0)) + session_tx_fifo_dequeue_internal (vm, node, e, s, &n_tx_packets); + break; case FIFO_EVENT_RPC: fp = e->rpc_args.fp; (*fp) (e->rpc_args.arg); diff --git a/src/vnet/tls/tls.c b/src/vnet/tls/tls.c index d5dbf2e16d1..aba7919927c 100644 --- a/src/vnet/tls/tls.c +++ b/src/vnet/tls/tls.c @@ -39,10 +39,35 @@ tls_get_available_engine (void) } int -tls_add_vpp_q_evt (svm_fifo_t * f, u8 evt_type) +tls_add_vpp_q_rx_evt (stream_session_t * s) { - if (svm_fifo_set_event (f)) - session_send_io_evt_to_thread (f, evt_type); + if (svm_fifo_set_event (s->server_rx_fifo)) + session_send_io_evt_to_thread (s->server_rx_fifo, FIFO_EVENT_APP_RX); + return 0; +} + +int +tls_add_vpp_q_builtin_rx_evt (stream_session_t * s) +{ + if (svm_fifo_set_event (s->server_rx_fifo)) + session_send_io_evt_to_thread (s->server_rx_fifo, FIFO_EVENT_BUILTIN_RX); + return 0; +} + +int +tls_add_vpp_q_tx_evt (stream_session_t * s) +{ + if (svm_fifo_set_event (s->server_tx_fifo)) + session_send_io_evt_to_thread (s->server_tx_fifo, FIFO_EVENT_APP_TX); + return 0; +} + +int +tls_add_vpp_q_builtin_tx_evt (stream_session_t * s) +{ + if (svm_fifo_set_event (s->server_tx_fifo)) + session_send_io_evt_to_thread_custom (s, s->thread_index, + FIFO_EVENT_BUILTIN_TX); return 0; } @@ -156,7 +181,13 @@ tls_notify_app_accept (tls_ctx_t * ctx) tls_ctx_t *lctx; int rv; - app_wrk = app_worker_get (ctx->parent_app_index); + app_wrk = app_worker_get_if_valid (ctx->parent_app_index); + if (!app_wrk) + { + tls_disconnect (ctx->tls_ctx_handle, vlib_get_thread_index ()); + return -1; + } + app = application_get (app_wrk->app_index); lctx = tls_listener_ctx_get (ctx->listener_ctx_index); @@ -191,7 +222,13 @@ tls_notify_app_connected (tls_ctx_t * ctx, u8 is_failed) app_worker_t *app_wrk; application_t *app; - app_wrk = app_worker_get (ctx->parent_app_index); + app_wrk = app_worker_get_if_valid (ctx->parent_app_index); + if (!app_wrk) + { + tls_disconnect (ctx->tls_ctx_handle, vlib_get_thread_index ()); + return -1; + } + app = application_get (app_wrk->app_index); cb_fn = app->cb_fns.session_connected_callback; @@ -404,19 +441,23 @@ tls_session_connected_callback (u32 tls_app_index, u32 ho_ctx_index, if (is_fail) { - int (*cb_fn) (u32, u32, stream_session_t *, u8); + int (*cb_fn) (u32, u32, stream_session_t *, u8), rv = 0; u32 wrk_index, api_context; app_worker_t *app_wrk; application_t *app; wrk_index = ho_ctx->parent_app_index; - api_context = ho_ctx->c_s_index; + app_wrk = app_worker_get_if_valid (ho_ctx->parent_app_index); + if (app_wrk) + { + api_context = ho_ctx->c_s_index; + app = application_get (app_wrk->app_index); + cb_fn = app->cb_fns.session_connected_callback; + rv = cb_fn (wrk_index, api_context, 0, 1 /* failed */ ); + } tls_ctx_half_open_reader_unlock (); tls_ctx_half_open_free (ho_ctx_index); - app_wrk = app_worker_get (ho_ctx->parent_app_index); - app = application_get (app_wrk->app_index); - cb_fn = app->cb_fns.session_connected_callback; - return cb_fn (wrk_index, api_context, 0, 1 /* failed */ ); + return rv; } ctx_handle = tls_ctx_alloc (ho_ctx->tls_ctx_engine); @@ -644,7 +685,9 @@ format_tls_connection (u8 * s, va_list * args) s = format (s, "%-50U", format_tls_ctx, ctx, thread_index); if (verbose) { - s = format (s, "%-15s", "state"); + stream_session_t *ts; + ts = session_get_from_handle (ctx->app_session_handle); + s = format (s, "state: %-7u", ts->session_state); if (verbose > 1) s = format (s, "\n"); } diff --git a/src/vnet/tls/tls.h b/src/vnet/tls/tls.h index 5515cb25b8a..09f1bdc7b07 100644 --- a/src/vnet/tls/tls.h +++ b/src/vnet/tls/tls.h @@ -116,7 +116,10 @@ typedef enum tls_engine_type_ tls_main_t *vnet_tls_get_main (void); void tls_register_engine (const tls_engine_vft_t * vft, tls_engine_type_t type); -int tls_add_vpp_q_evt (svm_fifo_t * f, u8 evt_type); +int tls_add_vpp_q_rx_evt (stream_session_t * s); +int tls_add_vpp_q_tx_evt (stream_session_t * s); +int tls_add_vpp_q_builtin_tx_evt (stream_session_t * s); +int tls_add_vpp_q_builtin_rx_evt (stream_session_t * s); int tls_notify_app_accept (tls_ctx_t * ctx); int tls_notify_app_connected (tls_ctx_t * ctx, u8 is_failed); void tls_notify_app_enqueue (tls_ctx_t * ctx, stream_session_t * app_session); -- 2.16.6