timers: it is always reasonable to remove a timer

If struct timer_list has not been setup, it is zeroed, in which case
timer_pending is false, so calling del_timer is safe. Calling del_timer
is also safe on a timer that has already been del_timer'd. And calling
del_timer is safe after a peer is dead, since the whole point of it
being dead is that no more timers are created and all contexts
eventually stop. Finally del_timer uses a lock, which means it's safe to
call it concurrently.

Therefore, we do not need any guards around calls to del_timer. While
we're at it, we can get rid of the old lingering timers_enabled boolean
which wasn't doing anything anyway anymore.

3 files changed, 6 insertions, 17 deletions

diff --git a/src/device.c b/src/device.c
index f47ed0f..010fb28 100644
--- a/src/device.c
+++ b/src/device.c
@@ -80,10 +80,9 @@ static int wg_pm_notification(struct notifier_block *nb, unsigned long action,
 	list_for_each_entry(wg, &device_list, device_list) {
 		mutex_lock(&wg->device_update_lock);
 		list_for_each_entry(peer, &wg->peer_list, peer_list) {
+			del_timer(&peer->timer_zero_key_material);
 			wg_noise_handshake_clear(&peer->handshake);
 			wg_noise_keypairs_clear(&peer->keypairs);
-			if (peer->timers_enabled)
-				del_timer(&peer->timer_zero_key_material);
 		}
 		mutex_unlock(&wg->device_update_lock);
 	}
diff --git a/src/peer.h b/src/peer.h
index 10de4b6..d19e617 100644
--- a/src/peer.h
+++ b/src/peer.h
@@ -54,7 +54,7 @@ struct wg_peer {
 	struct timer_list timer_persistent_keepalive;
 	unsigned int timer_handshake_attempts;
 	u16 persistent_keepalive_interval;
-	bool timers_enabled, timer_need_another_keepalive;
+	bool timer_need_another_keepalive;
 	bool sent_lastminute_handshake;
 	struct timespec walltime_last_handshake;
 	struct kref refcount;
diff --git a/src/timers.c b/src/timers.c
index d2697b5..a9bcd36 100644
--- a/src/timers.c
+++ b/src/timers.c
@@ -37,16 +37,6 @@ static inline void mod_peer_timer(struct wg_peer *peer,
 	rcu_read_unlock_bh();
 }
 
-static inline void del_peer_timer(struct wg_peer *peer,
-				  struct timer_list *timer)
-{
-	rcu_read_lock_bh();
-	if (likely(netif_running(peer->device->dev) &&
-		   !READ_ONCE(peer->is_dead)))
-		del_timer(timer);
-	rcu_read_unlock_bh();
-}
-
 static void wg_expired_retransmit_handshake(struct timer_list *timer)
 {
 	struct wg_peer *peer = from_timer(peer, timer,
@@ -57,7 +47,7 @@ static void wg_expired_retransmit_handshake(struct timer_list *timer)
 			 peer->device->dev->name, peer->internal_id,
 			 &peer->endpoint.addr, MAX_TIMER_HANDSHAKES + 2);
 
-		del_peer_timer(peer, &peer->timer_send_keepalive);
+		del_timer(&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.
 		 */
@@ -175,7 +165,7 @@ void wg_timers_data_received(struct wg_peer *peer)
  */
 void wg_timers_any_authenticated_packet_sent(struct wg_peer *peer)
 {
-	del_peer_timer(peer, &peer->timer_send_keepalive);
+	del_timer(&peer->timer_send_keepalive);
 }
 
 /* Should be called after any type of authenticated packet is received, whether
@@ -183,7 +173,7 @@ void wg_timers_any_authenticated_packet_sent(struct wg_peer *peer)
  */
 void wg_timers_any_authenticated_packet_received(struct wg_peer *peer)
 {
-	del_peer_timer(peer, &peer->timer_new_handshake);
+	del_timer(&peer->timer_new_handshake);
 }
 
 /* Should be called after a handshake initiation message is sent. */
@@ -199,7 +189,7 @@ void wg_timers_handshake_initiated(struct wg_peer *peer)
  */
 void wg_timers_handshake_complete(struct wg_peer *peer)
 {
-	del_peer_timer(peer, &peer->timer_retransmit_handshake);
+	del_timer(&peer->timer_retransmit_handshake);
 	peer->timer_handshake_attempts = 0;
 	peer->sent_lastminute_handshake = false;
 	getnstimeofday(&peer->walltime_last_handshake);