From e63a2d44d16774a88763c5f6368a3f7210c64ddc Mon Sep 17 00:00:00 2001 From: Klement Sekera Date: Thu, 20 May 2021 12:23:00 +0200 Subject: [PATCH] ip: fix buffer leaks in reassembly Type: fix Signed-off-by: Klement Sekera Change-Id: I952ba7e042779855e29628d048da7edec1caaafd --- src/vnet/ip/reass/ip4_full_reass.c | 24 ++++++++++++------------ src/vnet/ip/reass/ip6_full_reass.c | 37 ++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/vnet/ip/reass/ip4_full_reass.c b/src/vnet/ip/reass/ip4_full_reass.c index e4839aaa981..b0840b1327a 100644 --- a/src/vnet/ip/reass/ip4_full_reass.c +++ b/src/vnet/ip/reass/ip4_full_reass.c @@ -177,8 +177,6 @@ typedef struct // convenience vlib_main_t *vlib_main; - // node index of ip4-drop node - u32 ip4_drop_idx; u32 ip4_full_reass_expire_node_idx; /** Worker handoff */ @@ -421,7 +419,7 @@ ip4_full_reass_free (ip4_full_reass_main_t * rm, always_inline void ip4_full_reass_drop_all (vlib_main_t *vm, vlib_node_runtime_t *node, - ip4_full_reass_t *reass) + ip4_full_reass_t *reass, u32 offending_bi) { u32 range_bi = reass->first_bi; vlib_buffer_t *range_b; @@ -435,6 +433,10 @@ ip4_full_reass_drop_all (vlib_main_t *vm, vlib_node_runtime_t *node, while (~0 != bi) { vec_add1 (to_free, bi); + if (offending_bi == bi) + { + offending_bi = ~0; + } vlib_buffer_t *b = vlib_get_buffer (vm, bi); if (b->flags & VLIB_BUFFER_NEXT_PRESENT) { @@ -448,6 +450,10 @@ ip4_full_reass_drop_all (vlib_main_t *vm, vlib_node_runtime_t *node, } range_bi = range_vnb->ip.reass.next_range_bi; } + if (~0 != offending_bi) + { + vec_add1 (to_free, offending_bi); + } /* send to next_error_index */ if (~0 != reass->error_next_index) { @@ -518,7 +524,7 @@ again: if (now > reass->last_heard + rm->timeout) { - ip4_full_reass_drop_all (vm, node, reass); + ip4_full_reass_drop_all (vm, node, reass, ~0); ip4_full_reass_free (rm, rt, reass); reass = NULL; } @@ -1219,7 +1225,7 @@ ip4_full_reass_inline (vlib_main_t *vm, vlib_node_runtime_t *node, { vlib_node_increment_counter (vm, node->node_index, counter, 1); - ip4_full_reass_drop_all (vm, node, reass); + ip4_full_reass_drop_all (vm, node, reass, bi0); ip4_full_reass_free (rm, rt, reass); goto next_packet; } @@ -1531,10 +1537,6 @@ ip4_full_reass_init_function (vlib_main_t * vm) nbuckets = ip4_full_reass_get_nbuckets (); clib_bihash_init_16_8 (&rm->hash, "ip4-dr", nbuckets, nbuckets * 1024); - node = vlib_get_node_by_name (vm, (u8 *) "ip4-drop"); - ASSERT (node); - rm->ip4_drop_idx = node->index; - rm->fq_index = vlib_frame_queue_main_init (ip4_full_reass_node.index, 0); rm->fq_local_index = vlib_frame_queue_main_init (ip4_local_full_reass_node.index, 0); @@ -1605,7 +1607,7 @@ ip4_full_reass_walk_expired (vlib_main_t *vm, vlib_node_runtime_t *node, vec_foreach (i, pool_indexes_to_free) { ip4_full_reass_t *reass = pool_elt_at_index (rt->pool, i[0]); - ip4_full_reass_drop_all (vm, node, reass); + ip4_full_reass_drop_all (vm, node, reass, ~0); ip4_full_reass_free (rm, rt, reass); } @@ -1911,7 +1913,6 @@ VLIB_NODE_FN (ip4_full_reass_feature_handoff_node) (vlib_main_t * vm, false /* is_local */); } - VLIB_REGISTER_NODE (ip4_full_reass_feature_handoff_node) = { .name = "ip4-full-reass-feature-hoff", .vector_size = sizeof (u32), @@ -1935,7 +1936,6 @@ VLIB_NODE_FN (ip4_full_reass_custom_handoff_node) (vlib_main_t * vm, false /* is_local */); } - VLIB_REGISTER_NODE (ip4_full_reass_custom_handoff_node) = { .name = "ip4-full-reass-custom-hoff", .vector_size = sizeof (u32), diff --git a/src/vnet/ip/reass/ip6_full_reass.c b/src/vnet/ip/reass/ip6_full_reass.c index 3b61d141d85..4a6c6a106f0 100644 --- a/src/vnet/ip/reass/ip6_full_reass.c +++ b/src/vnet/ip/reass/ip6_full_reass.c @@ -42,6 +42,7 @@ typedef enum IP6_FULL_REASS_RC_NO_BUF, IP6_FULL_REASS_RC_HANDOFF, IP6_FULL_REASS_RC_INVALID_FRAG_LEN, + IP6_FULL_REASS_RC_OVERLAP, } ip6_full_reass_rc_t; typedef struct @@ -157,8 +158,6 @@ typedef struct // convenience vlib_main_t *vlib_main; - // node index of ip6-drop node - u32 ip6_drop_idx; u32 ip6_icmp_error_idx; u32 ip6_full_reass_expire_node_idx; @@ -408,7 +407,7 @@ ip6_full_reass_free (ip6_full_reass_main_t * rm, always_inline void ip6_full_reass_drop_all (vlib_main_t *vm, vlib_node_runtime_t *node, - ip6_full_reass_t *reass) + ip6_full_reass_t *reass, u32 offending_bi) { u32 range_bi = reass->first_bi; vlib_buffer_t *range_b; @@ -422,6 +421,10 @@ ip6_full_reass_drop_all (vlib_main_t *vm, vlib_node_runtime_t *node, while (~0 != bi) { vec_add1 (to_free, bi); + if (bi == offending_bi) + { + offending_bi = ~0; + } vlib_buffer_t *b = vlib_get_buffer (vm, bi); if (b->flags & VLIB_BUFFER_NEXT_PRESENT) { @@ -435,6 +438,10 @@ ip6_full_reass_drop_all (vlib_main_t *vm, vlib_node_runtime_t *node, } range_bi = range_vnb->ip.reass.next_range_bi; } + if (~0 != offending_bi) + { + vec_add1 (to_free, offending_bi); + } /* send to next_error_index */ if (~0 != reass->error_next_index) { @@ -503,7 +510,7 @@ ip6_full_reass_on_timeout (vlib_main_t * vm, vlib_node_runtime_t * node, 0); } } - ip6_full_reass_drop_all (vm, node, reass); + ip6_full_reass_drop_all (vm, node, reass, ~0); } always_inline ip6_full_reass_t * @@ -666,13 +673,13 @@ ip6_full_reass_finalize (vlib_main_t * vm, vlib_node_runtime_t * node, if (trim_front > tmp->current_length) { /* drop whole buffer */ - vec_add1 (vec_drop_compress, tmp_bi); - trim_front -= tmp->current_length; if (!(tmp->flags & VLIB_BUFFER_NEXT_PRESENT)) { rv = IP6_FULL_REASS_RC_INTERNAL_ERROR; goto free_buffers_and_return; } + trim_front -= tmp->current_length; + vec_add1 (vec_drop_compress, tmp_bi); tmp->flags &= ~VLIB_BUFFER_NEXT_PRESENT; tmp_bi = tmp->next_buffer; tmp = vlib_get_buffer (vm, tmp_bi); @@ -710,12 +717,12 @@ ip6_full_reass_finalize (vlib_main_t * vm, vlib_node_runtime_t * node, } else { - vec_add1 (vec_drop_compress, tmp_bi); if (reass->first_bi == tmp_bi) { rv = IP6_FULL_REASS_RC_INTERNAL_ERROR; goto free_buffers_and_return; } + vec_add1 (vec_drop_compress, tmp_bi); ++dropped_cnt; } if (tmp->flags & VLIB_BUFFER_NEXT_PRESENT) @@ -966,11 +973,7 @@ ip6_full_reass_update (vlib_main_t *vm, vlib_node_runtime_t *node, ip6_full_reass_add_trace (vm, node, reass, *bi0, frag_hdr, RANGE_OVERLAP, ~0); } - ip6_full_reass_drop_all (vm, node, 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; + return IP6_FULL_REASS_RC_OVERLAP; } break; } @@ -1230,14 +1233,18 @@ ip6_full_reassembly_inline (vlib_main_t *vm, vlib_node_runtime_t *node, case IP6_FULL_REASS_RC_INVALID_FRAG_LEN: counter = IP6_ERROR_REASS_INVALID_FRAG_LEN; break; + case IP6_FULL_REASS_RC_OVERLAP: + counter = IP6_ERROR_REASS_OVERLAPPING_FRAGMENT; + break; } if (~0 != counter) { vlib_node_increment_counter (vm, node->node_index, counter, 1); - ip6_full_reass_drop_all (vm, node, reass); + ip6_full_reass_drop_all (vm, node, reass, bi0); ip6_full_reass_free (rm, rt, reass); goto next_packet; + break; } } else @@ -1530,9 +1537,6 @@ ip6_full_reass_init_function (vlib_main_t * vm) clib_bihash_init_48_8 (&rm->hash, "ip6-full-reass", nbuckets, nbuckets * 1024); - node = vlib_get_node_by_name (vm, (u8 *) "ip6-drop"); - ASSERT (node); - rm->ip6_drop_idx = node->index; node = vlib_get_node_by_name (vm, (u8 *) "ip6-icmp-error"); ASSERT (node); rm->ip6_icmp_error_idx = node->index; @@ -1931,7 +1935,6 @@ VLIB_NODE_FN (ip6_full_reassembly_feature_handoff_node) (vlib_main_t * vm, vm, node, frame, true /* is_feature */, false /* is_local */); } - VLIB_REGISTER_NODE (ip6_full_reassembly_feature_handoff_node) = { .name = "ip6-full-reass-feature-hoff", .vector_size = sizeof (u32), -- 2.16.6