[PATCH net] ipv6: add READ_ONCE() annotations to fib6_metrics reader paths

[PATCH net] ipv6: add READ_ONCE() annotations to fib6_metrics reader paths

[Hangbin Liu]

All reader paths that access fib6_metrics on a shared fib6_info need
READ_ONCE() annotations to prevent the compiler from reloading the
pointer and producing inconsistent views.

Without READ_ONCE() in the readers, the compiler may reload fib6_metrics
between two uses. For example, in rt6_set_from(), ip_dst_init_metrics()
is inlined and references fib_metrics twice. If the first load sees
&dst_default_metrics and a subsequent load sees the newly allocated
pointer p, dst->_metrics could end up incorrectly flagged, e.g.

  (&dst_default_metrics) | DST_METRICS_READ_ONLY | DST_METRICS_REFCOUNTED

Fix all reader paths on shared fib6_info objects:

Fixes: d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/net/ip6_fib.h |  4 +++-
 net/ipv6/ip6_fib.c    |  6 ++++--
 net/ipv6/route.c      | 18 +++++++++++-------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 9f8b6814a96a..2dfb04fab5da 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -594,7 +594,9 @@ void fib6_update_sernum_stub(struct net *net, struct fib6_info *f6i);
 void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val);
 static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
 {
-	return !!(f6i->fib6_metrics->metrics[RTAX_LOCK - 1] & (1 << metric));
+	struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
+
+	return !!(m->metrics[RTAX_LOCK - 1] & (1 << metric));
 }
 void fib6_info_hw_flags_set(struct net *net, struct fib6_info *f6i,
 			    bool offload, bool trap, bool offload_failed);
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index dd26657b6a4a..3c96580c2a03 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1144,9 +1144,11 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
 					iter->fib6_flags &= ~RTF_PREFIX_RT;
 				}
 
-				if (rt->fib6_pmtu)
+				u32 pmtu = READ_ONCE(rt->fib6_pmtu);
+
+				if (pmtu)
 					fib6_metric_set(iter, RTAX_MTU,
-							rt->fib6_pmtu);
+							pmtu);
 				return -EEXIST;
 			}
 			/* If we have the same destination and the same metric,
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index cb521700cee7..002a7cda9f6b 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1163,7 +1163,7 @@ static void rt6_set_from(struct rt6_info *rt, struct fib6_info *from)
 {
 	rt->rt6i_flags &= ~RTF_EXPIRES;
 	rcu_assign_pointer(rt->from, from);
-	ip_dst_init_metrics(&rt->dst, from->fib6_metrics);
+	ip_dst_init_metrics(&rt->dst, READ_ONCE(from->fib6_metrics));
 }
 
 /* Caller must already hold reference to f6i in result */
@@ -1635,11 +1635,12 @@ __rt6_find_exception_rcu(struct rt6_exception_bucket **bucket,
 static unsigned int fib6_mtu(const struct fib6_result *res)
 {
 	const struct fib6_nh *nh = res->nh;
+	struct dst_metrics *m;
 	unsigned int mtu;
 
-	if (res->f6i->fib6_pmtu) {
-		mtu = res->f6i->fib6_pmtu;
-	} else {
+	m = READ_ONCE(res->f6i->fib6_metrics);
+	mtu = READ_ONCE(m->metrics[RTAX_MTU - 1]);
+	if (!mtu) {
 		struct net_device *dev = nh->fib_nh_dev;
 		struct inet6_dev *idev;
 
@@ -3300,7 +3301,9 @@ u32 ip6_mtu_from_fib6(const struct fib6_result *res,
 	u32 mtu = 0;
 
 	if (unlikely(fib6_metric_locked(f6i, RTAX_MTU))) {
-		mtu = f6i->fib6_pmtu;
+		struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
+
+		mtu = READ_ONCE(m->metrics[RTAX_MTU - 1]);
 		if (mtu)
 			goto out;
 	}
@@ -5037,7 +5040,8 @@ static int fib6_nh_mtu_change(struct fib6_nh *nh, void *_arg)
 	 */
 	if (nh->fib_nh_dev == arg->dev) {
 		struct inet6_dev *idev = __in6_dev_get(arg->dev);
-		u32 mtu = f6i->fib6_pmtu;
+		struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
+		u32 mtu = READ_ONCE(m->metrics[RTAX_MTU - 1]);
 
 		if (mtu >= arg->mtu ||
 		    (mtu < arg->mtu && mtu == idev->cnf.mtu6))
@@ -5844,7 +5848,7 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
 			goto nla_put_failure;
 	}
 
-	pmetrics = dst ? dst_metrics_ptr(dst) : rt->fib6_metrics->metrics;
+	pmetrics = dst ? dst_metrics_ptr(dst) : READ_ONCE(rt->fib6_metrics)->metrics;
 	if (rtnetlink_put_metrics(skb, pmetrics) < 0)
 		goto nla_put_failure;
 

---
base-commit: 48b3cd69265f346f64b93064723492da46206e9b
change-id: 20260401-fib6_metric_read_once-72f2bba54a02

Best regards,
-- 
Hangbin Liu <liuhangbin@gmail.com>

Re: [PATCH net] ipv6: add READ_ONCE() annotations to fib6_metrics reader paths

[Paolo Abeni]

On 4/3/26 6:16 AM, Hangbin Liu wrote:
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index 9f8b6814a96a..2dfb04fab5da 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -594,7 +594,9 @@ void fib6_update_sernum_stub(struct net *net, struct fib6_info *f6i);
>  void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val);
>  static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
>  {
> -	return !!(f6i->fib6_metrics->metrics[RTAX_LOCK - 1] & (1 << metric));
> +	struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
> +
> +	return !!(m->metrics[RTAX_LOCK - 1] & (1 << metric));

Sashiko notes that here you may want to add an additional READ_ONCE() on
m->metrics[RTAX_LOCK - 1], which in turn looks like more a
follow-up/separate change than a change specific to this patch

>  }
>  void fib6_info_hw_flags_set(struct net *net, struct fib6_info *f6i,
>  			    bool offload, bool trap, bool offload_failed);
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index dd26657b6a4a..3c96580c2a03 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1144,9 +1144,11 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
>  					iter->fib6_flags &= ~RTF_PREFIX_RT;
>  				}
>  
> -				if (rt->fib6_pmtu)
> +				u32 pmtu = READ_ONCE(rt->fib6_pmtu);

Here the READ_ONCE() on rt->metrics is still missing.

> @@ -1635,11 +1635,12 @@ __rt6_find_exception_rcu(struct rt6_exception_bucket **bucket,
>  static unsigned int fib6_mtu(const struct fib6_result *res)
>  {
>  	const struct fib6_nh *nh = res->nh;
> +	struct dst_metrics *m;
>  	unsigned int mtu;
>  
> -	if (res->f6i->fib6_pmtu) {
> -		mtu = res->f6i->fib6_pmtu;
> -	} else {
> +	m = READ_ONCE(res->f6i->fib6_metrics);
> +	mtu = READ_ONCE(m->metrics[RTAX_MTU - 1]);

After this patch there will be a single usage of the `fib6_pmtu` macro.
I think it would be better to entirely drop it and replace with an helper:

static inline unsigned int fib6_mtu(const struct fib6_info *f6i)
{
	const struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
	return READ_ONCE(m->metrics[RTAX_MTU - 1]);
}

/P

Re: [PATCH net] ipv6: add READ_ONCE() annotations to fib6_metrics reader paths

[Hangbin Liu]

On Tue, Apr 07, 2026 at 12:27:30PM +0200, Paolo Abeni wrote:
> On 4/3/26 6:16 AM, Hangbin Liu wrote:
> > diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> > index 9f8b6814a96a..2dfb04fab5da 100644
> > --- a/include/net/ip6_fib.h
> > +++ b/include/net/ip6_fib.h
> > @@ -594,7 +594,9 @@ void fib6_update_sernum_stub(struct net *net, struct fib6_info *f6i);
> >  void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val);
> >  static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
> >  {
> > -	return !!(f6i->fib6_metrics->metrics[RTAX_LOCK - 1] & (1 << metric));
> > +	struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
> > +
> > +	return !!(m->metrics[RTAX_LOCK - 1] & (1 << metric));
> 
> Sashiko notes that here you may want to add an additional READ_ONCE() on
> m->metrics[RTAX_LOCK - 1], which in turn looks like more a
> follow-up/separate change than a change specific to this patch

OK, I will drop this change in the patch.

> 
> >  }
> >  void fib6_info_hw_flags_set(struct net *net, struct fib6_info *f6i,
> >  			    bool offload, bool trap, bool offload_failed);
> > diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> > index dd26657b6a4a..3c96580c2a03 100644
> > --- a/net/ipv6/ip6_fib.c
> > +++ b/net/ipv6/ip6_fib.c
> > @@ -1144,9 +1144,11 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
> >  					iter->fib6_flags &= ~RTF_PREFIX_RT;
> >  				}
> >  
> > -				if (rt->fib6_pmtu)
> > +				u32 pmtu = READ_ONCE(rt->fib6_pmtu);
> 
> Here the READ_ONCE() on rt->metrics is still missing.

Which rt->metrics do you mean? I can't find it..


> 
> > @@ -1635,11 +1635,12 @@ __rt6_find_exception_rcu(struct rt6_exception_bucket **bucket,
> >  static unsigned int fib6_mtu(const struct fib6_result *res)
> >  {
> >  	const struct fib6_nh *nh = res->nh;
> > +	struct dst_metrics *m;
> >  	unsigned int mtu;
> >  
> > -	if (res->f6i->fib6_pmtu) {
> > -		mtu = res->f6i->fib6_pmtu;
> > -	} else {
> > +	m = READ_ONCE(res->f6i->fib6_metrics);
> > +	mtu = READ_ONCE(m->metrics[RTAX_MTU - 1]);

This is already a fib6_mtu() function.
> 
> After this patch there will be a single usage of the `fib6_pmtu` macro.
> I think it would be better to entirely drop it and replace with an helper:
> 
> static inline unsigned int fib6_mtu(const struct fib6_info *f6i)
> {
> 	const struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
> 	return READ_ONCE(m->metrics[RTAX_MTU - 1]);
> }
> 

Should we rename the helper? e.g. get_fib6_mtu ?

Hangbin