commit 2d0ebd7ebc555565868038a09d80a61f5de29430
author Benoît Ganne <bganne@cisco.com> Fri Jul 19 13:42:12 2019 +0200
committer Ole Trøan <otroan@employees.org> Thu Sep 26 16:34:56 2019 +0000
tree c1d937ec41728ff611691e78cd70905c0731b1e9
parent a025b3ea353b5c5c356efda0888d75a2ab8979e0

ip: fix use-after-free in reassembly

 - ip{4,6}_reass_finalize() frees the reassembly context: do not access
it after the call.
 - traces access reassembly context: free it after and not before
tracing.

Type: fix

Change-Id: Ia3aaea9c7b74932e249e013be04b9bd7298fd187
Signed-off-by: Benoît Ganne <bganne@cisco.com>

src/vnet/ip/reass/ip4_full_reass.c

diff --git a/src/vnet/ip/reass/ip4_full_reass.c b/src/vnet/ip/reass/ip4_full_reass.c
index 18ac4d1..176c01c 100644
--- a/src/vnet/ip/reass/ip4_full_reass.c
+++ b/src/vnet/ip/reass/ip4_full_reass.c
@@ -1040,11 +1040,12 @@
       reass->data_len == reass->last_packet_octet + 1)
     {
       *handoff_thread_idx = reass->sendout_thread_index;
+      int handoff =
+	reass->memory_owner_thread_index != reass->sendout_thread_index;
       rc =
 	ip4_full_reass_finalize (vm, node, rm, rt, reass, bi0, next0, error0,
 				 is_custom_app);
-      if (IP4_REASS_RC_OK == rc
-	  && reass->memory_owner_thread_index != reass->sendout_thread_index)
+      if (IP4_REASS_RC_OK == rc && handoff)
 	{
 	  rc = IP4_REASS_RC_HANDOFF;
 	}

src/vnet/ip/reass/ip6_full_reass.c

diff --git a/src/vnet/ip/reass/ip6_full_reass.c b/src/vnet/ip/reass/ip6_full_reass.c
index 0b41dea..92fab60 100644
--- a/src/vnet/ip/reass/ip6_full_reass.c
+++ b/src/vnet/ip/reass/ip6_full_reass.c
@@ -885,13 +885,13 @@
       else
 	{
 	  // overlapping fragment - not allowed by RFC 8200
-	  ip6_full_reass_drop_all (vm, node, rm, reass);
-	  ip6_full_reass_free (rm, rt, reass);
 	  if (PREDICT_FALSE (fb->flags & VLIB_BUFFER_IS_TRACED))
 	    {
 	      ip6_full_reass_add_trace (vm, node, rm, reass, *bi0,
 					RANGE_OVERLAP, ~0);
 	    }
+	  ip6_full_reass_drop_all (vm, node, rm, reass);
+	  ip6_full_reass_free (rm, rt, reass);
 	  *next0 = IP6_FULL_REASSEMBLY_NEXT_DROP;
 	  *error0 = IP6_ERROR_REASS_OVERLAPPING_FRAGMENT;
 	  return IP6_FULL_REASS_RC_OK;
@@ -911,11 +911,12 @@
       reass->data_len == reass->last_packet_octet + 1)
     {
       *handoff_thread_idx = reass->sendout_thread_index;
+      int handoff =
+	reass->memory_owner_thread_index != reass->sendout_thread_index;
       ip6_full_reass_rc_t rc =
 	ip6_full_reass_finalize (vm, node, rm, rt, reass, bi0, next0, error0,
 				 is_custom_app);
-      if (IP6_FULL_REASS_RC_OK == rc
-	  && reass->memory_owner_thread_index != reass->sendout_thread_index)
+      if (IP6_FULL_REASS_RC_OK == rc && handoff)
 	{
 	  return IP6_FULL_REASS_RC_HANDOFF;
 	}