Re: [PATCH ovpn net 3/9] ovpn: fix data race reading cached local endpoint on TX path

On 27/05/2026 01:18, Antonio Quartulli wrote:
> From: Antonio Quartulli <[email protected]>
> 
> The cached local source address bind->local is updated in place on a
> live, RCU-published ovpn_bind while holding peer->lock: the UDP output
> error paths reset it (ovpn_udp4_output()/ovpn_udp6_output()) and the RX
> float path learns it (ovpn_peer_endpoints_update()). The UDP TX fast
> path and the netlink dump, however, read bind->local holding only
> rcu_read_lock(), never peer->lock.
> 
> For bind->local.ipv6 this is a torn read: struct in6_addr is 128 bit and
> is copied as multiple words, so a concurrent in-place update can make a
> reader observe a mix of the old and new address. The mangled source
> address then feeds ip6_dst_lookup_flow() and udp_tunnel6_xmit_skb(). For
> bind->local.ipv4 (a single aligned word) it is a data race without
> tearing.
> 
> A spinlock on the per-packet TX path is not acceptable, and
> READ_ONCE()/WRITE_ONCE() cannot atomically access the 128-bit IPv6
> address (the >8-byte access is rejected at build time and per-word
> accesses still can't yield a consistent snapshot).
> 
> Serialize the IPv6 field with a per-peer seqcount_spinlock_t tied to the
> existing peer->lock: the in-place writers (already under peer->lock) bump
> it, and readers take a lock-free read_seqcount_begin()/retry() snapshot
> via the new ovpn_peer_local_ipv6() helper. The single-word IPv4 field is
> handled with plain READ_ONCE()/WRITE_ONCE(). bind->remote is untouched:
> it is immutable for a given bind object (only swapped via whole-bind RCU
> replacement), so reading it locklessly remains safe.
> 
> Fixes: 08857b5ec5d9 ("ovpn: implement basic TX path (UDP)")
> Signed-off-by: Antonio Quartulli <[email protected]>

Using seqcount_spinlock may be a bit overkill and complicated.

Ralf is attempting a fix by turning the ovpn_bind object to an RCU 
protected pointer.

This way any modification will be made by creating a new bind object and 
purging the old one, rather than modifying the current one.

We'll see how large the patch happens to be and possibly replace this one.

Regards,


-- 
Antonio Quartulli