Re: [PATCH net] ipv6: fix data race in fib6_metric_set() using cmpxchg

[Hangbin Liu]

On Thu, Mar 26, 2026 at 05:05:57AM -0700, Eric Dumazet wrote:
> > diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> > index dd26657b6a4a..64de761f40d5 100644
> > --- a/net/ipv6/ip6_fib.c
> > +++ b/net/ipv6/ip6_fib.c
> > @@ -730,14 +730,16 @@ void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val)
> >         if (!f6i)
> >                 return;
> >
> > -       if (f6i->fib6_metrics == &dst_default_metrics) {
> > +       if (READ_ONCE(f6i->fib6_metrics) == &dst_default_metrics) {
> > +               struct dst_metrics *dflt = (struct dst_metrics *)&dst_default_metrics;
> >                 struct dst_metrics *p = kzalloc_obj(*p, GFP_ATOMIC);
> >
> >                 if (!p)
> >                         return;
> >
> >                 refcount_set(&p->refcnt, 1);
> > -               f6i->fib6_metrics = p;
> > +               if (cmpxchg(&f6i->fib6_metrics, dflt, p) != dflt)
> > +                       kfree(p);
> >         }
> >
> 
> The following line should happen before the cmpxchg(),
>  ->metrics[X] accesses also need READ_ONCE()/WRITE_ONCE() annotations.

Hi Eric,

Jiayuan also suggested to using READ_ONCE()/WRITE_ONCE() for metrics[X]
accesses. But I don't get why this line should happen before the cmpxchg(),
Would you please help explain?

> 
> 
> >         f6i->fib6_metrics->metrics[metric - 1] = val;
> >

Thanks
Hangbin