cnat: Fix invalid adj_index 35/29735/2
authorNathan Skrzypczak <nathan.skrzypczak@gmail.com>
Tue, 3 Nov 2020 16:44:28 +0000 (17:44 +0100)
committerDave Barach <openvpp@barachs.net>
Tue, 10 Nov 2020 10:33:02 +0000 (10:33 +0000)
Type: fix

When using sNAT in combination with cnat translations
it might happen that the cnat_node_vip.c picks up a
translation on a session that has an invalid lb index,
thus resulting in a later crash in ip4-load-balance

Change-Id: I82607086b2d672a9dcf26bfb82ad7f83e6474562
Signed-off-by: Nathan Skrzypczak <nathan.skrzypczak@gmail.com>
src/plugins/cnat/cnat_node_vip.c
src/plugins/cnat/cnat_session.h

index 8dd53ad..ffe5899 100644 (file)
@@ -106,17 +106,15 @@ cnat_vip_node_fn (vlib_main_t * vm,
       goto trace;
     }
 
-  ct = cnat_find_translation (cc->parent_cci,
-                             clib_host_to_net_u16 (udp0->dst_port), iproto);
-
   if (!rv)
     {
       /* session table hit */
       cnat_timestamp_update (session->value.cs_ts_index, ctx->now);
 
-      if (NULL != ct)
+      if (INDEX_INVALID != session->value.cs_lbi)
        {
          /* Translate & follow the translation given LB */
+         ct = cnat_translation_get (session->value.ct_index);
          next0 = ct->ct_lb.dpoi_next_node;
          vnet_buffer (b)->ip.adj_index[VLIB_TX] = session->value.cs_lbi;
        }
@@ -135,6 +133,9 @@ cnat_vip_node_fn (vlib_main_t * vm,
     }
   else
     {
+      ct =
+       cnat_find_translation (cc->parent_cci,
+                              clib_host_to_net_u16 (udp0->dst_port), iproto);
       if (NULL == ct)
        {
          /* Dont translate & Follow the fib programming */
@@ -192,7 +193,7 @@ cnat_vip_node_fn (vlib_main_t * vm,
       session->value.cs_port[VLIB_RX] =
        clib_host_to_net_u16 (trk0->ct_ep[VLIB_RX].ce_port);
 
-      session->value.flags = 0;
+      session->value.ct_index = ct - cnat_translation_pool;
       session->value.cs_lbi = dpo0->dpoi_index;
 
       rv = cspm->vip_policy (vm, b, session, &rsession_flags, ct, ctx);
index a1f3486..e352fe6 100644 (file)
@@ -91,18 +91,36 @@ typedef struct cnat_session_t_
      * Timestamp index this session was last used
      */
     u32 cs_ts_index;
-    /**
-     * Indicates a return path session that was source NATed
-     * on the way in.
-     */
-    u32 flags;
+
+    union
+    {
+       /**
+        * session flags if cs_lbi == INDEX_INVALID
+        */
+      u32 flags;
+       /**
+        * Persist translation->ct_lb.dpoi_next_node
+        * when cs_lbi != INDEX_INVALID
+        */
+      u32 ct_index;
+    };
   } value;
 } cnat_session_t;
 
 typedef enum cnat_session_flag_t_
 {
+  /**
+   * Indicates a return path session that was source NATed
+   * on the way in.
+   */
   CNAT_SESSION_FLAG_HAS_SNAT = (1 << 0),
+  /**
+   * This session source port was allocated, free it on cleanup
+   */
   CNAT_SESSION_FLAG_ALLOC_PORT = (1 << 1),
+  /**
+   * This session doesn't have a client, do not attempt to free it
+   */
   CNAT_SESSION_FLAG_NO_CLIENT = (1 << 2),
 } cnat_session_flag_t;