diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c index 4dad85294198..8e21fa3e7822 100644 --- a/drivers/net/ovpn/netlink.c +++ b/drivers/net/ovpn/netlink.c @@ -610,14 +610,23 @@ static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, bind = rcu_dereference(peer->bind); if (bind) { if (bind->remote.in4.sin_family == AF_INET) { + /* bind->local is updated in place under peer->lock; + * READ_ONCE() pairs with the WRITE_ONCE() updaters + */ if (nla_put_in_addr(skb, OVPN_A_PEER_REMOTE_IPV4, bind->remote.in4.sin_addr.s_addr) || nla_put_net16(skb, OVPN_A_PEER_REMOTE_PORT, bind->remote.in4.sin_port) || nla_put_in_addr(skb, OVPN_A_PEER_LOCAL_IPV4, - bind->local.ipv4.s_addr)) + READ_ONCE(bind->local.ipv4.s_addr))) goto err_unlock; } else if (bind->remote.in4.sin_family == AF_INET6) { + struct in6_addr local_ipv6; + + /* read the 128-bit local address under the peer + * seqcount to avoid a torn read + */ + ovpn_peer_local_ipv6(peer, bind, &local_ipv6); if (nla_put_in6_addr(skb, OVPN_A_PEER_REMOTE_IPV6, &bind->remote.in6.sin6_addr) || nla_put_u32(skb, OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID, @@ -625,7 +634,7 @@ static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, nla_put_net16(skb, OVPN_A_PEER_REMOTE_PORT, bind->remote.in6.sin6_port) || nla_put_in6_addr(skb, OVPN_A_PEER_LOCAL_IPV6, - &bind->local.ipv6)) + &local_ipv6)) goto err_unlock; } } diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index 8aa07560bb30..bbb1946fa5b4 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c @@ -112,6 +112,7 @@ struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 id) RCU_INIT_POINTER(peer->bind, NULL); ovpn_crypto_state_init(&peer->crypto); spin_lock_init(&peer->lock); + seqcount_spinlock_init(&peer->bind_local_seq, &peer->lock); kref_init(&peer->refcount); ovpn_peer_stats_init(&peer->vpn_stats); ovpn_peer_stats_init(&peer->link_stats); @@ -175,6 +176,27 @@ int ovpn_peer_reset_sockaddr(struct ovpn_peer *peer, return 0; } +/** + * ovpn_peer_local_ipv6 - read the cached local IPv6 endpoint of a peer + * @peer: the peer owning the binding + * @bind: the binding to read the local address from + * @dst: where the local IPv6 address is copied to + * + * bind->local is updated in place under peer->lock (TX error path and RX + * float path). Read the 128-bit address under the peer seqcount so that + * lockless readers never observe a torn value. + */ +void ovpn_peer_local_ipv6(const struct ovpn_peer *peer, + const struct ovpn_bind *bind, struct in6_addr *dst) +{ + unsigned int seq; + + do { + seq = read_seqcount_begin(&peer->bind_local_seq); + *dst = bind->local.ipv6; + } while (read_seqcount_retry(&peer->bind_local_seq, seq)); +} + /* variable name __tbl2 needs to be different from __tbl1 * in the macro below to avoid confusing clang */ @@ -237,7 +259,7 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb) netdev_name(peer->ovpn->dev), peer->id, &bind->local.ipv4.s_addr, &ip_hdr(skb)->daddr); - bind->local.ipv4.s_addr = ip_hdr(skb)->daddr; + WRITE_ONCE(bind->local.ipv4.s_addr, ip_hdr(skb)->daddr); reset_cache = true; } break; @@ -268,7 +290,9 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb) netdev_name(peer->ovpn->dev), peer->id, &bind->local.ipv6, &ipv6_hdr(skb)->daddr); + write_seqcount_begin(&peer->bind_local_seq); bind->local.ipv6 = ipv6_hdr(skb)->daddr; + write_seqcount_end(&peer->bind_local_seq); reset_cache = true; } break; diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h index dfa5c0037e02..c0994c606554 100644 --- a/drivers/net/ovpn/peer.h +++ b/drivers/net/ovpn/peer.h @@ -10,6 +10,7 @@ #ifndef _NET_OVPN_OVPNPEER_H_ #define _NET_OVPN_OVPNPEER_H_ +#include #include #include @@ -56,6 +57,8 @@ * @link_stats: per-peer link/transport TX/RX stats * @delete_reason: why peer was deleted (i.e. timeout, transport error, ..) * @lock: protects binding to peer (bind) and keepalive* fields + * @bind_local_seq: seqcount serializing in-place updates of bind->local + * (done under @lock) against lockless readers on the TX path * @refcount: reference counter * @rcu: used to free peer in an RCU safe way * @release_entry: entry for the socket release list @@ -110,6 +113,7 @@ struct ovpn_peer { struct ovpn_peer_stats link_stats; enum ovpn_del_peer_reason delete_reason; spinlock_t lock; /* protects bind and keepalive* */ + seqcount_spinlock_t bind_local_seq; /* protects bind->local */ struct kref refcount; struct rcu_head rcu; struct llist_node release_entry; @@ -151,6 +155,8 @@ struct ovpn_peer *ovpn_peer_get_by_dst(struct ovpn_priv *ovpn, struct sk_buff *skb); void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer); void ovpn_peer_hash_transp_addr(struct ovpn_peer *peer); +void ovpn_peer_local_ipv6(const struct ovpn_peer *peer, + const struct ovpn_bind *bind, struct in6_addr *dst); bool ovpn_peer_check_by_src(struct ovpn_priv *ovpn, struct sk_buff *skb, struct ovpn_peer *peer); diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c index 8811aa9eedeb..60d32dc5af4a 100644 --- a/drivers/net/ovpn/udp.c +++ b/drivers/net/ovpn/udp.c @@ -147,7 +147,10 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, { struct rtable *rt; struct flowi4 fl = { - .saddr = bind->local.ipv4.s_addr, + /* bind->local is updated in place under peer->lock; a single + * aligned word is read/written atomically via {READ,WRITE}_ONCE + */ + .saddr = READ_ONCE(bind->local.ipv4.s_addr), .daddr = bind->remote.in4.sin_addr.s_addr, .fl4_sport = inet_sk(sk)->inet_sport, .fl4_dport = bind->remote.in4.sin_port, @@ -169,7 +172,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, */ fl.saddr = 0; spin_lock_bh(&peer->lock); - bind->local.ipv4.s_addr = 0; + WRITE_ONCE(bind->local.ipv4.s_addr, 0); spin_unlock_bh(&peer->lock); dst_cache_reset(cache); } @@ -178,7 +181,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, if (IS_ERR(rt) && PTR_ERR(rt) == -EINVAL) { fl.saddr = 0; spin_lock_bh(&peer->lock); - bind->local.ipv4.s_addr = 0; + WRITE_ONCE(bind->local.ipv4.s_addr, 0); spin_unlock_bh(&peer->lock); dst_cache_reset(cache); @@ -224,7 +227,6 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind, int ret; struct flowi6 fl = { - .saddr = bind->local.ipv6, .daddr = bind->remote.in6.sin6_addr, .fl6_sport = inet_sk(sk)->inet_sport, .fl6_dport = bind->remote.in6.sin6_port, @@ -233,6 +235,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind, .flowi6_oif = bind->remote.in6.sin6_scope_id, }; + /* bind->local is updated in place under peer->lock; read the 128-bit + * address under the peer seqcount to avoid a torn read + */ + ovpn_peer_local_ipv6(peer, bind, &fl.saddr); + local_bh_disable(); dst = dst_cache_get_ip6(cache, &fl.saddr); if (dst) @@ -245,7 +252,9 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind, */ fl.saddr = in6addr_any; spin_lock_bh(&peer->lock); + write_seqcount_begin(&peer->bind_local_seq); bind->local.ipv6 = in6addr_any; + write_seqcount_end(&peer->bind_local_seq); spin_unlock_bh(&peer->lock); dst_cache_reset(cache); }