The UDP output fallback clears bind->local in place when the remembered
source address is no longer usable. The bind is RCU-published and read
locklessly by concurrent TX, so an IPv6 reader can observe a torn
address.
Retry the route lookup with source address autoselection without
modifying the bind. After a successful lookup, revalidate the bind and
route key under peer->lock, reset the dst cache, and best-effort publish
a replacement bind with a wildcard local address.
Do not cache the resolved dst when clearing the local source. Replacing
the source invalidates all per-CPU cache entries, while
dst_cache_set_ip4 and dst_cache_set_ip6 update only the current CPU
slot. The current packet can still use the resolved route; if bind
allocation fails, a later cache miss retries the repair.
Fixes: 08857b5ec5d9 ("ovpn: implement basic TX path (UDP)")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
---
Changes since v2 https://lore.kernel.org/openvpn-devel/082540583b9145d89e1cdd5a74c485ea3a53d285.1785308184.git.ralf@mandelbit.com/
- Split former 4/5 into this plus the previous two patches. (Sabrina)
- No functional changes.
No changes since v1 https://lore.kernel.org/openvpn-devel/082540583b9145d89e1cdd5a74c485ea3a53d285.1785253480.git.ralf@mandelbit.com/
drivers/net/ovpn/udp.c | 81 +++++++++++++++++++++++++++++-------------
1 file changed, 56 insertions(+), 25 deletions(-)
@@ -185,7 +185,7 @@ static void ovpn_dst_cache_check_key(struct ovpn_peer *peer,
* to detect whether the bind was replaced while the route lookup was running.
*
* Return: true if the lookup result still matches the current peer state and
- * may update the dst cache.
+ * may update the dst cache or replace the bind.
*/
static bool ovpn_dst_cache_current(const struct ovpn_peer *peer,
const struct ovpn_bind *bind,
@@ -218,6 +218,9 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
struct sk_buff *skb,
const struct ovpn_route_key *key)
{
+ struct sockaddr_storage remote;
+ struct in_addr local = {};
+ bool reset_local = false;
struct rtable *rt;
struct flowi4 fl = {
.saddr = bind->local.ipv4.s_addr,
@@ -236,24 +239,17 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
if (fl.saddr && unlikely(!inet_confirm_addr(sock_net(sk), NULL, 0,
fl.saddr, RT_SCOPE_HOST))) {
- /* we may end up here when the cached address is not usable
- * anymore. In this case we reset address/cache and perform a
- * new look up
+ /* The learned local address is not usable anymore.
+ * Retry with source address autoselection.
*/
fl.saddr = 0;
- spin_lock_bh(&peer->lock);
- bind->local.ipv4.s_addr = 0;
- spin_unlock_bh(&peer->lock);
- dst_cache_reset(cache);
+ reset_local = true;
}
rt = ip_route_output_flow(sock_net(sk), &fl, sk);
if (IS_ERR(rt) && PTR_ERR(rt) == -EINVAL) {
fl.saddr = 0;
- spin_lock_bh(&peer->lock);
- bind->local.ipv4.s_addr = 0;
- spin_unlock_bh(&peer->lock);
- dst_cache_reset(cache);
+ reset_local = true;
rt = ip_route_output_flow(sock_net(sk), &fl, sk);
}
@@ -267,10 +263,28 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
goto err;
}
- /* avoid storing a stale cache */
+ /* avoid storing a stale cache or local address */
spin_lock_bh(&peer->lock);
- if (likely(ovpn_dst_cache_current(peer, bind, key)))
- dst_cache_set_ip4(cache, &rt->dst, fl.saddr);
+ if (likely(ovpn_dst_cache_current(peer, bind, key))) {
+ if (!reset_local) {
+ dst_cache_set_ip4(cache, &rt->dst, fl.saddr);
+ spin_unlock_bh(&peer->lock);
+ goto transmit;
+ }
+
+ /* invalidate per-CPU dst entries that may still carry
+ * the stale source
+ */
+ dst_cache_reset(cache);
+
+ /* preserve the current remote */
+ memcpy(&remote, &bind->remote, sizeof(struct sockaddr_in));
+ /* The current packet already has a valid wildcard-source route.
+ * If replacing the bind fails, leave the stale local in place;
+ * a later cache miss will retry the repair.
+ */
+ ovpn_peer_reset_sockaddr(peer, &remote, &local);
+ }
spin_unlock_bh(&peer->lock);
transmit:
@@ -300,6 +314,9 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
struct sk_buff *skb,
const struct ovpn_route_key *key)
{
+ struct in6_addr local = in6addr_any;
+ struct sockaddr_storage remote;
+ bool reset_local = false;
struct dst_entry *dst;
int ret;
@@ -320,15 +337,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
if (!ipv6_addr_any(&fl.saddr) &&
unlikely(!ipv6_chk_addr(sock_net(sk), &fl.saddr, NULL, 0))) {
- /* we may end up here when the cached address is not usable
- * anymore. In this case we reset address/cache and perform a
- * new look up
+ /* The learned local address is not usable anymore.
+ * Retry with source address autoselection.
*/
fl.saddr = in6addr_any;
- spin_lock_bh(&peer->lock);
- bind->local.ipv6 = in6addr_any;
- spin_unlock_bh(&peer->lock);
- dst_cache_reset(cache);
+ reset_local = true;
}
dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl, NULL);
@@ -340,10 +353,28 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
goto err;
}
- /* avoid storing a stale cache */
+ /* avoid storing a stale cache or local address */
spin_lock_bh(&peer->lock);
- if (likely(ovpn_dst_cache_current(peer, bind, key)))
- dst_cache_set_ip6(cache, dst, &fl.saddr);
+ if (likely(ovpn_dst_cache_current(peer, bind, key))) {
+ if (!reset_local) {
+ dst_cache_set_ip6(cache, dst, &fl.saddr);
+ spin_unlock_bh(&peer->lock);
+ goto transmit;
+ }
+
+ /* invalidate per-CPU dst entries that may still carry
+ * the stale source
+ */
+ dst_cache_reset(cache);
+
+ /* preserve the current remote */
+ memcpy(&remote, &bind->remote, sizeof(struct sockaddr_in6));
+ /* The current packet already has a valid wildcard-source route.
+ * If replacing the bind fails, leave the stale local in place;
+ * a later cache miss will retry the repair.
+ */
+ ovpn_peer_reset_sockaddr(peer, &remote, &local);
+ }
spin_unlock_bh(&peer->lock);
transmit: