From 9ad423fceb8d9877b337ada5fc1e053de21323b2 Mon Sep 17 00:00:00 2001 From: Gabriel Oginski Date: Tue, 21 Feb 2023 08:42:06 +0000 Subject: wireguard: add barrier to sync data The current implmentation of the hash table is not thread-safe. This design leads to a segfault when VPP is handling a lot of tunnels for Wireguard, where one thread modifies the hash table and other threads start the lookup at the same time. This fix adds a barrier sync to the hash table access when Wireguard adds or deletes an element. Type: fix Signed-off-by: Gabriel Oginski Change-Id: Id460dfcd46ace17c7bdcd23bd9687d26cecf0a39 --- src/plugins/wireguard/wireguard_if.c | 8 +++---- src/plugins/wireguard/wireguard_index_table.c | 14 +++++++++--- src/plugins/wireguard/wireguard_index_table.h | 7 +++--- src/plugins/wireguard/wireguard_noise.c | 32 ++++++++++++++------------- src/plugins/wireguard/wireguard_noise.h | 10 ++++----- src/plugins/wireguard/wireguard_peer.c | 4 ++-- 6 files changed, 43 insertions(+), 32 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/wireguard/wireguard_if.c b/src/plugins/wireguard/wireguard_if.c index a869df08ce2..ee744e7c3e4 100644 --- a/src/plugins/wireguard/wireguard_if.c +++ b/src/plugins/wireguard/wireguard_if.c @@ -116,20 +116,20 @@ wg_remote_get (const uint8_t public[NOISE_PUBLIC_KEY_LEN]) } static uint32_t -wg_index_set (noise_remote_t * remote) +wg_index_set (vlib_main_t *vm, noise_remote_t *remote) { wg_main_t *wmp = &wg_main; u32 rnd_seed = (u32) (vlib_time_now (wmp->vlib_main) * 1e6); u32 ret = - wg_index_table_add (&wmp->index_table, remote->r_peer_idx, rnd_seed); + wg_index_table_add (vm, &wmp->index_table, remote->r_peer_idx, rnd_seed); return ret; } static void -wg_index_drop (uint32_t key) +wg_index_drop (vlib_main_t *vm, uint32_t key) { wg_main_t *wmp = &wg_main; - wg_index_table_del (&wmp->index_table, key); + wg_index_table_del (vm, &wmp->index_table, key); } static clib_error_t * diff --git a/src/plugins/wireguard/wireguard_index_table.c b/src/plugins/wireguard/wireguard_index_table.c index 5f81204b4c0..da53bfd75f1 100644 --- a/src/plugins/wireguard/wireguard_index_table.c +++ b/src/plugins/wireguard/wireguard_index_table.c @@ -13,13 +13,15 @@ * limitations under the License. */ +#include #include #include #include #include u32 -wg_index_table_add (wg_index_table_t * table, u32 peer_pool_idx, u32 rnd_seed) +wg_index_table_add (vlib_main_t *vm, wg_index_table_t *table, + u32 peer_pool_idx, u32 rnd_seed) { u32 key; @@ -29,19 +31,25 @@ wg_index_table_add (wg_index_table_t * table, u32 peer_pool_idx, u32 rnd_seed) if (hash_get (table->hash, key)) continue; + vlib_worker_thread_barrier_sync (vm); hash_set (table->hash, key, peer_pool_idx); + vlib_worker_thread_barrier_release (vm); break; } return key; } void -wg_index_table_del (wg_index_table_t * table, u32 key) +wg_index_table_del (vlib_main_t *vm, wg_index_table_t *table, u32 key) { uword *p; p = hash_get (table->hash, key); if (p) - hash_unset (table->hash, key); + { + vlib_worker_thread_barrier_sync (vm); + hash_unset (table->hash, key); + vlib_worker_thread_barrier_release (vm); + } } u32 * diff --git a/src/plugins/wireguard/wireguard_index_table.h b/src/plugins/wireguard/wireguard_index_table.h index 67cae1f49d5..e9aa374c0ca 100644 --- a/src/plugins/wireguard/wireguard_index_table.h +++ b/src/plugins/wireguard/wireguard_index_table.h @@ -16,6 +16,7 @@ #ifndef __included_wg_index_table_h__ #define __included_wg_index_table_h__ +#include #include typedef struct @@ -23,9 +24,9 @@ typedef struct uword *hash; } wg_index_table_t; -u32 wg_index_table_add (wg_index_table_t * table, u32 peer_pool_idx, - u32 rnd_seed); -void wg_index_table_del (wg_index_table_t * table, u32 key); +u32 wg_index_table_add (vlib_main_t *vm, wg_index_table_t *table, + u32 peer_pool_idx, u32 rnd_seed); +void wg_index_table_del (vlib_main_t *vm, wg_index_table_t *table, u32 key); u32 *wg_index_table_lookup (const wg_index_table_t * table, u32 key); #endif //__included_wg_index_table_h__ diff --git a/src/plugins/wireguard/wireguard_noise.c b/src/plugins/wireguard/wireguard_noise.c index c9d8e31061a..19b0ce5a58c 100644 --- a/src/plugins/wireguard/wireguard_noise.c +++ b/src/plugins/wireguard/wireguard_noise.c @@ -33,8 +33,10 @@ noise_local_t *noise_local_pool; static noise_keypair_t *noise_remote_keypair_allocate (noise_remote_t *); static void noise_remote_keypair_free (vlib_main_t * vm, noise_remote_t *, noise_keypair_t **); -static uint32_t noise_remote_handshake_index_get (noise_remote_t *); -static void noise_remote_handshake_index_drop (noise_remote_t *); +static uint32_t noise_remote_handshake_index_get (vlib_main_t *vm, + noise_remote_t *); +static void noise_remote_handshake_index_drop (vlib_main_t *vm, + noise_remote_t *); static uint64_t noise_counter_send (noise_counter_t *); bool noise_counter_recv (noise_counter_t *, uint64_t); @@ -86,7 +88,7 @@ noise_local_set_private (noise_local_t * l, } void -noise_remote_init (noise_remote_t * r, uint32_t peer_pool_idx, +noise_remote_init (vlib_main_t *vm, noise_remote_t *r, uint32_t peer_pool_idx, const uint8_t public[NOISE_PUBLIC_KEY_LEN], u32 noise_local_idx) { @@ -97,18 +99,18 @@ noise_remote_init (noise_remote_t * r, uint32_t peer_pool_idx, r->r_local_idx = noise_local_idx; r->r_handshake.hs_state = HS_ZEROED; - noise_remote_precompute (r); + noise_remote_precompute (vm, r); } void -noise_remote_precompute (noise_remote_t * r) +noise_remote_precompute (vlib_main_t *vm, noise_remote_t *r) { noise_local_t *l = noise_local_get (r->r_local_idx); if (!curve25519_gen_shared (r->r_ss, l->l_private, r->r_public)) clib_memset (r->r_ss, 0, NOISE_PUBLIC_KEY_LEN); - noise_remote_handshake_index_drop (r); + noise_remote_handshake_index_drop (vm, r); wg_secure_zero_memory (&r->r_handshake, sizeof (r->r_handshake)); } @@ -154,9 +156,9 @@ noise_create_initiation (vlib_main_t * vm, noise_remote_t * r, /* {t} */ noise_tai64n_now (ets); noise_msg_encrypt (vm, ets, ets, NOISE_TIMESTAMP_LEN, key_idx, hs->hs_hash); - noise_remote_handshake_index_drop (r); + noise_remote_handshake_index_drop (vm, r); hs->hs_state = CREATED_INITIATION; - hs->hs_local_index = noise_remote_handshake_index_get (r); + hs->hs_local_index = noise_remote_handshake_index_get (vm, r); *s_idx = hs->hs_local_index; ret = true; error: @@ -237,7 +239,7 @@ noise_consume_initiation (vlib_main_t * vm, noise_local_t * l, goto error; /* Ok, we're happy to accept this initiation now */ - noise_remote_handshake_index_drop (r); + noise_remote_handshake_index_drop (vm, r); r->r_handshake = hs; *rp = r; ret = true; @@ -291,7 +293,7 @@ noise_create_response (vlib_main_t * vm, noise_remote_t * r, uint32_t * s_idx, hs->hs_state = CREATED_RESPONSE; - hs->hs_local_index = noise_remote_handshake_index_get (r); + hs->hs_local_index = noise_remote_handshake_index_get (vm, r); *r_idx = hs->hs_remote_index; *s_idx = hs->hs_local_index; ret = true; @@ -451,7 +453,7 @@ noise_remote_begin_session (vlib_main_t * vm, noise_remote_t * r) void noise_remote_clear (vlib_main_t * vm, noise_remote_t * r) { - noise_remote_handshake_index_drop (r); + noise_remote_handshake_index_drop (vm, r); wg_secure_zero_memory (&r->r_handshake, sizeof (r->r_handshake)); clib_rwlock_writer_lock (&r->r_keypair_lock); @@ -555,21 +557,21 @@ noise_remote_keypair_allocate (noise_remote_t * r) } static uint32_t -noise_remote_handshake_index_get (noise_remote_t * r) +noise_remote_handshake_index_get (vlib_main_t *vm, noise_remote_t *r) { noise_local_t *local = noise_local_get (r->r_local_idx); struct noise_upcall *u = &local->l_upcall; - return u->u_index_set (r); + return u->u_index_set (vm, r); } static void -noise_remote_handshake_index_drop (noise_remote_t * r) +noise_remote_handshake_index_drop (vlib_main_t *vm, noise_remote_t *r) { noise_handshake_t *hs = &r->r_handshake; noise_local_t *local = noise_local_get (r->r_local_idx); struct noise_upcall *u = &local->l_upcall; if (hs->hs_state != HS_ZEROED) - u->u_index_drop (hs->hs_local_index); + u->u_index_drop (vm, hs->hs_local_index); } static void diff --git a/src/plugins/wireguard/wireguard_noise.h b/src/plugins/wireguard/wireguard_noise.h index e95211b8884..fd2c09ebfa5 100644 --- a/src/plugins/wireguard/wireguard_noise.h +++ b/src/plugins/wireguard/wireguard_noise.h @@ -121,8 +121,8 @@ typedef struct noise_local { void *u_arg; noise_remote_t *(*u_remote_get) (const uint8_t[NOISE_PUBLIC_KEY_LEN]); - uint32_t (*u_index_set) (noise_remote_t *); - void (*u_index_drop) (uint32_t); + uint32_t (*u_index_set) (vlib_main_t *, noise_remote_t *); + void (*u_index_drop) (vlib_main_t *, uint32_t); } l_upcall; } noise_local_t; @@ -148,11 +148,11 @@ void noise_local_init (noise_local_t *, struct noise_upcall *); bool noise_local_set_private (noise_local_t *, const uint8_t[NOISE_PUBLIC_KEY_LEN]); -void noise_remote_init (noise_remote_t *, uint32_t, +void noise_remote_init (vlib_main_t *, noise_remote_t *, uint32_t, const uint8_t[NOISE_PUBLIC_KEY_LEN], uint32_t); /* Should be called anytime noise_local_set_private is called */ -void noise_remote_precompute (noise_remote_t *); +void noise_remote_precompute (vlib_main_t *, noise_remote_t *); /* Cryptographic functions */ bool noise_create_initiation (vlib_main_t * vm, noise_remote_t *, @@ -266,7 +266,7 @@ noise_remote_keypair_free (vlib_main_t *vm, noise_remote_t *r, struct noise_upcall *u = &local->l_upcall; if (*kp) { - u->u_index_drop ((*kp)->kp_local_index); + u->u_index_drop (vm, (*kp)->kp_local_index); vnet_crypto_key_del (vm, (*kp)->kp_send_index); vnet_crypto_key_del (vm, (*kp)->kp_recv_index); clib_mem_free (*kp); diff --git a/src/plugins/wireguard/wireguard_peer.c b/src/plugins/wireguard/wireguard_peer.c index f7bf2352db4..32a92da102b 100644 --- a/src/plugins/wireguard/wireguard_peer.c +++ b/src/plugins/wireguard/wireguard_peer.c @@ -242,7 +242,7 @@ wg_peer_enable (vlib_main_t *vm, wg_peer_t *peer) wg_if = wg_if_get (wg_if_find_by_sw_if_index (peer->wg_sw_if_index)); clib_memcpy (public_key, peer->remote.r_public, NOISE_PUBLIC_KEY_LEN); - noise_remote_init (&peer->remote, peeri, public_key, wg_if->local_idx); + noise_remote_init (vm, &peer->remote, peeri, public_key, wg_if->local_idx); wg_timers_send_first_handshake (peer); } @@ -484,7 +484,7 @@ wg_peer_add (u32 tun_sw_if_index, const u8 public_key[NOISE_PUBLIC_KEY_LEN], return (rv); } - noise_remote_init (&peer->remote, peer - wg_peer_pool, public_key, + noise_remote_init (vm, &peer->remote, peer - wg_peer_pool, public_key, wg_if->local_idx); cookie_maker_init (&peer->cookie_maker, public_key); -- cgit 1.2.3-korg