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 <[email protected]>
---
Changes since v2 
https://lore.kernel.org/openvpn-devel/082540583b9145d89e1cdd5a74c485ea3a53d285.1785308184.git.r...@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.r...@mandelbit.com/

 drivers/net/ovpn/udp.c | 81 +++++++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index eeef4a7229f5..055cdb1bee13 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -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:
-- 
2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to