From 3a63fc5470caffda434064a439ffdbe8518963f9 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Mon, 7 Jan 2019 09:15:47 -0500 Subject: [PATCH] Handle buffer alloc failure in vlib_buffer_add_data It's not OK to crash due to a transient buffer allocation failure. Return 1 if the requested operation failed, otherwise 0. Buffer index parameter change to a value-result, so the caller can differentiate between partial and complete allocation failure: callers which request an initial allocation (inbound bi = ~0) need to check the (out) value to decide whether or not to call vlib_buffer_free(...). Change-Id: I03029d7f2714c17dca4630dfd95a1eb578b68384 Signed-off-by: Dave Barach --- src/vlib/buffer.c | 13 ++++----- src/vlib/buffer_funcs.h | 4 +-- src/vnet/ip/icmp6.h | 3 ++- src/vnet/ip/ip6_neighbor.c | 67 +++++++++++++++++++++++++++++++--------------- src/vnet/srp/node.c | 36 +++++++++++++++++-------- 5 files changed, 81 insertions(+), 42 deletions(-) diff --git a/src/vlib/buffer.c b/src/vlib/buffer.c index d024aba1e0f..5370d501293 100644 --- a/src/vlib/buffer.c +++ b/src/vlib/buffer.c @@ -705,16 +705,16 @@ vlib_packet_template_get_packet (vlib_main_t * vm, } /* Append given data to end of buffer, possibly allocating new buffers. */ -u32 +int vlib_buffer_add_data (vlib_main_t * vm, vlib_buffer_free_list_index_t free_list_index, - u32 buffer_index, void *data, u32 n_data_bytes) + u32 * buffer_index, void *data, u32 n_data_bytes) { u32 n_buffer_bytes, n_left, n_left_this_buffer, bi; vlib_buffer_t *b; void *d; - bi = buffer_index; + bi = *buffer_index; if (bi == ~0 && 1 != vlib_buffer_alloc_from_free_list (vm, &bi, 1, free_list_index)) goto out_of_buffers; @@ -756,11 +756,12 @@ vlib_buffer_add_data (vlib_main_t * vm, b = vlib_get_buffer (vm, b->next_buffer); } - return bi; + *buffer_index = bi; + return 0; out_of_buffers: - clib_error ("out of buffers"); - return bi; + clib_warning ("out of buffers"); + return 1; } u16 diff --git a/src/vlib/buffer_funcs.h b/src/vlib/buffer_funcs.h index 54fc1f61598..b561a91c394 100644 --- a/src/vlib/buffer_funcs.h +++ b/src/vlib/buffer_funcs.h @@ -672,9 +672,9 @@ vlib_buffer_free_list_buffer_size (vlib_main_t * vm, } /* Append given data to end of buffer, possibly allocating new buffers. */ -u32 vlib_buffer_add_data (vlib_main_t * vm, +int vlib_buffer_add_data (vlib_main_t * vm, vlib_buffer_free_list_index_t free_list_index, - u32 buffer_index, void *data, u32 n_data_bytes); + u32 * buffer_index, void *data, u32 n_data_bytes); /* duplicate all buffers in chain */ always_inline vlib_buffer_t * diff --git a/src/vnet/ip/icmp6.h b/src/vnet/ip/icmp6.h index 1cac8240466..628d225eac4 100644 --- a/src/vnet/ip/icmp6.h +++ b/src/vnet/ip/icmp6.h @@ -46,7 +46,8 @@ _ (PACKET_TOO_BIG_SENT, "packet too big response sent") \ _ (TTL_EXPIRE_SENT, "hop limit exceeded response sent") \ _ (PARAM_PROBLEM_SENT, "parameter problem response sent") \ - _ (DROP, "error message dropped") + _ (DROP, "error message dropped") \ + _ (ALLOC_FAILURE, "buffer allocation failure") typedef enum diff --git a/src/vnet/ip/ip6_neighbor.c b/src/vnet/ip/ip6_neighbor.c index f9acfeff389..fde3a7e71d4 100755 --- a/src/vnet/ip/ip6_neighbor.c +++ b/src/vnet/ip/ip6_neighbor.c @@ -1690,11 +1690,15 @@ icmp6_router_solicitation (vlib_main_t * vm, u16 payload_length = sizeof (icmp6_router_advertisement_header_t); - vlib_buffer_add_data (vm, - vlib_buffer_get_free_list_index - (p0), bi0, (void *) &rh, - sizeof - (icmp6_router_advertisement_header_t)); + if (vlib_buffer_add_data + (vm, vlib_buffer_get_free_list_index + (p0), &bi0, (void *) &rh, + sizeof (icmp6_router_advertisement_header_t))) + { + /* buffer allocation failed, drop the pkt */ + error0 = ICMP6_ERROR_ALLOC_FAILURE; + goto drop0; + } if (radv_info->adv_link_layer_address) { @@ -1709,11 +1713,15 @@ icmp6_router_solicitation (vlib_main_t * vm, clib_memcpy (&h.ethernet_address[0], eth_if0->address, 6); - vlib_buffer_add_data (vm, - vlib_buffer_get_free_list_index - (p0), bi0, (void *) &h, - sizeof - (icmp6_neighbor_discovery_ethernet_link_layer_address_option_t)); + if (vlib_buffer_add_data + (vm, vlib_buffer_get_free_list_index + (p0), &bi0, (void *) &h, + sizeof + (icmp6_neighbor_discovery_ethernet_link_layer_address_option_t))) + { + error0 = ICMP6_ERROR_ALLOC_FAILURE; + goto drop0; + } payload_length += sizeof @@ -1734,11 +1742,15 @@ icmp6_router_solicitation (vlib_main_t * vm, payload_length += sizeof (icmp6_neighbor_discovery_mtu_option_t); - vlib_buffer_add_data (vm, - vlib_buffer_get_free_list_index - (p0), bi0, (void *) &h, - sizeof - (icmp6_neighbor_discovery_mtu_option_t)); + if (vlib_buffer_add_data + (vm, vlib_buffer_get_free_list_index + (p0), &bi0, (void *) &h, + sizeof + (icmp6_neighbor_discovery_mtu_option_t))) + { + error0 = ICMP6_ERROR_ALLOC_FAILURE; + goto drop0; + } } /* add advertised prefix options */ @@ -1787,10 +1799,15 @@ icmp6_router_solicitation (vlib_main_t * vm, payload_length += sizeof( icmp6_neighbor_discovery_prefix_information_option_t); - vlib_buffer_add_data (vm, - vlib_buffer_get_free_list_index (p0), - bi0, - (void *)&h, sizeof(icmp6_neighbor_discovery_prefix_information_option_t)); + if (vlib_buffer_add_data + (vm, vlib_buffer_get_free_list_index (p0), + &bi0, + (void *)&h, + sizeof(icmp6_neighbor_discovery_prefix_information_option_t))) + { + error0 = ICMP6_ERROR_ALLOC_FAILURE; + goto drop0; + } } })); @@ -1875,6 +1892,7 @@ icmp6_router_solicitation (vlib_main_t * vm, } } + drop0: p0->error = error_node->errors[error0]; if (error0 != ICMP6_ERROR_NONE) @@ -2761,6 +2779,7 @@ ip6_neighbor_send_mldpv2_report (u32 sw_if_index) n_allocated = vlib_buffer_alloc (vm, &bo0, n_to_alloc); if (PREDICT_FALSE (n_allocated == 0)) { + alloc_fail: clib_warning ("buffer allocation failure"); return; } @@ -2827,9 +2846,13 @@ ip6_neighbor_send_mldpv2_report (u32 sw_if_index) num_addr_records++; - vlib_buffer_add_data - (vm, vlib_buffer_get_free_list_index (b0), bo0, - (void *)&rr, sizeof(icmp6_multicast_address_record_t)); + if(vlib_buffer_add_data + (vm, vlib_buffer_get_free_list_index (b0), &bo0, + (void *)&rr, sizeof(icmp6_multicast_address_record_t))) + { + vlib_buffer_free (vm, &bo0, 1); + goto alloc_fail; + } payload_length += sizeof( icmp6_multicast_address_record_t); })); diff --git a/src/vnet/srp/node.c b/src/vnet/srp/node.c index cec014f987d..cf9bd31c513 100644 --- a/src/vnet/srp/node.c +++ b/src/vnet/srp/node.c @@ -328,16 +328,24 @@ srp_topology_packet (vlib_main_t * vm, u32 sw_if_index, u8 ** contents) vec_len (*contents) - STRUCT_OFFSET_OF (srp_generic_control_header_t, control))); { - vlib_frame_t * f = vlib_get_frame_to_node (vm, hi->output_node_index); + vlib_frame_t * f; vlib_buffer_t * b; - u32 * to_next = vlib_frame_vector_args (f); - u32 bi; - - bi = vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX, - /* buffer to append to */ ~0, - *contents, vec_len (*contents)); + u32 * to_next; + u32 bi = ~0; + + if (vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX, + /* buffer to append to */ &bi, + *contents, vec_len (*contents))) + { + /* complete or partial buffer allocation failure */ + if (bi != ~0) + vlib_buffer_free (vm, &bi, 1); + return SRP_ERROR_CONTROL_PACKETS_PROCESSED; + } b = vlib_get_buffer (vm, bi); vnet_buffer (b)->sw_if_index[VLIB_RX] = vnet_buffer (b)->sw_if_index[VLIB_TX] = sw_if_index; + f = vlib_get_frame_to_node (vm, hi->output_node_index); + to_next = vlib_frame_vector_args (f); to_next[0] = bi; f->n_vectors = 1; vlib_put_frame_to_node (vm, hi->output_node_index, f); @@ -609,7 +617,7 @@ static void tx_ips_packet (srp_interface_t * si, vnet_hw_interface_t * hi = vnet_get_hw_interface (vnm, si->rings[tx_ring].hw_if_index); vlib_frame_t * f; vlib_buffer_t * b; - u32 * to_next, bi; + u32 * to_next, bi = ~0; if (! vnet_sw_interface_is_admin_up (vnm, hi->sw_if_index)) return; @@ -620,9 +628,15 @@ static void tx_ips_packet (srp_interface_t * si, = ~ip_csum_fold (ip_incremental_checksum (0, &i->control, sizeof (i[0]) - STRUCT_OFFSET_OF (srp_ips_header_t, control))); - bi = vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX, - /* buffer to append to */ ~0, - i, sizeof (i[0])); + if (vlib_buffer_add_data (vm, VLIB_BUFFER_DEFAULT_FREE_LIST_INDEX, + /* buffer to append to */ &bi, + i, sizeof (i[0]))) + { + /* complete or partial allocation failure */ + if (bi != ~0) + vlib_buffer_free (vm, &bi, 1); + return; + } /* FIXME trace. */ if (0) -- 2.16.6