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

[Jakub Kicinski <kuba@kernel.org>]

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