[PATCH net] wireguard: mark racy persistent_keepalive_interval accesses

[PATCH net] wireguard: mark racy persistent_keepalive_interval accesses

[Nguyen Ngoc Thang]

wg_set_device_doit() updates peer->persistent_keepalive_interval under
device_update_lock, while the RX/TX paths read it locklessly in
wg_timers_any_authenticated_packet_traversal() and in the persistent
keepalive timer. KCSAN reports the race:

  BUG: KCSAN: data-race in set_peer / wg_timers_any_authenticated_packet_traversal

  write to ... by task 3648 on cpu 1:
   set_peer+0x...  drivers/net/wireguard/netlink.c:457
   wg_set_device_doit+0x...
  read to ... by task 3652 on cpu 0:
   wg_timers_any_authenticated_packet_traversal+0x...  timers.c:217
   wg_packet_consume_data_done+0x...
   wg_packet_rx_poll+0x...

The race is benign, but the reader also loads the field twice, so the
zero check and the timer expiry computation can see different values.
Load it once with READ_ONCE() and mark the writer and the other
lockless reader accordingly.

Reported-by: syzbot+de9701a2cad597c14d34@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=de9701a2cad597c14d34
Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
 drivers/net/wireguard/netlink.c | 3 ++-
 drivers/net/wireguard/timers.c  | 8 +++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireguard/netlink.c b/drivers/net/wireguard/netlink.c
index 1da7e98d0d50..fafe2900c92b 100644
--- a/drivers/net/wireguard/netlink.c
+++ b/drivers/net/wireguard/netlink.c
@@ -454,7 +454,8 @@ static int set_peer(struct wg_device *wg, struct nlattr **attrs)
 			persistent_keepalive_interval &&
 			netif_running(wg->dev);
 
-		peer->persistent_keepalive_interval = persistent_keepalive_interval;
+		WRITE_ONCE(peer->persistent_keepalive_interval,
+			   persistent_keepalive_interval);
 		if (send_keepalive)
 			wg_packet_send_keepalive(peer);
 	}
diff --git a/drivers/net/wireguard/timers.c b/drivers/net/wireguard/timers.c
index 4016a3065602..c4697e8522b3 100644
--- a/drivers/net/wireguard/timers.c
+++ b/drivers/net/wireguard/timers.c
@@ -140,7 +140,7 @@ static void wg_expired_send_persistent_keepalive(struct timer_list *timer)
 	struct wg_peer *peer = timer_container_of(peer, timer,
 						  timer_persistent_keepalive);
 
-	if (likely(peer->persistent_keepalive_interval))
+	if (likely(READ_ONCE(peer->persistent_keepalive_interval)))
 		wg_packet_send_keepalive(peer);
 }
 
@@ -214,9 +214,11 @@ void wg_timers_session_derived(struct wg_peer *peer)
  */
 void wg_timers_any_authenticated_packet_traversal(struct wg_peer *peer)
 {
-	if (peer->persistent_keepalive_interval)
+	u16 interval = READ_ONCE(peer->persistent_keepalive_interval);
+
+	if (interval)
 		mod_peer_timer(peer, &peer->timer_persistent_keepalive,
-			jiffies + peer->persistent_keepalive_interval * HZ);
+			jiffies + interval * HZ);
 }
 
 void wg_timers_init(struct wg_peer *peer)
-- 
2.43.0