From 81eb0e30f9b39e99d1bb7b56828fd32e50ea055a Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 1 Aug 2018 15:59:37 +0200 Subject: peer: ensure destruction doesn't race Completely rework peer removal to ensure peers don't jump between contexts and create races. --- src/timers.c | 60 ++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 26 deletions(-) (limited to 'src/timers.c') diff --git a/src/timers.c b/src/timers.c index e8bb101..762152a 100644 --- a/src/timers.c +++ b/src/timers.c @@ -27,9 +27,20 @@ if (unlikely(!peer)) \ return; -static inline bool timers_active(struct wireguard_peer *peer) +static inline void mod_peer_timer(struct wireguard_peer *peer, struct timer_list *timer, unsigned long expires) { - return netif_running(peer->device->dev) && !list_empty(&peer->peer_list); + rcu_read_lock_bh(); + if (likely(netif_running(peer->device->dev) && !peer->is_dead)) + mod_timer(timer, expires); + rcu_read_unlock_bh(); +} + +static inline void del_peer_timer(struct wireguard_peer *peer, struct timer_list *timer) +{ + rcu_read_lock_bh(); + if (likely(netif_running(peer->device->dev) && !peer->is_dead)) + del_timer(timer); + rcu_read_unlock_bh(); } static void expired_retransmit_handshake(struct timer_list *timer) @@ -39,8 +50,7 @@ static void expired_retransmit_handshake(struct timer_list *timer) if (peer->timer_handshake_attempts > MAX_TIMER_HANDSHAKES) { pr_debug("%s: Handshake for peer %llu (%pISpfsc) did not complete after %d attempts, giving up\n", peer->device->dev->name, peer->internal_id, &peer->endpoint.addr, MAX_TIMER_HANDSHAKES + 2); - if (likely(timers_active(peer))) - del_timer(&peer->timer_send_keepalive); + del_peer_timer(peer, &peer->timer_send_keepalive); /* We drop all packets without a keypair and don't try again, * if we try unsuccessfully for too long to make a handshake. */ @@ -49,8 +59,8 @@ static void expired_retransmit_handshake(struct timer_list *timer) /* We set a timer for destroying any residue that might be left * of a partial exchange. */ - if (likely(timers_active(peer)) && !timer_pending(&peer->timer_zero_key_material)) - mod_timer(&peer->timer_zero_key_material, jiffies + REJECT_AFTER_TIME * 3 * HZ); + if (!timer_pending(&peer->timer_zero_key_material)) + mod_peer_timer(peer, &peer->timer_zero_key_material, jiffies + REJECT_AFTER_TIME * 3 * HZ); } else { ++peer->timer_handshake_attempts; pr_debug("%s: Handshake for peer %llu (%pISpfsc) did not complete after %d seconds, retrying (try %d)\n", peer->device->dev->name, peer->internal_id, &peer->endpoint.addr, REKEY_TIMEOUT, peer->timer_handshake_attempts + 1); @@ -70,8 +80,7 @@ static void expired_send_keepalive(struct timer_list *timer) packet_send_keepalive(peer); if (peer->timer_need_another_keepalive) { peer->timer_need_another_keepalive = false; - if (likely(timers_active(peer))) - mod_timer(&peer->timer_send_keepalive, jiffies + KEEPALIVE_TIMEOUT * HZ); + mod_peer_timer(peer, &peer->timer_send_keepalive, jiffies + KEEPALIVE_TIMEOUT * HZ); } peer_put(peer); } @@ -91,8 +100,12 @@ static void expired_zero_key_material(struct timer_list *timer) { peer_get_from_timer(timer_zero_key_material); - if (!queue_work(peer->device->handshake_send_wq, &peer->clear_peer_work)) /* Takes our reference. */ - peer_put(peer); /* If the work was already on the queue, we want to drop the extra reference */ + rcu_read_lock_bh(); + if (!peer->is_dead) { + if (!queue_work(peer->device->handshake_send_wq, &peer->clear_peer_work)) /* Should take our reference. */ + peer_put(peer); /* If the work was already on the queue, we want to drop the extra reference */ + } + rcu_read_unlock_bh(); } static void queued_expired_zero_key_material(struct work_struct *work) { @@ -116,16 +129,16 @@ static void expired_send_persistent_keepalive(struct timer_list *timer) /* Should be called after an authenticated data packet is sent. */ void timers_data_sent(struct wireguard_peer *peer) { - if (likely(timers_active(peer)) && !timer_pending(&peer->timer_new_handshake)) - mod_timer(&peer->timer_new_handshake, jiffies + (KEEPALIVE_TIMEOUT + REKEY_TIMEOUT) * HZ); + if (!timer_pending(&peer->timer_new_handshake)) + mod_peer_timer(peer, &peer->timer_new_handshake, jiffies + (KEEPALIVE_TIMEOUT + REKEY_TIMEOUT) * HZ); } /* Should be called after an authenticated data packet is received. */ void timers_data_received(struct wireguard_peer *peer) { - if (likely(timers_active(peer))) { + if (likely(netif_running(peer->device->dev))) { if (!timer_pending(&peer->timer_send_keepalive)) - mod_timer(&peer->timer_send_keepalive, jiffies + KEEPALIVE_TIMEOUT * HZ); + mod_peer_timer(peer, &peer->timer_send_keepalive, jiffies + KEEPALIVE_TIMEOUT * HZ); else peer->timer_need_another_keepalive = true; } @@ -134,29 +147,25 @@ void timers_data_received(struct wireguard_peer *peer) /* Should be called after any type of authenticated packet is sent -- keepalive, data, or handshake. */ void timers_any_authenticated_packet_sent(struct wireguard_peer *peer) { - if (likely(timers_active(peer))) - del_timer(&peer->timer_send_keepalive); + del_peer_timer(peer, &peer->timer_send_keepalive); } /* Should be called after any type of authenticated packet is received -- keepalive, data, or handshake. */ void timers_any_authenticated_packet_received(struct wireguard_peer *peer) { - if (likely(timers_active(peer))) - del_timer(&peer->timer_new_handshake); + del_peer_timer(peer, &peer->timer_new_handshake); } /* Should be called after a handshake initiation message is sent. */ void timers_handshake_initiated(struct wireguard_peer *peer) { - if (likely(timers_active(peer))) - mod_timer(&peer->timer_retransmit_handshake, jiffies + REKEY_TIMEOUT * HZ + prandom_u32_max(REKEY_TIMEOUT_JITTER_MAX_JIFFIES)); + mod_peer_timer(peer, &peer->timer_retransmit_handshake, jiffies + REKEY_TIMEOUT * HZ + prandom_u32_max(REKEY_TIMEOUT_JITTER_MAX_JIFFIES)); } /* Should be called after a handshake response message is received and processed or when getting key confirmation via the first data message. */ void timers_handshake_complete(struct wireguard_peer *peer) { - if (likely(timers_active(peer))) - del_timer(&peer->timer_retransmit_handshake); + del_peer_timer(peer, &peer->timer_retransmit_handshake); peer->timer_handshake_attempts = 0; peer->sent_lastminute_handshake = false; getnstimeofday(&peer->walltime_last_handshake); @@ -165,15 +174,14 @@ void timers_handshake_complete(struct wireguard_peer *peer) /* Should be called after an ephemeral key is created, which is before sending a handshake response or after receiving a handshake response. */ void timers_session_derived(struct wireguard_peer *peer) { - if (likely(timers_active(peer))) - mod_timer(&peer->timer_zero_key_material, jiffies + REJECT_AFTER_TIME * 3 * HZ); + mod_peer_timer(peer, &peer->timer_zero_key_material, jiffies + REJECT_AFTER_TIME * 3 * HZ); } /* Should be called before a packet with authentication -- keepalive, data, or handshake -- is sent, or after one is received. */ void timers_any_authenticated_packet_traversal(struct wireguard_peer *peer) { - if (peer->persistent_keepalive_interval && likely(timers_active(peer))) - mod_timer(&peer->timer_persistent_keepalive, jiffies + peer->persistent_keepalive_interval * HZ); + if (peer->persistent_keepalive_interval) + mod_peer_timer(peer, &peer->timer_persistent_keepalive, jiffies + peer->persistent_keepalive_interval * HZ); } void timers_init(struct wireguard_peer *peer) -- cgit v1.2.3-59-g8ed1b