From 6fedf35005ddeec21a73fb299e1f5a2cd772f96b Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Thu, 25 Oct 2018 14:49:32 +0200 Subject: global: do not allow compiler to reorder is_valid or is_dead Suggested-by: Jann Horn --- src/noise.c | 4 ++-- src/receive.c | 8 ++++---- src/send.c | 11 ++++++----- src/timers.c | 8 +++++--- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/noise.c b/src/noise.c index 4160fc0..d0a337a 100644 --- a/src/noise.c +++ b/src/noise.c @@ -775,8 +775,8 @@ bool wg_noise_handshake_begin_session(struct noise_handshake *handshake, handshake_zero(handshake); rcu_read_lock_bh(); - if (likely(!container_of(handshake, struct wg_peer, - handshake)->is_dead)) { + if (likely(!READ_ONCE(container_of(handshake, struct wg_peer, + handshake)->is_dead))) { add_new_keypair(keypairs, new_keypair); net_dbg_ratelimited("%s: Keypair %llu created for peer %llu\n", handshake->entry.peer->device->dev->name, diff --git a/src/receive.c b/src/receive.c index 33a60b7..d5bce92 100644 --- a/src/receive.c +++ b/src/receive.c @@ -231,7 +231,7 @@ static void keep_key_fresh(struct wg_peer *peer) rcu_read_lock_bh(); keypair = rcu_dereference_bh(peer->keypairs.current_keypair); - if (likely(keypair && keypair->sending.is_valid) && + if (likely(keypair && READ_ONCE(keypair->sending.is_valid)) && keypair->i_am_the_initiator && unlikely(wg_birthdate_has_expired(keypair->sending.birthdate, REJECT_AFTER_TIME - KEEPALIVE_TIMEOUT - REKEY_TIMEOUT))) @@ -255,10 +255,10 @@ static bool decrypt_packet(struct sk_buff *skb, struct noise_symmetric_key *key, if (unlikely(!key)) return false; - if (unlikely(!key->is_valid || + if (unlikely(!READ_ONCE(key->is_valid) || wg_birthdate_has_expired(key->birthdate, REJECT_AFTER_TIME) || key->counter.receive.counter >= REJECT_AFTER_MESSAGES)) { - key->is_valid = false; + WRITE_ONCE(key->is_valid, false); return false; } @@ -534,7 +534,7 @@ static void wg_packet_consume_data(struct wg_device *wg, struct sk_buff *skb) if (unlikely(!wg_noise_keypair_get(PACKET_CB(skb)->keypair))) goto err_keypair; - if (unlikely(peer->is_dead)) + if (unlikely(READ_ONCE(peer->is_dead))) goto err; ret = wg_queue_enqueue_per_device_and_peer(&wg->decrypt_queue, diff --git a/src/send.c b/src/send.c index b7f8283..1e97aa1 100644 --- a/src/send.c +++ b/src/send.c @@ -65,7 +65,8 @@ void wg_packet_send_queued_handshake_initiation(struct wg_peer *peer, * necessary: */ if (!wg_birthdate_has_expired(atomic64_read(&peer->last_sent_handshake), - REKEY_TIMEOUT) || unlikely(peer->is_dead)) + REKEY_TIMEOUT) || + unlikely(READ_ONCE(peer->is_dead))) goto out; wg_peer_get(peer); @@ -128,7 +129,7 @@ static void keep_key_fresh(struct wg_peer *peer) rcu_read_lock_bh(); keypair = rcu_dereference_bh(peer->keypairs.current_keypair); - if (likely(keypair && keypair->sending.is_valid) && + if (likely(keypair && READ_ONCE(keypair->sending.is_valid)) && (unlikely(atomic64_read(&keypair->sending.counter.counter) > REKEY_AFTER_MESSAGES) || (keypair->i_am_the_initiator && @@ -327,7 +328,7 @@ static void wg_packet_create_data(struct sk_buff *first) int ret = -EINVAL; rcu_read_lock_bh(); - if (unlikely(peer->is_dead)) + if (unlikely(READ_ONCE(peer->is_dead))) goto err; ret = wg_queue_enqueue_per_device_and_peer(&wg->encrypt_queue, @@ -369,7 +370,7 @@ void wg_packet_send_staged_packets(struct wg_peer *peer) if (unlikely(!keypair)) goto out_nokey; key = &keypair->sending; - if (unlikely(!key->is_valid)) + if (unlikely(!READ_ONCE(key->is_valid))) goto out_nokey; if (unlikely(wg_birthdate_has_expired(key->birthdate, REJECT_AFTER_TIME))) @@ -398,7 +399,7 @@ void wg_packet_send_staged_packets(struct wg_peer *peer) return; out_invalid: - key->is_valid = false; + WRITE_ONCE(key->is_valid, false); out_nokey: wg_noise_keypair_put(keypair, false); diff --git a/src/timers.c b/src/timers.c index 0312bd8..22eb1ee 100644 --- a/src/timers.c +++ b/src/timers.c @@ -39,7 +39,8 @@ static inline void mod_peer_timer(struct wg_peer *peer, unsigned long expires) { rcu_read_lock_bh(); - if (likely(netif_running(peer->device->dev) && !peer->is_dead)) + if (likely(netif_running(peer->device->dev) && + !READ_ONCE(peer->is_dead))) mod_timer(timer, expires); rcu_read_unlock_bh(); } @@ -48,7 +49,8 @@ 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) && !peer->is_dead)) + if (likely(netif_running(peer->device->dev) && + !READ_ONCE(peer->is_dead))) del_timer(timer); rcu_read_unlock_bh(); } @@ -136,7 +138,7 @@ static void wg_expired_zero_key_material(struct timer_list *timer) return; rcu_read_lock_bh(); - if (!peer->is_dead) { + if (!READ_ONCE(peer->is_dead)) { /* Should take our reference. */ if (!queue_work(peer->device->handshake_send_wq, &peer->clear_peer_work)) -- cgit v1.2.3-59-g8ed1b