ip: align reass.save_rewrite_length 45/24345/2 v20.05-rc0
authorKlement Sekera <ksekera@cisco.com>
Wed, 15 Jan 2020 10:30:48 +0000 (10:30 +0000)
committerOle Trøan <otroan@employees.org>
Wed, 15 Jan 2020 21:37:55 +0000 (21:37 +0000)
By aligning vnet_buffer_opaque.ip.save_rewrite_length and
vnet_buffer_opaque.ip.reass.save_rewrite_length we prevent shallow
virtual reassembly code from overwrite save_rewrite_length, allowing
other features down the pipe to rely on this value.

A static assert is added to guard this alignment.

Type: fix
Fixes: f126e746fc01c75bc99329d10ce9127b26b23814

Change-Id: Ie7c7f3abc2a221bbcf2830c0f006a4368088b342
Signed-off-by: Klement Sekera <ksekera@cisco.com>
src/vnet/buffer.h
src/vnet/ip/reass/ip4_sv_reass.c
src/vnet/ip/reass/ip6_sv_reass.c

index b174587..5293561 100644 (file)
@@ -209,13 +209,13 @@ typedef struct
              /* shallow virtual reassembly output variables */
              struct
              {
-               u8 ip_proto;    /* protocol in ip header */
-               u8 icmp_type_or_tcp_flags;
-               u8 is_non_first_fragment;
-               u8 save_rewrite_length;
                u16 l4_src_port;        /* tcp/udp/icmp src port */
                u16 l4_dst_port;        /* tcp/udp/icmp dst port */
                u32 tcp_ack_number;
+               u8 save_rewrite_length;
+               u8 ip_proto;    /* protocol in ip header */
+               u8 icmp_type_or_tcp_flags;
+               u8 is_non_first_fragment;
                u32 tcp_seq_number;
              };
              /* full reassembly output variables */
@@ -397,6 +397,15 @@ STATIC_ASSERT (STRUCT_SIZE_OF (vnet_buffer_opaque_t, ip.save_rewrite_length)
               && VNET_REWRITE_TOTAL_BYTES < UINT8_MAX,
               "save_rewrite_length member must be able to hold the max value of rewrite length");
 
+STATIC_ASSERT (STRUCT_OFFSET_OF (vnet_buffer_opaque_t, ip.save_rewrite_length)
+              == STRUCT_OFFSET_OF (vnet_buffer_opaque_t,
+                                   ip.reass.save_rewrite_length)
+              && STRUCT_OFFSET_OF (vnet_buffer_opaque_t,
+                                   mpls.save_rewrite_length) ==
+              STRUCT_OFFSET_OF (vnet_buffer_opaque_t,
+                                ip.reass.save_rewrite_length),
+              "save_rewrite_length must be aligned so that reass doesn't overwrite it");
+
 /*
  * The opaque field of the vlib_buffer_t is interpreted as a
  * vnet_buffer_opaque_t. Hence it should be big enough to accommodate one.
index b94e9b2..c50cf6d 100644 (file)
@@ -466,8 +466,6 @@ ip4_sv_reass_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
                {
                  next0 = IP4_SV_REASSEMBLY_NEXT_INPUT;
                }
-             vnet_buffer (b0)->ip.reass.save_rewrite_length =
-               vnet_buffer (b0)->ip.save_rewrite_length;
              vnet_buffer (b0)->ip.reass.is_non_first_fragment = 0;
              vnet_buffer (b0)->ip.reass.ip_proto = ip0->protocol;
              if (IP_PROTOCOL_TCP == ip0->protocol)
@@ -547,8 +545,6 @@ ip4_sv_reass_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
                {
                  next0 = IP4_SV_REASSEMBLY_NEXT_INPUT;
                }
-             vnet_buffer (b0)->ip.reass.save_rewrite_length =
-               vnet_buffer (b0)->ip.save_rewrite_length;
              vnet_buffer (b0)->ip.reass.is_non_first_fragment =
                ! !fragment_first;
              vnet_buffer (b0)->ip.reass.ip_proto = reass->ip_proto;
@@ -620,9 +616,6 @@ ip4_sv_reass_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
                to_next[0] = bi0;
                to_next += 1;
                n_left_to_next -= 1;
-               ASSERT (vnet_buffer (b0)->ip.save_rewrite_length < (2 << 14));
-               vnet_buffer (b0)->ip.reass.save_rewrite_length =
-                 vnet_buffer (b0)->ip.save_rewrite_length;
                vnet_buffer (b0)->ip.reass.is_non_first_fragment =
                  ! !ip4_get_fragment_offset (vlib_buffer_get_current (b0));
                vnet_buffer (b0)->ip.reass.ip_proto = reass->ip_proto;
index 4426177..5f46966 100644 (file)
@@ -559,9 +559,6 @@ ip6_sv_reassembly_inline (vlib_main_t * vm,
                  next0 = IP6_SV_REASSEMBLY_NEXT_DROP;
                  goto packet_enqueue;
                }
-             ASSERT (vnet_buffer (b0)->ip.save_rewrite_length < (2 << 14));
-             vnet_buffer (b0)->ip.reass.save_rewrite_length =
-               vnet_buffer (b0)->ip.save_rewrite_length;
              vnet_buffer (b0)->ip.reass.is_non_first_fragment = 0;
              next0 = IP6_SV_REASSEMBLY_NEXT_INPUT;
              if (PREDICT_FALSE (b0->flags & VLIB_BUFFER_IS_TRACED))
@@ -631,9 +628,6 @@ ip6_sv_reassembly_inline (vlib_main_t * vm,
 
          if (reass->is_complete)
            {
-             ASSERT (vnet_buffer (b0)->ip.save_rewrite_length < (2 << 14));
-             vnet_buffer (b0)->ip.reass.save_rewrite_length =
-               vnet_buffer (b0)->ip.save_rewrite_length;
              vnet_buffer (b0)->ip.reass.is_non_first_fragment =
                ! !ip6_frag_hdr_offset (frag_hdr);
              vnet_buffer (b0)->ip.reass.ip_proto = reass->ip_proto;
@@ -712,9 +706,6 @@ ip6_sv_reassembly_inline (vlib_main_t * vm,
                frag_hdr =
                  vlib_buffer_get_current (b0) +
                  vnet_buffer (b0)->ip.reass.ip6_frag_hdr_offset;
-               ASSERT (vnet_buffer (b0)->ip.save_rewrite_length < (2 << 14));
-               vnet_buffer (b0)->ip.reass.save_rewrite_length =
-                 vnet_buffer (b0)->ip.save_rewrite_length;
                vnet_buffer (b0)->ip.reass.is_non_first_fragment =
                  ! !ip6_frag_hdr_offset (frag_hdr);
                vnet_buffer (b0)->ip.reass.ip_proto = reass->ip_proto;