Re: [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel

Re: [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says "Seems orthogonal but also legit"
---
seg6: separate dst_cache for input and output paths in seg6 lwtunnel

This commit splits the shared seg6 lwtunnel dst_cache into separate input
and output caches to prevent blind reuse across different routing contexts.

> diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
> index 3e1b9991131a2..d6a0f7df90807 100644
> --- a/net/ipv6/seg6_iptunnel.c
> +++ b/net/ipv6/seg6_iptunnel.c

[ ... ]

> @@ -504,7 +505,7 @@ static int seg6_input_core(struct net *net, struct sock *sk,
>  		/* cache only if we don't create a dst reference loop */
>  		if (!dst->error && lwtst != dst->lwtstate) {
>  			local_bh_disable();
> -			dst_cache_set_ip6(&slwt->cache, dst,
> +			dst_cache_set_ip6(&slwt->cache_input, dst,
>  					  &ipv6_hdr(skb)->saddr);
>  			local_bh_enable();
>  		}

This code appears to cache the dst entry resolved just prior by calling
ip6_route_input(skb).

Does ip6_route_input() assign a NOREF dst to the skb?

If so, this dst is protected only by RCU. If the underlying route is
concurrently deleted, its refcount can drop to zero. When
dst_cache_set_ip6() is called, it attempts to use dst_hold(dst).

If the refcount is already zero, dst_hold() will warn and skip incrementing
the refcount, but dst_cache_set_ip6() will still store the pointer.

Could this cause the cache to hold a dangling pointer, leading to a
use-after-free once the RCU grace period expires?

Should this path use skb_dst_force(skb) or dst_hold_safe(dst) to safely
upgrade the dst to a refcounted reference before it is cached?
-- 
pw-bot: cr

Re: [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel

[Andrea Mayer]

On Tue,  7 Apr 2026 20:20:38 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> This is an AI-generated review of your patch. The human sending this
> email says "Seems orthogonal but also legit"
> ---
> seg6: separate dst_cache for input and output paths in seg6 lwtunnel
> 
> This commit splits the shared seg6 lwtunnel dst_cache into separate input
> and output caches to prevent blind reuse across different routing contexts.
> 
> > diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
> > index 3e1b9991131a2..d6a0f7df90807 100644
> > --- a/net/ipv6/seg6_iptunnel.c
> > +++ b/net/ipv6/seg6_iptunnel.c
> 
> [ ... ]
> 
> > @@ -504,7 +505,7 @@ static int seg6_input_core(struct net *net, struct sock *sk,
> >  		/* cache only if we don't create a dst reference loop */
> >  		if (!dst->error && lwtst != dst->lwtstate) {
> >  			local_bh_disable();
> > -			dst_cache_set_ip6(&slwt->cache, dst,
> > +			dst_cache_set_ip6(&slwt->cache_input, dst,
> >  					  &ipv6_hdr(skb)->saddr);
> >  			local_bh_enable();
> >  		}
> 
> This code appears to cache the dst entry resolved just prior by calling
> ip6_route_input(skb).
> 
> Does ip6_route_input() assign a NOREF dst to the skb?
> 
> If so, this dst is protected only by RCU. If the underlying route is
> concurrently deleted, its refcount can drop to zero. When
> dst_cache_set_ip6() is called, it attempts to use dst_hold(dst).
> 
> If the refcount is already zero, dst_hold() will warn and skip incrementing
> the refcount, but dst_cache_set_ip6() will still store the pointer.
> 
> Could this cause the cache to hold a dangling pointer, leading to a
> use-after-free once the RCU grace period expires?
> 
> Should this path use skb_dst_force(skb) or dst_hold_safe(dst) to safely
> upgrade the dst to a refcounted reference before it is cached?
> -- 
> pw-bot: cr

Thanks Jakub.

It does seem orthogonal to the dst_cache split and worth investigating.
I'll take a look.

Andrea

Re: [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel

[Jakub Kicinski]

On Thu, 9 Apr 2026 01:27:08 +0200 Andrea Mayer wrote:
> It does seem orthogonal to the dst_cache split and worth investigating.
> I'll take a look.

I fingered the review send command, to be clear the series we're
commenting on has been applied already.