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 <fcoras@cisco.com>

diff --git a/src/plugins/tlsmbedtls/tls_mbedtls.c b/src/plugins/tlsmbedtls/tls_mbedtls.c
index 57a6e48..1010e37 100644 (file)
--- 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 744a07a..b8757a9 100644 (file)
--- 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 e67de95..b47dcf2 100644 (file)
--- 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 3eaf984..372a6f9 100644 (file)
--- 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 aa82755..1e08ccc 100644 (file)
--- 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 e2a6f4c..9a954bb 100644 (file)
--- 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 d5dbf2e..aba7919 100644 (file)
--- 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 5515cb2..09f1bdc 100644 (file)
--- 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);