Re: [PATCH] ipv4: fib: annotate data-race around nh->nh_saddr
From: Eric Dumazet <edumazet@google.com>
Date: 2026-09-14 20:21:36
Also in:
lkml
Subsystem:
networking [general], networking [ipv4/ipv6], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Ido Schimmel, Linus Torvalds
On Mon, Sep 14, 2026 at 12:57 PM Eric Dumazet [off-list ref] wrote:
On Mon, Sep 14, 2026 at 12:51 PM Kuniyuki Iwashima [off-list ref] wrote:quoted
On Fri, Sep 11, 2026 at 12:38 AM Linkui Xiao [off-list ref] wrote:quoted
From: Linkui Xiao <redacted> fib_select_multipath() compares nexthop_nh->nh_saddr against the flow source address with no lock held, while fib_info_update_nhc_saddr() stores a new value from another CPU as soon as the preferred source address of the egress device changes. Commit 195374d89368 ("ipv4: fib: annotate races around nh->nh_saddr_genid and nh->nh_saddr") added WRITE_ONCE() on the store side and READ_ONCE() in fib_result_prefsrc() after syzbot reported BUG: KCSAN: data-race in fib_select_path / fib_select_path but it only covered that reader. fib_select_multipath(), reached from fib_select_path(), is a second lockless reader of nh->nh_saddr and was left bare.32607a332cfe added the reader after 195374d89368.Indeed, please put in V2: Fixes: 32607a332cfe ("ipv4: prefer multipath nexthop that matches source address")
Adding Willem It seems that this code also lacks a check against genid?
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 50e96f86ca59ab164f764f5d2185d158c14a4a6a..5a8bbcb65daaada1c3ae96339cabc2e801653dac100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c@@ -2185,6 +2185,7 @@ void fib_select_multipath(struct fib_result*res, int hash,
bool use_neigh;
int score = -1;
__be32 saddr;
+ int genid;
if (unlikely(res->fi->nh)) {
nexthop_path_fib_result(res, hash);@@ -2193,6 +2194,7 @@ void fib_select_multipath(struct fib_result*res, int hash,
use_neigh = READ_ONCE(net->ipv4.sysctl_fib_multipath_use_neigh);
saddr = fl4 ? fl4->saddr : 0;
+ genid = saddr ? atomic_read(&net->ipv4.dev_addr_genid) : 0;
change_nexthops(fi) {
int nh_upper_bound, nh_score = 0;@@ -2205,7 +2207,9 @@ void fib_select_multipath(struct fib_result*res, int hash,
(use_neigh && !fib_good_nh(nexthop_nh)))
continue;
- if (saddr && nexthop_nh->nh_saddr == saddr)
+ if (saddr &&
+ READ_ONCE(nexthop_nh->nh_saddr_genid) == genid &&
+ READ_ONCE(nexthop_nh->nh_saddr) == saddr)
nh_score += 2;
if (hash <= nh_upper_bound)
nh_score++;