Netdev List
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Kuniyuki Iwashima <kuniyu@google.com>
Cc: David Ahern <dsahern@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>, Wei Wang <weiwan@google.com>,
	Marc Harvey <marcharvey@google.com>,
	Kuniyuki Iwashima <kuni1840@gmail.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH v1 net] ipv6: Fix dst leak for uncached routes.
Date: Sun, 20 Sep 2026 20:26:28 +0300	[thread overview]
Message-ID: <20260920172628.GA2051643@shredder> (raw)
In-Reply-To: <20260918041439.2575935-1-kuniyu@google.com>

On Fri, Sep 18, 2026 at 04:14:37AM +0000, Kuniyuki Iwashima wrote:
> ip6_route_output_flags(), ip6_rt_put_flags(), and ip6_dst_check()
> detect an uncached route by list_empty(&rt->dst.rt_uncached),
> which replaced the static DST_NOCACHE flag check in commit
> a4c2fd7f7891 ("net: remove DST_NOCACHE flag").
> 
> When a device is unregistered, rt6_uncached_list_flush_dev()
> unlinks uncached routes tied to the device from rt6_uncached_list.
> 
> Previously, they were moved to another list with list_move()
> (__list_del_entry() + list_add()), and since commit 98aa546af5e4
> ("inet: remove (struct uncached_list)->quarantine"), the routes
> are just unlinked with list_del_init().
> 
> If list_del_init() runs concurrently, list_empty() evaluates to
> true; ip6_route_output_flags() calls dst_hold_safe() incorrectly
> and ip6_rt_put_flags() skips ip6_rt_put(), leaking dst, and thus
> dev tied via rt->from as well.
> 
> The same race is partially fixed by commit 9a6f0c4d5796 ("dst:
> fix races in rt6_uncached_list_del() and rt_del_uncached_list()").
> 
> Let's check rt6->dst.rt_uncached_list instead.

Please add a comment that we now rely on this field being written once (never
NULL-ed). Otherwise it's quite fragile. Something like:

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 36e999b22d42..a6b21edbda29 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -139,6 +139,7 @@ void rt6_uncached_list_add(struct rt6_info *rt)
 {
 	struct uncached_list *ul = raw_cpu_ptr(&rt6_uncached_list);
 
+	/* Set once and never cleared: non-NULL marks an uncached route. */
 	rt->dst.rt_uncached_list = ul;
 
 	spin_lock_bh(&ul->lock);

> 
> Note that IPv4 does not have the same issue.
> 
> Fixes: 7d9e5f422150 ("ipv6: convert major tx path to use RT6_LOOKUP_F_DST_NOREF")
> Fixes: d64a1f574a29 ("ipv6: honor RT6_LOOKUP_F_DST_NOREF in rule lookup logic")
> Fixes: a4c2fd7f7891 ("net: remove DST_NOCACHE flag")

The commit message reads as if the problem is only visible since
98aa546af5e4 (v6.11), so why blame commits going back to v4.13?

> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  include/net/ip6_route.h | 2 +-
>  net/ipv6/route.c        | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index b9e8d2b759e9..2c5ded121f54 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -106,7 +106,7 @@ static inline struct dst_entry *ip6_route_output(struct net *net,
>  static inline void ip6_rt_put_flags(struct rt6_info *rt, int flags)
>  {
>  	if (!(flags & RT6_LOOKUP_F_DST_NOREF) ||
> -	    !list_empty(&rt->dst.rt_uncached))
> +	    rt->dst.rt_uncached_list)
>  		ip6_rt_put(rt);
>  }
>  
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 08bd68f1b5bb..b18cd0d9148c 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2722,7 +2722,7 @@ struct dst_entry *ip6_route_output_flags(struct net *net,
>  	dst = ip6_route_output_flags_noref(net, sk, fl6, flags);
>  	rt6 = dst_rt6_info(dst);
>  	/* For dst cached in uncached_list, refcnt is already taken. */
> -	if (list_empty(&rt6->dst.rt_uncached) && !dst_hold_safe(dst)) {
> +	if (!rt6->dst.rt_uncached_list && !dst_hold_safe(dst)) {
>  		dst = &net->ipv6.ip6_null_entry->dst;
>  		dst_hold(dst);
>  	}

Nit: The comments here and above ip6_rt_put_flags() are now stale given
that we no longer check for membership in uncached_list. You can change
to something like:

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 2c5ded121f54..0f9b7a260d25 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -101,7 +101,7 @@ static inline struct dst_entry *ip6_route_output(struct net *net,
 }
 
 /* Only conditionally release dst if flags indicates
- * !RT6_LOOKUP_F_DST_NOREF or dst is in uncached_list.
+ * !RT6_LOOKUP_F_DST_NOREF or dst is uncached.
  */
 static inline void ip6_rt_put_flags(struct rt6_info *rt, int flags)
 {
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 36e999b22d42..515115abe2f0 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2721,7 +2721,7 @@ struct dst_entry *ip6_route_output_flags(struct net *net,
 	rcu_read_lock();
 	dst = ip6_route_output_flags_noref(net, sk, fl6, flags);
 	rt6 = dst_rt6_info(dst);
-	/* For dst cached in uncached_list, refcnt is already taken. */
+	/* For an uncached dst, refcnt is already taken. */
 	if (!rt6->dst.rt_uncached_list && !dst_hold_safe(dst)) {
 		dst = &net->ipv6.ip6_null_entry->dst;
 		dst_hold(dst);

  parent reply	other threads:[~2026-09-20 17:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18  4:14 [PATCH v1 net] ipv6: Fix dst leak for uncached routes Kuniyuki Iwashima
2026-09-20  2:36 ` Hangbin Liu
2026-09-20  7:39 ` Xuanqiang Luo
2026-09-20 17:26 ` Ido Schimmel [this message]
2026-09-20 19:01   ` Kuniyuki Iwashima
2026-09-21  4:17 ` netdev-bot+sashiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260920172628.GA2051643@shredder \
    --to=idosch@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuni1840@gmail.com \
    --cc=kuniyu@google.com \
    --cc=marcharvey@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=weiwan@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox