From efbb2e09ffebb712903070c7f89c2796597a6043 Mon Sep 17 00:00:00 2001 From: mvs Date: Tue, 5 Mar 2024 17:48:01 +0000 Subject: [PATCH] Convert `t_lock', `r_keypair_lock' and `c_lock' rwlock(9)s to corresponding mutex(9)es. ifq_start() and following wg_qstart() could be called from software interrupt context if bandwidth control is enabled in pf.conf(5). Remove sleep points provided by rwlock(9)s from wg(4) output start routine. looks ok claudio --- sys/net/if_wg.c | 48 +++++++++++++++++++++++----------------------- sys/net/wg_noise.c | 44 ++++++++++++++++++++---------------------- sys/net/wg_noise.h | 7 ++++--- 3 files changed, 49 insertions(+), 50 deletions(-) diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c index a35f43510ab..9a7d00aac32 100644 --- a/sys/net/if_wg.c +++ b/sys/net/if_wg.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_wg.c,v 1.36 2024/01/18 08:46:41 mvs Exp $ */ +/* $OpenBSD: if_wg.c,v 1.37 2024/03/05 17:48:01 mvs Exp $ */ /* * Copyright (C) 2015-2020 Jason A. Donenfeld . All Rights Reserved. @@ -150,8 +150,8 @@ struct wg_index { }; struct wg_timers { - /* t_lock is for blocking wg_timers_event_* when setting t_disabled. */ - struct rwlock t_lock; + /* t_mtx is for blocking wg_timers_event_* when setting t_disabled. */ + struct mutex t_mtx; int t_disabled; int t_need_another_keepalive; @@ -930,7 +930,7 @@ void wg_timers_init(struct wg_timers *t) { bzero(t, sizeof(*t)); - rw_init(&t->t_lock, "wg_timers"); + mtx_init_flags(&t->t_mtx, IPL_NET, "wg_timers", 0); mtx_init(&t->t_handshake_mtx, IPL_NET); timeout_set(&t->t_new_handshake, wg_timers_run_new_handshake, t); @@ -945,19 +945,19 @@ wg_timers_init(struct wg_timers *t) void wg_timers_enable(struct wg_timers *t) { - rw_enter_write(&t->t_lock); + mtx_enter(&t->t_mtx); t->t_disabled = 0; - rw_exit_write(&t->t_lock); + mtx_leave(&t->t_mtx); wg_timers_run_persistent_keepalive(t); } void wg_timers_disable(struct wg_timers *t) { - rw_enter_write(&t->t_lock); + mtx_enter(&t->t_mtx); t->t_disabled = 1; t->t_need_another_keepalive = 0; - rw_exit_write(&t->t_lock); + mtx_leave(&t->t_mtx); timeout_del_barrier(&t->t_new_handshake); timeout_del_barrier(&t->t_send_keepalive); @@ -969,12 +969,12 @@ wg_timers_disable(struct wg_timers *t) void wg_timers_set_persistent_keepalive(struct wg_timers *t, uint16_t interval) { - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled) { t->t_persistent_keepalive_interval = interval; wg_timers_run_persistent_keepalive(t); } - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } int @@ -1020,16 +1020,16 @@ wg_timers_event_data_sent(struct wg_timers *t) int msecs = NEW_HANDSHAKE_TIMEOUT * 1000; msecs += arc4random_uniform(REKEY_TIMEOUT_JITTER); - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled && !timeout_pending(&t->t_new_handshake)) timeout_add_msec(&t->t_new_handshake, msecs); - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } void wg_timers_event_data_received(struct wg_timers *t) { - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled) { if (!timeout_pending(&t->t_send_keepalive)) timeout_add_sec(&t->t_send_keepalive, @@ -1037,7 +1037,7 @@ wg_timers_event_data_received(struct wg_timers *t) else t->t_need_another_keepalive = 1; } - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } void @@ -1055,11 +1055,11 @@ wg_timers_event_any_authenticated_packet_received(struct wg_timers *t) void wg_timers_event_any_authenticated_packet_traversal(struct wg_timers *t) { - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled && t->t_persistent_keepalive_interval > 0) timeout_add_sec(&t->t_persistent_keepalive, t->t_persistent_keepalive_interval); - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } void @@ -1068,10 +1068,10 @@ wg_timers_event_handshake_initiated(struct wg_timers *t) int msecs = REKEY_TIMEOUT * 1000; msecs += arc4random_uniform(REKEY_TIMEOUT_JITTER); - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled) timeout_add_msec(&t->t_retry_handshake, msecs); - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } void @@ -1085,7 +1085,7 @@ wg_timers_event_handshake_responded(struct wg_timers *t) void wg_timers_event_handshake_complete(struct wg_timers *t) { - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled) { mtx_enter(&t->t_handshake_mtx); timeout_del(&t->t_retry_handshake); @@ -1094,25 +1094,25 @@ wg_timers_event_handshake_complete(struct wg_timers *t) mtx_leave(&t->t_handshake_mtx); wg_timers_run_send_keepalive(t); } - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } void wg_timers_event_session_derived(struct wg_timers *t) { - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled) timeout_add_sec(&t->t_zero_key_material, REJECT_AFTER_TIME * 3); - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } void wg_timers_event_want_initiation(struct wg_timers *t) { - rw_enter_read(&t->t_lock); + mtx_enter(&t->t_mtx); if (!t->t_disabled) wg_timers_run_send_initiation(t, 0); - rw_exit_read(&t->t_lock); + mtx_leave(&t->t_mtx); } void diff --git a/sys/net/wg_noise.c b/sys/net/wg_noise.c index 2a1954d94cc..c4d64baeeec 100644 --- a/sys/net/wg_noise.c +++ b/sys/net/wg_noise.c @@ -1,4 +1,4 @@ -/* $OpenBSD: wg_noise.c,v 1.6 2023/02/03 18:31:17 miod Exp $ */ +/* $OpenBSD: wg_noise.c,v 1.7 2024/03/05 17:48:01 mvs Exp $ */ /* * Copyright (C) 2015-2020 Jason A. Donenfeld . All Rights Reserved. * Copyright (C) 2019-2020 Matt Dunwoodie @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -139,7 +140,7 @@ noise_remote_init(struct noise_remote *r, uint8_t public[NOISE_PUBLIC_KEY_LEN], bzero(r, sizeof(*r)); memcpy(r->r_public, public, NOISE_PUBLIC_KEY_LEN); rw_init(&r->r_handshake_lock, "noise_handshake"); - rw_init(&r->r_keypair_lock, "noise_keypair"); + mtx_init_flags(&r->r_keypair_mtx, IPL_NET, "noise_keypair", 0); SLIST_INSERT_HEAD(&r->r_unused_keypairs, &r->r_keypair[0], kp_entry); SLIST_INSERT_HEAD(&r->r_unused_keypairs, &r->r_keypair[1], kp_entry); @@ -468,10 +469,10 @@ noise_remote_begin_session(struct noise_remote *r) kp.kp_remote_index = hs->hs_remote_index; getnanouptime(&kp.kp_birthdate); bzero(&kp.kp_ctr, sizeof(kp.kp_ctr)); - rw_init(&kp.kp_ctr.c_lock, "noise_counter"); + mtx_init_flags(&kp.kp_ctr.c_mtx, IPL_NET, "noise_counter", 0); /* Now we need to add_new_keypair */ - rw_enter_write(&r->r_keypair_lock); + mtx_enter(&r->r_keypair_mtx); next = r->r_next; current = r->r_current; previous = r->r_previous; @@ -497,7 +498,7 @@ noise_remote_begin_session(struct noise_remote *r) r->r_next = noise_remote_keypair_allocate(r); *r->r_next = kp; } - rw_exit_write(&r->r_keypair_lock); + mtx_leave(&r->r_keypair_mtx); explicit_bzero(&r->r_handshake, sizeof(r->r_handshake)); rw_exit_write(&r->r_handshake_lock); @@ -514,25 +515,25 @@ noise_remote_clear(struct noise_remote *r) explicit_bzero(&r->r_handshake, sizeof(r->r_handshake)); rw_exit_write(&r->r_handshake_lock); - rw_enter_write(&r->r_keypair_lock); + mtx_enter(&r->r_keypair_mtx); noise_remote_keypair_free(r, r->r_next); noise_remote_keypair_free(r, r->r_current); noise_remote_keypair_free(r, r->r_previous); r->r_next = NULL; r->r_current = NULL; r->r_previous = NULL; - rw_exit_write(&r->r_keypair_lock); + mtx_leave(&r->r_keypair_mtx); } void noise_remote_expire_current(struct noise_remote *r) { - rw_enter_write(&r->r_keypair_lock); + mtx_enter(&r->r_keypair_mtx); if (r->r_next != NULL) r->r_next->kp_valid = 0; if (r->r_current != NULL) r->r_current->kp_valid = 0; - rw_exit_write(&r->r_keypair_lock); + mtx_leave(&r->r_keypair_mtx); } int @@ -541,7 +542,7 @@ noise_remote_ready(struct noise_remote *r) struct noise_keypair *kp; int ret; - rw_enter_read(&r->r_keypair_lock); + mtx_enter(&r->r_keypair_mtx); /* kp_ctr isn't locked here, we're happy to accept a racy read. */ if ((kp = r->r_current) == NULL || !kp->kp_valid || @@ -551,7 +552,7 @@ noise_remote_ready(struct noise_remote *r) ret = EINVAL; else ret = 0; - rw_exit_read(&r->r_keypair_lock); + mtx_leave(&r->r_keypair_mtx); return ret; } @@ -562,7 +563,7 @@ noise_remote_encrypt(struct noise_remote *r, uint32_t *r_idx, uint64_t *nonce, struct noise_keypair *kp; int ret = EINVAL; - rw_enter_read(&r->r_keypair_lock); + mtx_enter(&r->r_keypair_mtx); if ((kp = r->r_current) == NULL) goto error; @@ -601,7 +602,7 @@ noise_remote_encrypt(struct noise_remote *r, uint32_t *r_idx, uint64_t *nonce, ret = 0; error: - rw_exit_read(&r->r_keypair_lock); + mtx_leave(&r->r_keypair_mtx); return ret; } @@ -616,7 +617,7 @@ noise_remote_decrypt(struct noise_remote *r, uint32_t r_idx, uint64_t nonce, * attempt the current keypair first as that is most likely. We also * want to make sure that the keypair is valid as it would be * catastrophic to decrypt against a zero'ed keypair. */ - rw_enter_read(&r->r_keypair_lock); + mtx_enter(&r->r_keypair_mtx); if (r->r_current != NULL && r->r_current->kp_local_index == r_idx) { kp = r->r_current; @@ -651,8 +652,6 @@ noise_remote_decrypt(struct noise_remote *r, uint32_t r_idx, uint64_t nonce, * we skip the REKEY_AFTER_TIME_RECV check. This is safe to do as a * data packet can't confirm a session that we are an INITIATOR of. */ if (kp == r->r_next) { - rw_exit_read(&r->r_keypair_lock); - rw_enter_write(&r->r_keypair_lock); if (kp == r->r_next && kp->kp_local_index == r_idx) { noise_remote_keypair_free(r, r->r_previous); r->r_previous = r->r_current; @@ -662,7 +661,6 @@ noise_remote_decrypt(struct noise_remote *r, uint32_t r_idx, uint64_t nonce, ret = ECONNRESET; goto error; } - rw_enter(&r->r_keypair_lock, RW_DOWNGRADE); } /* Similar to when we encrypt, we want to notify the caller when we @@ -680,7 +678,7 @@ noise_remote_decrypt(struct noise_remote *r, uint32_t r_idx, uint64_t nonce, ret = 0; error: - rw_exit(&r->r_keypair_lock); + mtx_leave(&r->r_keypair_mtx); return ret; } @@ -731,9 +729,9 @@ noise_counter_send(struct noise_counter *ctr) return atomic_inc_long_nv((u_long *)&ctr->c_send) - 1; #else uint64_t ret; - rw_enter_write(&ctr->c_lock); + mtx_enter(&ctr->c_mtx); ret = ctr->c_send++; - rw_exit_write(&ctr->c_lock); + mtx_leave(&ctr->c_mtx); return ret; #endif } @@ -745,7 +743,7 @@ noise_counter_recv(struct noise_counter *ctr, uint64_t recv) unsigned long bit; int ret = EEXIST; - rw_enter_write(&ctr->c_lock); + mtx_enter(&ctr->c_mtx); /* Check that the recv counter is valid */ if (ctr->c_recv >= REJECT_AFTER_MESSAGES || @@ -779,7 +777,7 @@ noise_counter_recv(struct noise_counter *ctr, uint64_t recv) ret = 0; error: - rw_exit_write(&ctr->c_lock); + mtx_leave(&ctr->c_mtx); return ret; } @@ -976,7 +974,7 @@ noise_timer_expired(struct timespec *birthdate, time_t sec, long nsec) #define T_LIM (COUNTER_WINDOW_SIZE + 1) #define T_INIT do { \ bzero(&ctr, sizeof(ctr)); \ - rw_init(&ctr.c_lock, "counter"); \ + mtx_init_flags(&ctr.c_mtx, IPL_NET, "counter", 0); \ } while (0) #define T(num, v, e) do { \ if (noise_counter_recv(&ctr, v) != e) { \ diff --git a/sys/net/wg_noise.h b/sys/net/wg_noise.h index 5bcc156739c..5daedbc80d8 100644 --- a/sys/net/wg_noise.h +++ b/sys/net/wg_noise.h @@ -1,4 +1,4 @@ -/* $OpenBSD: wg_noise.h,v 1.2 2020/12/09 05:53:33 tb Exp $ */ +/* $OpenBSD: wg_noise.h,v 1.3 2024/03/05 17:48:01 mvs Exp $ */ /* * Copyright (C) 2015-2020 Jason A. Donenfeld . All Rights Reserved. * Copyright (C) 2019-2020 Matt Dunwoodie @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -71,7 +72,7 @@ struct noise_handshake { }; struct noise_counter { - struct rwlock c_lock; + struct mutex c_mtx; uint64_t c_send; uint64_t c_recv; unsigned long c_backtrack[COUNTER_NUM]; @@ -100,7 +101,7 @@ struct noise_remote { uint8_t r_timestamp[NOISE_TIMESTAMP_LEN]; struct timespec r_last_init; /* nanouptime */ - struct rwlock r_keypair_lock; + struct mutex r_keypair_mtx; SLIST_HEAD(,noise_keypair) r_unused_keypairs; struct noise_keypair *r_next, *r_current, *r_previous; struct noise_keypair r_keypair[3]; /* 3: next, current, previous. */ -- 2.20.1