diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h index c0994c606554..17d57b12fa5e 100644 --- a/drivers/net/ovpn/peer.h +++ b/drivers/net/ovpn/peer.h @@ -46,6 +46,12 @@ * @tcp.sk_cb.ops: pointer to the original prot_ops object (TCP only) * @crypto: the crypto configuration (ciphers, keys, etc..) * @dst_cache: cache for dst_entry used to send to peer + * @dst_cache_sport: inet_sport observed when the dst_cache was last + * (re-)populated; compared on every TX to detect changes + * (e.g. connect(AF_UNSPEC)) and invalidate the cache + * @dst_cache_mark: sk_mark observed when the dst_cache was last + * (re-)populated; compared on every TX to detect changes + * via setsockopt(SO_MARK) and invalidate the cache * @bind: remote peer binding * @keepalive_interval: seconds after which a new keepalive should be sent * @keepalive_xmit_exp: future timestamp when next keepalive should be sent @@ -102,6 +108,8 @@ struct ovpn_peer { } tcp; struct ovpn_crypto_state crypto; struct dst_cache dst_cache; + __be16 dst_cache_sport; + u32 dst_cache_mark; struct ovpn_bind __rcu *bind; unsigned long keepalive_interval; unsigned long keepalive_xmit_exp; diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c index 17d65d1595ed..fc5ef77d17b0 100644 --- a/drivers/net/ovpn/udp.c +++ b/drivers/net/ovpn/udp.c @@ -143,19 +143,24 @@ static int ovpn_udp_encap_recv(struct sock *sk, struct sk_buff *skb) */ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, struct dst_cache *cache, struct sock *sk, - struct sk_buff *skb) + struct sk_buff *skb, __be16 sport, u32 mark) { + /* bind->local is updated in place under peer->lock; a single aligned + * word is read/written atomically via {READ,WRITE}_ONCE. Snapshot it + * so the post-lookup validity check can compare against the value we + * actually resolved with: fl.saddr may be overwritten by the FIB when + * the local address is unset, and comparing bind->local against that + * would spuriously skip caching. + */ + __be32 saddr = READ_ONCE(bind->local.ipv4.s_addr); struct rtable *rt; struct flowi4 fl = { - /* 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), + .saddr = saddr, .daddr = bind->remote.in4.sin_addr.s_addr, - .fl4_sport = inet_sk(sk)->inet_sport, + .fl4_sport = sport, .fl4_dport = bind->remote.in4.sin_port, .flowi4_proto = sk->sk_protocol, - .flowi4_mark = sk->sk_mark, + .flowi4_mark = mark, }; int ret; @@ -171,6 +176,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, * new look up */ fl.saddr = 0; + saddr = 0; spin_lock_bh(&peer->lock); WRITE_ONCE(bind->local.ipv4.s_addr, 0); spin_unlock_bh(&peer->lock); @@ -180,6 +186,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, rt = ip_route_output_flow(sock_net(sk), &fl, sk); if (IS_ERR(rt) && PTR_ERR(rt) == -EINVAL) { fl.saddr = 0; + saddr = 0; spin_lock_bh(&peer->lock); WRITE_ONCE(bind->local.ipv4.s_addr, 0); spin_unlock_bh(&peer->lock); @@ -196,7 +203,21 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, ret); goto err; } - dst_cache_set_ip4(cache, &rt->dst, fl.saddr); + /* only cache the result if the parameters we resolved with are still + * current: a concurrent ovpn_peer_endpoints_update() may have replaced + * the bind (float) or updated bind->local in place, and a concurrent + * ovpn_udp_output() may have re-tagged the cache with a different + * sport/mark after we sampled them. In any of these cases the cache was + * already reset and re-caching a route resolved with the stale values + * would poison it, so bail out and reset instead. + */ + if (rcu_access_pointer(peer->bind) == bind && + READ_ONCE(bind->local.ipv4.s_addr) == saddr && + READ_ONCE(peer->dst_cache_sport) == sport && + READ_ONCE(peer->dst_cache_mark) == mark) + dst_cache_set_ip4(cache, &rt->dst, fl.saddr); + else + dst_cache_reset(cache); transmit: udp_tunnel_xmit_skb(rt, sk, skb, fl.saddr, fl.daddr, 0, @@ -221,24 +242,30 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, */ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind, struct dst_cache *cache, struct sock *sk, - struct sk_buff *skb) + struct sk_buff *skb, __be16 sport, u32 mark) { struct dst_entry *dst; + struct in6_addr local, saddr; int ret; struct flowi6 fl = { .daddr = bind->remote.in6.sin6_addr, - .fl6_sport = inet_sk(sk)->inet_sport, + .fl6_sport = sport, .fl6_dport = bind->remote.in6.sin6_port, .flowi6_proto = sk->sk_protocol, - .flowi6_mark = sk->sk_mark, + .flowi6_mark = mark, .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 + * address under the peer seqcount to avoid a torn read. Snapshot it so + * the post-lookup validity check can compare against the value we + * actually resolved with: fl.saddr may be overwritten by the FIB when + * the local address is unset, and comparing bind->local against that + * would spuriously skip caching. */ ovpn_peer_local_ipv6(peer, bind, &fl.saddr); + saddr = fl.saddr; local_bh_disable(); dst = dst_cache_get_ip6(cache, &fl.saddr); @@ -251,6 +278,7 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind, * new look up */ fl.saddr = in6addr_any; + saddr = in6addr_any; spin_lock_bh(&peer->lock); write_seqcount_begin(&peer->bind_local_seq); bind->local.ipv6 = in6addr_any; @@ -267,7 +295,22 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind, &bind->remote.in6, ret); goto err; } - dst_cache_set_ip6(cache, dst, &fl.saddr); + /* only cache the result if the parameters we resolved with are still + * current: a concurrent ovpn_peer_endpoints_update() may have replaced + * the bind (float) or updated bind->local in place, and a concurrent + * ovpn_udp_output() may have re-tagged the cache with a different + * sport/mark after we sampled them. In any of these cases the cache was + * already reset and re-caching a route resolved with the stale values + * would poison it, so bail out and reset instead. + */ + ovpn_peer_local_ipv6(peer, bind, &local); + if (rcu_access_pointer(peer->bind) == bind && + ipv6_addr_equal(&local, &saddr) && + READ_ONCE(peer->dst_cache_sport) == sport && + READ_ONCE(peer->dst_cache_mark) == mark) + dst_cache_set_ip6(cache, dst, &fl.saddr); + else + dst_cache_reset(cache); transmit: /* user IPv6 packets may be larger than the transport interface @@ -306,12 +349,40 @@ static int ovpn_udp_output(struct ovpn_peer *peer, struct dst_cache *cache, struct sock *sk, struct sk_buff *skb) { struct ovpn_bind *bind; + __be16 sport; + u32 mark; int ret; /* set sk to null if skb is already orphaned */ if (!skb->destructor) skb->sk = NULL; + sport = READ_ONCE(inet_sk(sk)->inet_sport); + if (unlikely(!sport)) { + /* the transport UDP socket has been disconnected (e.g. via + * connect(AF_UNSPEC)): inet_sport == 0 means the socket has + * been unhashed and sending from source port 0 is nonsense; + * refuse and tell the operator + */ + netdev_warn_once(peer->ovpn->dev, + "UDP transport socket has no source port; was it disconnected?\n"); + return -EIO; + } + mark = READ_ONCE(sk->sk_mark); + + /* userspace can change sk_mark (via setsockopt(SO_MARK)) and + * inet_sport (via connect(AF_UNSPEC)) at any time without notifying + * ovpn; if either differs from what the dst_cache was last populated + * with, invalidate the cache now so a hit doesn't return a dst + * resolved with the old value + */ + if (READ_ONCE(peer->dst_cache_sport) != sport || + READ_ONCE(peer->dst_cache_mark) != mark) { + dst_cache_reset(cache); + WRITE_ONCE(peer->dst_cache_sport, sport); + WRITE_ONCE(peer->dst_cache_mark, mark); + } + rcu_read_lock(); bind = rcu_dereference(peer->bind); if (unlikely(!bind)) { @@ -323,11 +394,11 @@ static int ovpn_udp_output(struct ovpn_peer *peer, struct dst_cache *cache, switch (bind->remote.in4.sin_family) { case AF_INET: - ret = ovpn_udp4_output(peer, bind, cache, sk, skb); + ret = ovpn_udp4_output(peer, bind, cache, sk, skb, sport, mark); break; #if IS_ENABLED(CONFIG_IPV6) case AF_INET6: - ret = ovpn_udp6_output(peer, bind, cache, sk, skb); + ret = ovpn_udp6_output(peer, bind, cache, sk, skb, sport, mark); break; #endif default: