Skip to content

Commit ffb5a48

Browse files
liuhangbinkuba-moo
authored andcommitted
ipv6: fix data race in fib6_metric_set() using cmpxchg
fib6_metric_set() may be called concurrently from softirq context without holding the FIB table lock. A typical path is: ndisc_router_discovery() spin_unlock_bh(&table->tb6_lock) <- lock released fib6_metric_set(rt, RTAX_HOPLIMIT, ...) <- lockless call When two CPUs process Router Advertisement packets for the same router simultaneously, they can both arrive at fib6_metric_set() with the same fib6_info pointer whose fib6_metrics still points to dst_default_metrics. if (f6i->fib6_metrics == &dst_default_metrics) { /* both CPUs: true */ struct dst_metrics *p = kzalloc_obj(*p, GFP_ATOMIC); refcount_set(&p->refcnt, 1); f6i->fib6_metrics = p; /* CPU1 overwrites CPU0's p -> p0 leaked */ } The dst_metrics allocated by the losing CPU has refcnt=1 but no pointer to it anywhere in memory, producing a kmemleak report: unreferenced object 0xff1100025aca1400 (size 96): comm "softirq", pid 0, jiffies 4299271239 backtrace: kmalloc_trace+0x28a/0x380 fib6_metric_set+0xcd/0x180 ndisc_router_discovery+0x12dc/0x24b0 icmpv6_rcv+0xc16/0x1360 Fix this by: - Set val for p->metrics before published via cmpxchg() so the metrics value is ready before the pointer becomes visible to other CPUs. - Replace the plain pointer store with cmpxchg() and free the allocation safely when competition failed. - Add READ_ONCE()/WRITE_ONCE() for metrics[] setting in the non-default metrics path to prevent compiler-based data races. Fixes: d4ead6b ("net/ipv6: move metrics from dst to rt6_info") Reported-by: Fei Liu <feliu@redhat.com> Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20260331-b4-fib6_metric_set-kmemleak-v3-1-88d27f4d8825@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 48b3cd6 commit ffb5a48

1 file changed

Lines changed: 11 additions & 3 deletions

File tree

net/ipv6/ip6_fib.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -727,20 +727,28 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
727727

728728
void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val)
729729
{
730+
struct dst_metrics *m;
731+
730732
if (!f6i)
731733
return;
732734

733-
if (f6i->fib6_metrics == &dst_default_metrics) {
735+
if (READ_ONCE(f6i->fib6_metrics) == &dst_default_metrics) {
736+
struct dst_metrics *dflt = (struct dst_metrics *)&dst_default_metrics;
734737
struct dst_metrics *p = kzalloc_obj(*p, GFP_ATOMIC);
735738

736739
if (!p)
737740
return;
738741

742+
p->metrics[metric - 1] = val;
739743
refcount_set(&p->refcnt, 1);
740-
f6i->fib6_metrics = p;
744+
if (cmpxchg(&f6i->fib6_metrics, dflt, p) != dflt)
745+
kfree(p);
746+
else
747+
return;
741748
}
742749

743-
f6i->fib6_metrics->metrics[metric - 1] = val;
750+
m = READ_ONCE(f6i->fib6_metrics);
751+
WRITE_ONCE(m->metrics[metric - 1], val);
744752
}
745753

746754
/*

0 commit comments

Comments
 (0)