* Re: [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel [not found] <20260404004405.4057-2-andrea.mayer@uniroma2.it> @ 2026-04-08 3:20 ` Jakub Kicinski 2026-04-08 23:27 ` Andrea Mayer 0 siblings, 1 reply; 3+ messages in thread From: Jakub Kicinski @ 2026-04-08 3:20 UTC (permalink / raw) To: andrea.mayer Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, horms, dsahern, david.lebrun, stefano.salsano, paolo.lungaroni, nicolas.dichtel, justin.iurman, linux-kernel, shuah, linux-kselftest 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel 2026-04-08 3:20 ` [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel Jakub Kicinski @ 2026-04-08 23:27 ` Andrea Mayer 2026-04-09 0:08 ` Jakub Kicinski 0 siblings, 1 reply; 3+ messages in thread From: Andrea Mayer @ 2026-04-08 23:27 UTC (permalink / raw) To: Jakub Kicinski Cc: netdev, davem, edumazet, pabeni, horms, dsahern, david.lebrun, stefano.salsano, paolo.lungaroni, nicolas.dichtel, justin.iurman, linux-kernel, shuah, linux-kselftest, 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel 2026-04-08 23:27 ` Andrea Mayer @ 2026-04-09 0:08 ` Jakub Kicinski 0 siblings, 0 replies; 3+ messages in thread From: Jakub Kicinski @ 2026-04-09 0:08 UTC (permalink / raw) To: Andrea Mayer Cc: netdev, davem, edumazet, pabeni, horms, dsahern, david.lebrun, stefano.salsano, paolo.lungaroni, nicolas.dichtel, justin.iurman, linux-kernel, shuah, linux-kselftest 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. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-09 0:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260404004405.4057-2-andrea.mayer@uniroma2.it>
2026-04-08 3:20 ` [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel Jakub Kicinski
2026-04-08 23:27 ` Andrea Mayer
2026-04-09 0:08 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox