From: Gabriel Oginski Date: Thu, 10 Nov 2022 09:22:17 +0000 (+0000) Subject: wireguard: add local variable X-Git-Tag: v23.06-rc0~15 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=f4b82f52e8b0fcc59a4c3020724022a7bc184b1a;p=vpp.git wireguard: add local variable The current implementation of wireguard use dereference value from pointer, but between get and dereference the value from pointer can be occur change in pool memory, which means that this pointer can be invalid. Since current implementation doesn't handle with invalid pointers, segfault can occur. The fix add a local variable to keep index of peer from pool and also handle with null pointers from get pointer from pool. Type: fix Signed-off-by: Gabriel Oginski Change-Id: Ic161ab08266e584493338c682d827ea1fd754b98 --- diff --git a/src/plugins/wireguard/wireguard_input.c b/src/plugins/wireguard/wireguard_input.c index f4d9132d948..777f0ec54b3 100644 --- a/src/plugins/wireguard/wireguard_input.c +++ b/src/plugins/wireguard/wireguard_input.c @@ -343,9 +343,11 @@ wg_input_post_process (vlib_main_t *vm, vlib_buffer_t *b, u16 *next, bool *is_keepalive) { next[0] = WG_INPUT_NEXT_PUNT; + noise_keypair_t *kp; - noise_keypair_t *kp = - wg_get_active_keypair (&peer->remote, data->receiver_index); + if ((kp = wg_get_active_keypair (&peer->remote, data->receiver_index)) == + NULL) + return -1; if (!noise_counter_recv (&kp->kp_ctr, data->counter)) { @@ -632,6 +634,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node, bool is_keepalive = false; u32 *peer_idx = NULL; + index_t peeri = INDEX_INVALID; while (n_left_from > 0) { @@ -665,9 +668,15 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node, data->receiver_index); if (PREDICT_TRUE (peer_idx != NULL)) { - peer = wg_peer_get (*peer_idx); + peeri = *peer_idx; + peer = wg_peer_get (peeri); + last_rec_idx = data->receiver_index; + } + else + { + peer = NULL; + last_rec_idx = ~0; } - last_rec_idx = data->receiver_index; } if (PREDICT_FALSE (!peer_idx)) @@ -739,7 +748,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node, } else if (PREDICT_FALSE (state_cr == SC_KEEP_KEY_FRESH)) { - wg_send_handshake_from_mt (*peer_idx, false); + wg_send_handshake_from_mt (peeri, false); goto next; } else if (PREDICT_TRUE (state_cr == SC_OK)) @@ -780,7 +789,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node, t->type = header_type; t->current_length = b[0]->current_length; t->is_keepalive = is_keepalive; - t->peer = peer_idx ? *peer_idx : INDEX_INVALID; + t->peer = peer_idx ? peeri : INDEX_INVALID; } next: @@ -827,20 +836,37 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node, { peer_idx = wg_index_table_lookup (&wmp->index_table, data->receiver_index); - peer = wg_peer_get (*peer_idx); - last_rec_idx = data->receiver_index; + if (PREDICT_TRUE (peer_idx != NULL)) + { + peeri = *peer_idx; + peer = wg_peer_get (peeri); + last_rec_idx = data->receiver_index; + } + else + { + peer = NULL; + last_rec_idx = ~0; + } } - if (PREDICT_FALSE (wg_input_post_process (vm, b[0], data_next, peer, - data, &is_keepalive) < 0)) - goto trace; + if (PREDICT_TRUE (peer != NULL)) + { + if (PREDICT_FALSE (wg_input_post_process (vm, b[0], data_next, peer, + data, &is_keepalive) < 0)) + goto trace; + } + else + { + data_next[0] = WG_INPUT_NEXT_PUNT; + goto trace; + } if (PREDICT_FALSE (peer_idx && (last_peer_time_idx != peer_idx))) { if (PREDICT_FALSE ( !ip46_address_is_equal (&peer->dst.addr, &out_src_ip) || peer->dst.port != out_udp_src_port)) - wg_peer_update_endpoint_from_mt (*peer_idx, &out_src_ip, + wg_peer_update_endpoint_from_mt (peeri, &out_src_ip, out_udp_src_port); wg_timers_any_authenticated_packet_received_opt (peer, time); wg_timers_any_authenticated_packet_traversal (peer); @@ -860,7 +886,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node, t->type = header_type; t->current_length = b[0]->current_length; t->is_keepalive = is_keepalive; - t->peer = peer_idx ? *peer_idx : INDEX_INVALID; + t->peer = peer_idx ? peeri : INDEX_INVALID; } b += 1; @@ -922,6 +948,7 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame, wg_peer_t *peer = NULL; u32 *peer_idx = NULL; u32 *last_peer_time_idx = NULL; + index_t peeri = INDEX_INVALID; u32 last_rec_idx = ~0; f64 time = clib_time_now (&vm->clib_time) + vm->time_offset; @@ -955,8 +982,17 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame, peer_idx = wg_index_table_lookup (&wmp->index_table, data->receiver_index); - peer = wg_peer_get (*peer_idx); - last_rec_idx = data->receiver_index; + if (PREDICT_TRUE (peer_idx != NULL)) + { + peeri = *peer_idx; + peer = wg_peer_get (peeri); + last_rec_idx = data->receiver_index; + } + else + { + peer = NULL; + last_rec_idx = ~0; + } } if (PREDICT_TRUE (peer != NULL)) @@ -976,7 +1012,7 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame, if (PREDICT_FALSE ( !ip46_address_is_equal (&peer->dst.addr, &out_src_ip) || peer->dst.port != out_udp_src_port)) - wg_peer_update_endpoint_from_mt (*peer_idx, &out_src_ip, + wg_peer_update_endpoint_from_mt (peeri, &out_src_ip, out_udp_src_port); wg_timers_any_authenticated_packet_received_opt (peer, time); wg_timers_any_authenticated_packet_traversal (peer); @@ -995,7 +1031,7 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame, wg_input_post_trace_t *t = vlib_add_trace (vm, node, b[0], sizeof (*t)); t->next = next[0]; - t->peer = peer_idx ? *peer_idx : INDEX_INVALID; + t->peer = peer_idx ? peeri : INDEX_INVALID; } b += 1;