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..ca502c920f54 100644 --- a/drivers/net/ovpn/udp.c +++ b/drivers/net/ovpn/udp.c @@ -143,7 +143,7 @@ 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) { struct rtable *rt; struct flowi4 fl = { @@ -152,10 +152,10 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, */ .saddr = READ_ONCE(bind->local.ipv4.s_addr), .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; @@ -196,7 +196,17 @@ 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 bind is still current: a concurrent + * ovpn_peer_endpoints_update() may have replaced the bind (float) or + * updated bind->local in place, in which case ovpn already reset the + * cache and re-caching here would reinstate a stale route. sport/mark + * are validated at TX entry by ovpn_udp_output(). + */ + if (rcu_access_pointer(peer->bind) == bind && + READ_ONCE(bind->local.ipv4.s_addr) == fl.saddr) + 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,17 +231,18 @@ 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; 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, }; @@ -267,7 +278,18 @@ 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 bind is still current: a concurrent + * ovpn_peer_endpoints_update() may have replaced the bind (float) or + * updated bind->local in place, in which case ovpn already reset the + * cache and re-caching here would reinstate a stale route. sport/mark + * are validated at TX entry by ovpn_udp_output(). + */ + ovpn_peer_local_ipv6(peer, bind, &local); + if (rcu_access_pointer(peer->bind) == bind && + ipv6_addr_equal(&local, &fl.saddr)) + 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 +328,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 +373,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: