[PATCH net 0/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[PATCH net 0/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Ralf Lici]

Hi,

This patch was originally part of a wider ovpn series addressing UDP
route-cache correctness when mutable socket routing properties or peer
endpoint state change while packets are being transmitted [1]. The
ovpn-specific fixes were resubmitted separately after dropping this
patch, which addresses an independent IPv6 FIB race reproduced in ovpn.

During review, Sabrina suggested submitting the patch directly to netdev
rather than through the ovpn pull request because it touches generic
dst_cache code and the underlying late-cookie sampling pattern is not
specific to ovpn. I first sent an RFC describing the race and a possible
kernel-wide solution [2], but it has not received any feedback so far.

After discussing the submission path with Antonio, we decided to post
the concrete ovpn fix directly to netdev. It prevents ovpn from caching
a route if the IPv6 FIB generation changed during lookup, without
attempting the broader network-stack refactoring discussed in the RFC.

[1] https://lore.kernel.org/openvpn-devel/cover.1785308184.git.ralf@mandelbit.com/
[2] https://lore.kernel.org/netdev/20260901122501.482920-1-ralf@mandelbit.com/

Compared with the version previously posted as part of the ovpn series,
this patch has been rebased on current net/main and made independent of
the pending ovpn route-cache and endpoint changes.

Thanks,

Ralf Lici
Mandelbit Srl

---
Ralf Lici (1):
  ovpn: avoid caching stale IPv6 dst after FIB changes

 drivers/net/ovpn/udp.c  | 20 +++++++++++++++++---
 include/net/dst_cache.h | 13 +++++++++++++
 net/core/dst_cache.c    | 19 +++++++++++++++----
 3 files changed, 45 insertions(+), 7 deletions(-)

base-commit: c9151088f1674fd29ff26a20f5fc687acf53a2f0
-- 
2.55.0

[PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Ralf Lici]

ovpn stores the IPv6 route used for UDP transmission in a per-peer dst
cache. IPv6 dst validation uses a cookie derived from the route itself,
or, for routes without their own sernum, from the associated fib6 node.

If the IPv6 FIB changes after ip6_dst_lookup_flow returns but before
dst_cache_set_ip6 reads the cookie, ovpn can store an old dst with a new
cookie. Later dst_cache_get_ip6 can then consider that stale dst valid
because the stored cookie matches the updated fib6 node sernum.

Sample the IPv6 FIB generation before and after route lookup, and only
populate ovpn's peer dst cache if the generation did not change while
the lookup was in flight. Also add a dst_cache helper that stores a
caller-provided IPv6 cookie, so the cached dst carries the cookie
sampled from the lookup result instead of one read after a concurrent
FIB update.

The current packet may still be transmitted with the route returned by
the lookup if the FIB changes before TX completion. This patch only
prevents that potentially stale route from being preserved in ovpn's
peer dst cache and reused for later packets.

Fixes: 08857b5ec5d9 ("ovpn: implement basic TX path (UDP)")
Link: https://lore.kernel.org/netdev/20260901122501.482920-1-ralf@mandelbit.com/
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
---
 drivers/net/ovpn/udp.c  | 20 +++++++++++++++++---
 include/net/dst_cache.h | 13 +++++++++++++
 net/core/dst_cache.c    | 19 +++++++++++++++----
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index 7f69e8890b5b..a28974cc36e8 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -220,8 +220,10 @@ 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 net *net = sock_net(sk);
 	struct dst_entry *dst;
-	int ret;
+	int gen0, gen1, ret;
+	u32 cookie;
 
 	struct flowi6 fl = {
 		.saddr = bind->local.ipv6,
@@ -250,7 +252,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 		dst_cache_reset(cache);
 	}
 
-	dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl, NULL);
+	gen0 = rt_genid_ipv6(net);
+	/* keep the unordered initial generation read before the FIB lookup */
+	smp_rmb();
+
+	dst = ip6_dst_lookup_flow(net, sk, &fl, NULL);
 	if (IS_ERR(dst)) {
 		ret = PTR_ERR(dst);
 		net_dbg_ratelimited("%s: no route to host %pISpc: %d\n",
@@ -258,7 +264,15 @@ 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);
+
+	cookie = rt6_get_cookie(dst_rt6_info(dst));
+
+	/* keep the FIB and cookie reads before the final generation read */
+	smp_rmb();
+	gen1 = rt_genid_ipv6(net);
+
+	if (likely(gen0 == gen1))
+		dst_cache_set_ip6_cookie(cache, dst, &fl.saddr, cookie);
 
 transmit:
 	/* user IPv6 packets may be larger than the transport interface
diff --git a/include/net/dst_cache.h b/include/net/dst_cache.h
index 1961699598e2..5f9cc4fe926c 100644
--- a/include/net/dst_cache.h
+++ b/include/net/dst_cache.h
@@ -45,6 +45,19 @@ void dst_cache_set_ip4(struct dst_cache *dst_cache, struct dst_entry *dst,
 
 #if IS_ENABLED(CONFIG_IPV6)
 
+/**
+ *	dst_cache_set_ip6_cookie - store ipv6 dst with caller-provided cookie
+ *	@dst_cache: the cache
+ *	@dst: the entry to be cached
+ *	@saddr: the source address to be stored inside the cache
+ *	@cookie: the route validation cookie to store with @dst
+ *
+ *	local BH must be disabled.
+ */
+void dst_cache_set_ip6_cookie(struct dst_cache *dst_cache,
+			      struct dst_entry *dst,
+			      const struct in6_addr *saddr, u32 cookie);
+
 /**
  *	dst_cache_set_ip6 - store the ipv6 dst into the cache
  *	@dst_cache: the cache
diff --git a/net/core/dst_cache.c b/net/core/dst_cache.c
index 9ab4902324e1..76c9fc1b0acf 100644
--- a/net/core/dst_cache.c
+++ b/net/core/dst_cache.c
@@ -117,8 +117,9 @@ void dst_cache_set_ip4(struct dst_cache *dst_cache, struct dst_entry *dst,
 EXPORT_SYMBOL_GPL(dst_cache_set_ip4);
 
 #if IS_ENABLED(CONFIG_IPV6)
-void dst_cache_set_ip6(struct dst_cache *dst_cache, struct dst_entry *dst,
-		       const struct in6_addr *saddr)
+void dst_cache_set_ip6_cookie(struct dst_cache *dst_cache,
+			      struct dst_entry *dst,
+			      const struct in6_addr *saddr, u32 cookie)
 {
 	struct dst_cache_pcpu *idst;
 
@@ -128,11 +129,21 @@ void dst_cache_set_ip6(struct dst_cache *dst_cache, struct dst_entry *dst,
 	local_lock_nested_bh(&dst_cache->cache->bh_lock);
 
 	idst = this_cpu_ptr(dst_cache->cache);
-	dst_cache_per_cpu_dst_set(idst, dst,
-				  rt6_get_cookie(dst_rt6_info(dst)));
+	dst_cache_per_cpu_dst_set(idst, dst, cookie);
 	idst->in6_saddr = *saddr;
 	local_unlock_nested_bh(&dst_cache->cache->bh_lock);
 }
+EXPORT_SYMBOL_GPL(dst_cache_set_ip6_cookie);
+
+void dst_cache_set_ip6(struct dst_cache *dst_cache, struct dst_entry *dst,
+		       const struct in6_addr *saddr)
+{
+	if (!dst_cache->cache)
+		return;
+
+	dst_cache_set_ip6_cookie(dst_cache, dst, saddr,
+				 rt6_get_cookie(dst_rt6_info(dst)));
+}
 EXPORT_SYMBOL_GPL(dst_cache_set_ip6);
 
 struct dst_entry *dst_cache_get_ip6(struct dst_cache *dst_cache,
-- 
2.55.0

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[David Ahern]

On 9/17/26 4:09 AM, Ralf Lici wrote:
> diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
> index 7f69e8890b5b..a28974cc36e8 100644
> --- a/drivers/net/ovpn/udp.c
> +++ b/drivers/net/ovpn/udp.c
> @@ -250,7 +252,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>  		dst_cache_reset(cache);
>  	}
>  
> -	dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl, NULL);
> +	gen0 = rt_genid_ipv6(net);

this detail about routes should not be buried here. rt_genid_ipv6 really
should be local to net/ipv6/route.c

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Ralf Lici]

On Thu, 17 Sep 2026 13:09:10 -0600, David Ahern <dsahern@kernel.org> wrote:
> On 9/17/26 4:09 AM, Ralf Lici wrote:
> > diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
> > index 7f69e8890b5b..a28974cc36e8 100644
> > --- a/drivers/net/ovpn/udp.c
> > +++ b/drivers/net/ovpn/udp.c
> > @@ -250,7 +252,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
> >  		dst_cache_reset(cache);
> >  	}
> >  
> > -	dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl, NULL);
> > +	gen0 = rt_genid_ipv6(net);
>
> this detail about routes should not be buried here. rt_genid_ipv6 really
> should be local to net/ipv6/route.c
>
>

I agree. Keeping this detail inside the IPv6 routing code is exactly why
I sent the RFC linked from the cover letter, and I would appreciate your
thoughts on the proposed approach:

https://lore.kernel.org/netdev/20260901122501.482920-1-ralf@mandelbit.com/

The pattern of obtaining a dst and only later sampling its validation
cookie when publishing it into a persistent cache is widespread in both
drivers and the networking core. It is racy for IPv6 routes with
rt6_info::sernum == 0, because rt6_get_cookie then reads the current
fib6_node::fn_sernum, not necessarily the value corresponding to the
lookup which returned that dst.

-- 
Ralf Lici
Mandelbit Srl

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[David Ahern]

On 9/18/26 12:51 AM, Ralf Lici wrote:
> On Thu, 17 Sep 2026 13:09:10 -0600, David Ahern <dsahern@kernel.org> wrote:
>> On 9/17/26 4:09 AM, Ralf Lici wrote:
>>> diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
>>> index 7f69e8890b5b..a28974cc36e8 100644
>>> --- a/drivers/net/ovpn/udp.c
>>> +++ b/drivers/net/ovpn/udp.c
>>> @@ -250,7 +252,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>>>  		dst_cache_reset(cache);
>>>  	}
>>>  
>>> -	dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl, NULL);
>>> +	gen0 = rt_genid_ipv6(net);
>>
>> this detail about routes should not be buried here. rt_genid_ipv6 really
>> should be local to net/ipv6/route.c
>>
>>
> 
> I agree. Keeping this detail inside the IPv6 routing code is exactly why
> I sent the RFC linked from the cover letter, and I would appreciate your
> thoughts on the proposed approach:
> 
> https://lore.kernel.org/netdev/20260901122501.482920-1-ralf@mandelbit.com/
> 
> The pattern of obtaining a dst and only later sampling its validation
> cookie when publishing it into a persistent cache is widespread in both
> drivers and the networking core. It is racy for IPv6 routes with
> rt6_info::sernum == 0, because rt6_get_cookie then reads the current
> fib6_node::fn_sernum, not necessarily the value corresponding to the
> lookup which returned that dst.
> 

the patch is focused on detecting a genid change at the time the dst is
cached. Why? It can just as easily change immediately after the double
genid check, so you are not really solving the problem.

Staleness is irrelevant until it is used again (ie, genid can change
many times between dst_cache_set and next use). That is the proper time
to recheck the genid and refresh the dst as needed.

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Ralf Lici]

On Fri, 18 Sep 2026 12:42:40 -0600, David Ahern <dsahern@kernel.org> wrote:
> On 9/18/26 12:51 AM, Ralf Lici wrote:
> > On Thu, 17 Sep 2026 13:09:10 -0600, David Ahern <dsahern@kernel.org> wrote:
> >> On 9/17/26 4:09 AM, Ralf Lici wrote:
> >>> diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
> >>> index 7f69e8890b5b..a28974cc36e8 100644
> >>> --- a/drivers/net/ovpn/udp.c
> >>> +++ b/drivers/net/ovpn/udp.c
> >>> @@ -250,7 +252,11 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
> >>>  		dst_cache_reset(cache);
> >>>  	}
> >>>  
> >>> -	dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl, NULL);
> >>> +	gen0 = rt_genid_ipv6(net);
> >>
> >> this detail about routes should not be buried here. rt_genid_ipv6 really
> >> should be local to net/ipv6/route.c
> >>
> >>
> > 
> > I agree. Keeping this detail inside the IPv6 routing code is exactly why
> > I sent the RFC linked from the cover letter, and I would appreciate your
> > thoughts on the proposed approach:
> > 
> > https://lore.kernel.org/netdev/20260901122501.482920-1-ralf@mandelbit.com/
> > 
> > The pattern of obtaining a dst and only later sampling its validation
> > cookie when publishing it into a persistent cache is widespread in both
> > drivers and the networking core. It is racy for IPv6 routes with
> > rt6_info::sernum == 0, because rt6_get_cookie then reads the current
> > fib6_node::fn_sernum, not necessarily the value corresponding to the
> > lookup which returned that dst.
> > 
>
> the patch is focused on detecting a genid change at the time the dst is
> cached. Why? It can just as easily change immediately after the double
> genid check, so you are not really solving the problem.
>
> Staleness is irrelevant until it is used again (ie, genid can change
> many times between dst_cache_set and next use). That is the proper time
> to recheck the genid and refresh the dst as needed.
>

The double generation check is not intended to guarantee that the dst
cannot become stale after the final read. Its purpose is to ensure that
a pre-change dst is not cached with a post-change cookie. And that is
enough because dst_cache_get_ip6, on the next run, misses the cache if
it's no longer valid.

For example, suppose the lookup returns D0 under cookie C0. If a
relevant FIB change to C1 happens after the final generation read (even
if it happens before dst_cache_set_ip6_cookie) the cache stores (D0, C0).
On the next use, dst_cache_get_ip6 invokes the ipv6 dst check, which
compares the cached C0 with the node's current C1 and rejects the entry.

The problematic sequence is instead:

lookup returns D0 under C0
FIB changes to C1
dst_cache_set_ip6 samples C1 from D0's fib6 node
cache stores (D0, C1)

The reuse-time check then compares the cached C1 with the current C1 and
accepts D0, potentially until another relevant FIB change. The patch
ensures that the saved cookie is either coherent with the lookup or
conservatively old, so the existing check at reuse remains meaningful.

-- 
Ralf Lici
Mandelbit Srl

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Ralf Lici]

Hi David,

Just following up on this. Does my previous example clarify why storing
an old dst with a new cookie defeats reuse-time validation?

If so, would you prefer that I pursue the kernel-wide lookup-provenance
approach described in the RFC, or keep the fix self-contained in ovpn?
The same late-cookie-sampling pattern is used by several drivers and
callers in the networking core, so I think fixing it generically is the
better architectural choice.

If changing the generic interfaces is not acceptable, I can rework the
patch as an ovpn-local fix, although that would duplicate IPv6 routing
details in the driver and leave the equivalent pattern elsewhere
unchanged.

Thanks,

-- 
Ralf Lici
Mandelbit Srl

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[David Ahern]

On 9/28/26 3:56 AM, Ralf Lici wrote:
> Hi David,
> 
> Just following up on this. Does my previous example clarify why storing
> an old dst with a new cookie defeats reuse-time validation?
> 
> If so, would you prefer that I pursue the kernel-wide lookup-provenance
> approach described in the RFC, or keep the fix self-contained in ovpn?
> The same late-cookie-sampling pattern is used by several drivers and
> callers in the networking core, so I think fixing it generically is the
> better architectural choice.
> 
> If changing the generic interfaces is not acceptable, I can rework the
> patch as an ovpn-local fix, although that would duplicate IPv6 routing
> details in the driver and leave the equivalent pattern elsewhere
> unchanged.
> 

dst's are cached in lots of places. Why does ovpn need internal routing
details that the other places do not?

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Ralf Lici]

On Tue, 29 Sep 2026 08:09:46 -0600, David Ahern <dsahern@kernel.org> wrote:
> On 9/28/26 3:56 AM, Ralf Lici wrote:
> > Hi David,
> > 
> > Just following up on this. Does my previous example clarify why storing
> > an old dst with a new cookie defeats reuse-time validation?
> > 
> > If so, would you prefer that I pursue the kernel-wide lookup-provenance
> > approach described in the RFC, or keep the fix self-contained in ovpn?
> > The same late-cookie-sampling pattern is used by several drivers and
> > callers in the networking core, so I think fixing it generically is the
> > better architectural choice.
> > 
> > If changing the generic interfaces is not acceptable, I can rework the
> > patch as an ovpn-local fix, although that would duplicate IPv6 routing
> > details in the driver and leave the equivalent pattern elsewhere
> > unchanged.
> > 
>
> dst's are cached in lots of places. Why does ovpn need internal routing
> details that the other places do not?
>
>

Because, as explained in this thread and in the RFC, there is enough
evidence that the current IPv6 dst-caching pattern is racy. If other
kernel users are expected to live with that race, I cannot force a
generic fix, but I do not think that is a good reason to require ovpn to
do the same.

If you believe the sequence I described is safe, please explain which
invariant makes it so.

-- 
Ralf Lici
Mandelbit Srl

Re: [PATCH net 1/1] ovpn: avoid caching stale IPv6 dst after FIB changes

[Antonio Quartulli]

On 29/09/2026 17:04, Ralf Lici wrote:
> On Tue, 29 Sep 2026 08:09:46 -0600, David Ahern <dsahern@kernel.org> wrote:
>> On 9/28/26 3:56 AM, Ralf Lici wrote:
>>> Hi David,
>>>
>>> Just following up on this. Does my previous example clarify why storing
>>> an old dst with a new cookie defeats reuse-time validation?
>>>
>>> If so, would you prefer that I pursue the kernel-wide lookup-provenance
>>> approach described in the RFC, or keep the fix self-contained in ovpn?
>>> The same late-cookie-sampling pattern is used by several drivers and
>>> callers in the networking core, so I think fixing it generically is the
>>> better architectural choice.
>>>
>>> If changing the generic interfaces is not acceptable, I can rework the
>>> patch as an ovpn-local fix, although that would duplicate IPv6 routing
>>> details in the driver and leave the equivalent pattern elsewhere
>>> unchanged.
>>>
>>
>> dst's are cached in lots of places. Why does ovpn need internal routing
>> details that the other places do not?
>>
>>
> 

Hi David,

ovpn maintainer here: I wanted to back this patch.

You're right that a cached dst can go stale later and that reuse-time
validation handles that. This case is different, because it breaks
reuse-time validation.

dst_cache_set_ip6() samples the cookie itself:

         dst_cache_per_cpu_dst_set(idst, dst,
                                   rt6_get_cookie(dst_rt6_info(dst)));

rt6_get_cookie() only returns a route-tied value when rt->sernum is
non-zero, which happens in ip6_rt_pcpu_alloc() and only for nexthop
routes. Everything else reads the current node generation:

         *cookie = READ_ONCE(fn->fn_sernum);

So if the FIB changes between ip6_dst_lookup_flow() returning D0 and
dst_cache_set_ip6() reading the cookie, we cache D0 with cookie C1. At
reuse dst_check() compares C1 against the node's current C1, they match,
and the stale route is used until the next FIB change.

Which invariant stops that cookie read from seeing a generation newer
than the one the lookup used? If there is one, we'll drop both the patch
and the RFC.

Regarding the layering, I agree. rt_genid_ipv6() does not belong in a 
driver.
That's exactly what the RFC [1] was for, but it's had no feedback so 
far, hence I suggested Ralf to pursue this other path.
The late sampling lives in dst_cache_set_ip6(), not in ovpn.

Happy to confine the IPv6 bits to a helper, or to revive the RFC if
you'd like review it.

Please also note that the workaround we're looking at is already 
implemented by other drivers, which they all could drop if we'd be able 
to provide an IPv6-generic solution.


Please let us know!


[1] 
https://lore.kernel.org/netdev/20260901122501.482920-1-ralf@mandelbit.com/

Thanks,
         Antonio


> Because, as explained in this thread and in the RFC, there is enough
> evidence that the current IPv6 dst-caching pattern is racy. If other
> kernel users are expected to live with that race, I cannot force a
> generic fix, but I do not think that is a good reason to require ovpn to
> do the same.
> 
> If you believe the sequence I described is safe, please explain which
> invariant makes it so.
> 

-- 
Antonio Quartulli
OpenVPN Inc.