peer: ensure destruction doesn't race

Completely rework peer removal to ensure peers don't jump between
contexts and create races.

1 files changed, 34 insertions, 26 deletions

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)