struct ovpn_bind is published through peer->bind with RCU, but local
endpoint learning updates bind->local in place under peer->lock. UDP TX
reads the field without that lock. In particular, a concurrent IPv6
update can therefore result in a torn address read.
Use ovpn_peer_reset_sockaddr to publish a replacement bind when learning
a new local endpoint, just as a remote endpoint change does. Preserve
the current remote address and reset the dst cache only after the new
bind has been published successfully.
Track remote endpoint changes separately so that float notification and
transport-address rehashing remain limited to actual peer floats.
Fixes: f0281c1d3732 ("ovpn: add support for updating local or remote UDP
endpoint")
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 and the next patch.
(Sabrina)
- Tracked float with the specific floated boolean rather than indirectly
through salen.
- Refactored the code into a cleaner shape. (Sabrina)
drivers/net/ovpn/peer.c | 43 ++++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index b400783c2efa..430c6cd48db8 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -200,13 +200,12 @@ static void __ovpn_peer_hash_transp_addr(struct ovpn_peer
*peer,
*/
void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)
{
+ const void *local_ip = NULL;
struct sockaddr_storage ss;
struct sockaddr_in6 *sa6;
- bool reset_cache = false;
struct sockaddr_in *sa;
struct ovpn_bind *bind;
- const void *local_ip;
- size_t salen = 0;
+ bool floated = false;
spin_lock_bh(&peer->lock);
bind = rcu_dereference_protected(peer->bind,
@@ -233,8 +232,7 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer,
struct sk_buff *skb)
.sin_addr.s_addr = ip_hdr(skb)->saddr,
.sin_port = udp_hdr(skb)->source,
};
- salen = sizeof(*sa);
- reset_cache = true;
+ floated = true;
break;
}
@@ -246,10 +244,12 @@ 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;
- reset_cache = true;
+ local_ip = &ip_hdr(skb)->daddr;
+ memcpy(&ss, &bind->remote, sizeof(struct sockaddr_in));
+ break;
}
- break;
+ /* nothing changed */
+ goto unlock;
case htons(ETH_P_IPV6):
/* float check */
if (unlikely(!ovpn_bind_skb_src_match(bind, skb))) {
@@ -271,8 +271,7 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer,
struct sk_buff *skb)
ipv6_iface_scope_id(&ipv6_hdr(skb)->saddr,
skb->skb_iif),
};
- salen = sizeof(*sa6);
- reset_cache = true;
+ floated = true;
break;
}
@@ -285,26 +284,30 @@ 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);
- bind->local.ipv6 = ipv6_hdr(skb)->daddr;
- reset_cache = true;
+ local_ip = &ipv6_hdr(skb)->daddr;
+ memcpy(&ss, &bind->remote, sizeof(struct sockaddr_in6));
+ break;
}
- break;
+ /* nothing changed */
+ goto unlock;
default:
goto unlock;
}
- if (unlikely(reset_cache))
- dst_cache_reset(&peer->dst_cache);
-
- /* if the peer did not float, we can bail out now */
- if (likely(!salen))
- goto unlock;
-
if (unlikely(ovpn_peer_reset_sockaddr(peer,
(struct sockaddr_storage *)&ss,
local_ip) < 0))
goto unlock;
+ /* reset the cache only after a successful bind update to avoid useless
+ * cache misses on concurrent TX
+ */
+ dst_cache_reset(&peer->dst_cache);
+
+ /* if only the local address changed, bail out now */
+ if (!floated)
+ goto unlock;
+
net_dbg_ratelimited("%s: peer %d floated to %pIScp",
netdev_name(peer->ovpn->dev), peer->id, &ss);
--
2.55.0
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
