nat: fix potential out-of-bound worker array index

In several NAT submodules, the number of available ports (0xffff - 1024)
may not be divisible by the number of workers, so port_per_thread is
determined by integer division, which is the floor of the quotient.
Later when a worker index is needed, dividing the port with port_per_thread
may yield an out-of-bound array index into the workers array.

As an example, assume 2 workers are configured, then port_per_thread
will be (0xffff - 1024) / 2, which is 32255. When we compute a worker
index with port 0xffff, we get (0xffff - 1024) / 32255, which is 2,
but since we only have 2 workers, only 0 and 1 are valid indices.

This patch fixes the problem by adding a modulo at the end of the division.

Type: fix
Signed-off-by: Jing Peng <pj.hades@gmail.com>
Change-Id: Ieae3d5faf716410422610484a68222f1c957f3f8

diff --git a/src/plugins/nat/nat44-ed/nat44_ed.c b/src/plugins/nat/nat44-ed/nat44_ed.c
index 9c79753..27c1870 100644 (file)
--- a/src/plugins/nat/nat44-ed/nat44_ed.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed.c
@@ -761,9 +761,9 @@ get_thread_idx_by_port (u16 e_port)
   u32 thread_idx = sm->num_workers;
   if (sm->num_workers > 1)
     {
-      thread_idx =
-       sm->first_worker_index +
-       sm->workers[(e_port - 1024) / sm->port_per_thread];
+      thread_idx = sm->first_worker_index +
+                  sm->workers[(e_port - 1024) / sm->port_per_thread %
+                              _vec_len (sm->workers)];
     }
   return thread_idx;
 }
@@ -3166,9 +3166,7 @@ nat44_ed_get_out2in_worker_index (vlib_buffer_t *b, ip4_header_t *ip,
     }
 
   /* worker by outside port */
-  next_worker_index = sm->first_worker_index;
-  next_worker_index +=
-    sm->workers[(clib_net_to_host_u16 (port) - 1024) / sm->port_per_thread];
+  next_worker_index = get_thread_idx_by_port (clib_net_to_host_u16 (port));
 
 done:
   nat_elog_debug_handoff (sm, "HANDOFF OUT2IN", next_worker_index,

diff --git a/src/plugins/nat/nat44-ei/nat44_ei.c b/src/plugins/nat/nat44-ei/nat44_ei.c
index 30c61b2..4f2fa61 100644 (file)
--- a/src/plugins/nat/nat44-ei/nat44_ei.c
+++ b/src/plugins/nat/nat44-ei/nat44_ei.c
@@ -1683,6 +1683,20 @@ nat44_ei_get_in2out_worker_index (ip4_header_t *ip0, u32 rx_fib_index0,
   return next_worker_index;
 }
 
+u32
+nat44_ei_get_thread_idx_by_port (u16 e_port)
+{
+  nat44_ei_main_t *nm = &nat44_ei_main;
+  u32 thread_idx = nm->num_workers;
+  if (nm->num_workers > 1)
+    {
+      thread_idx = nm->first_worker_index +
+                  nm->workers[(e_port - 1024) / nm->port_per_thread %
+                              _vec_len (nm->workers)];
+    }
+  return thread_idx;
+}
+
 u32
 nat44_ei_get_out2in_worker_index (vlib_buffer_t *b, ip4_header_t *ip0,
                                  u32 rx_fib_index0, u8 is_output)
@@ -1761,9 +1775,8 @@ nat44_ei_get_out2in_worker_index (vlib_buffer_t *b, ip4_header_t *ip0,
     }
 
   /* worker by outside port */
-  next_worker_index = nm->first_worker_index;
-  next_worker_index +=
-    nm->workers[(clib_net_to_host_u16 (port) - 1024) / nm->port_per_thread];
+  next_worker_index =
+    nat44_ei_get_thread_idx_by_port (clib_net_to_host_u16 (port));
   return next_worker_index;
 }
 
@@ -2050,19 +2063,6 @@ nat44_ei_del_session (nat44_ei_main_t *nm, ip4_address_t *addr, u16 port,
   return VNET_API_ERROR_NO_SUCH_ENTRY;
 }
 
-u32
-nat44_ei_get_thread_idx_by_port (u16 e_port)
-{
-  nat44_ei_main_t *nm = &nat44_ei_main;
-  u32 thread_idx = nm->num_workers;
-  if (nm->num_workers > 1)
-    {
-      thread_idx = nm->first_worker_index +
-                  nm->workers[(e_port - 1024) / nm->port_per_thread];
-    }
-  return thread_idx;
-}
-
 void
 nat44_ei_add_del_addr_to_fib (ip4_address_t *addr, u8 p_len, u32 sw_if_index,
                              int is_add)

diff --git a/src/plugins/nat/nat64/nat64.c b/src/plugins/nat/nat64/nat64.c
index 1c1cdfb..79e9da0 100644 (file)
--- a/src/plugins/nat/nat64/nat64.c
+++ b/src/plugins/nat/nat64/nat64.c
@@ -135,6 +135,20 @@ nat64_get_worker_in2out (ip6_address_t * addr)
   return next_worker_index;
 }
 
+static u32
+get_thread_idx_by_port (u16 e_port)
+{
+  nat64_main_t *nm = &nat64_main;
+  u32 thread_idx = nm->num_workers;
+  if (nm->num_workers > 1)
+    {
+      thread_idx = nm->first_worker_index +
+                  nm->workers[(e_port - 1024) / nm->port_per_thread %
+                              _vec_len (nm->workers)];
+    }
+  return thread_idx;
+}
+
 u32
 nat64_get_worker_out2in (vlib_buffer_t * b, ip4_header_t * ip)
 {
@@ -202,7 +216,7 @@ nat64_get_worker_out2in (vlib_buffer_t * b, ip4_header_t * ip)
   /* worker by outside port  (TCP/UDP) */
   port = clib_net_to_host_u16 (port);
   if (port > 1024)
-    return nm->first_worker_index + ((port - 1024) / nm->port_per_thread);
+    return get_thread_idx_by_port (port);
 
   return vlib_get_thread_index ();
 }
@@ -916,7 +930,7 @@ nat64_add_del_static_bib_entry (ip6_address_t * in_addr,
       /* outside port must be assigned to same thread as internall address */
       if ((out_port > 1024) && (nm->num_workers > 1))
        {
-         if (thread_index != ((out_port - 1024) / nm->port_per_thread))
+         if (thread_index != get_thread_idx_by_port (out_port))
            return VNET_API_ERROR_INVALID_VALUE_2;
        }