[PATCH] ipv4: fib: annotate data-race around nh->nh_saddr

Subsystems: networking [general], networking [ipv4/ipv6], the rest

COOLING9d

6 messages, 3 authors, 9d ago · open the first message on its own page

[PATCH] ipv4: fib: annotate data-race around nh->nh_saddr

From: Linkui Xiao <hidden>
Date: 2026-09-11 07:39:26 

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. Annotate it as well so that the value cannot be torn or
reloaded while the per-nexthop scores are being computed.

Fixes: 195374d89368 ("ipv4: fib: annotate races around nh->nh_saddr_genid and nh->nh_saddr")
Signed-off-by: Linkui Xiao <redacted>
---
 net/ipv4/fib_semantics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 7a362f2e2c2b..885c6fae5232 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -2204,7 +2204,7 @@ 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) == saddr)
 			nh_score += 2;
 		if (hash <= nh_upper_bound)
 			nh_score++;
-- 
2.25.1

Re: [PATCH] ipv4: fib: annotate data-race around nh->nh_saddr

From: Kuniyuki Iwashima <kuniyu@google.com>
Date: 2026-09-14 19:51:05 

On Fri, Sep 11, 2026 at 12:38 AM Linkui Xiao [off-list ref] wrote:
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.
quoted hunk
Annotate it as well so that the value cannot be torn or
reloaded while the per-nexthop scores are being computed.

Fixes: 195374d89368 ("ipv4: fib: annotate races around nh->nh_saddr_genid and nh->nh_saddr")
Signed-off-by: Linkui Xiao <redacted>
---
 net/ipv4/fib_semantics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 7a362f2e2c2b..885c6fae5232 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -2204,7 +2204,7 @@ 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) == saddr)
                        nh_score += 2;
                if (hash <= nh_upper_bound)
                        nh_score++;
--
2.25.1

Re: [PATCH] ipv4: fib: annotate data-race around nh->nh_saddr

From: Eric Dumazet <edumazet@google.com>
Date: 2026-09-14 19:57:40 

On Mon, Sep 14, 2026 at 12:51 PM Kuniyuki Iwashima [off-list ref] wrote:
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")

pw-bot: cr

Re: [PATCH] ipv4: fib: annotate data-race around nh->nh_saddr

From: Eric Dumazet <edumazet@google.com>
Date: 2026-09-14 20:21:36 

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..5a8bbcb65daaada1c3ae96339cabc2e801653dac
100644
--- 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++;

Re: [PATCH] ipv4: fib: annotate data-race around nh->nh_saddr

From: Linkui Xiao <hidden>
Date: 2026-09-15 02:37:39 


On 2026/9/15 04:21, Eric Dumazet wrote:
On Mon, Sep 14, 2026 at 12:57 PM Eric Dumazet [off-list ref] wrote:
quoted
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?
Thanks Eric and Kuniyuki for the correction. I'll send a V2 with Fixes: 
32607a332cfe.

Kuniyuki, good catch on the missing genid check. nh_saddr is only 
meaningful when nh_saddr_genid matches dev_addr_genid, and 
fib_select_multipath() currently skips that validation. I'll fold the 
genid check into V2 along with the READ_ONCE annotations, unless you'd 
prefer to send it as a separate patch. Let me know.

Best regards,
Linkui Xiao
quoted hunk
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 50e96f86ca59ab164f764f5d2185d158c14a4a6a..5a8bbcb65daaada1c3ae96339cabc2e801653dac
100644
--- 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++;





  

    
Re: [PATCH] ipv4: fib: annotate data-race around nh->nh_saddr

    

      From: Eric Dumazet <edumazet@google.com>

      Date: 2026-09-15 03:23:55 
    

  

  
On Mon, Sep 14, 2026 at 7:37 PM Linkui Xiao [off-list ref] wrote:


On 2026/9/15 04:21, Eric Dumazet wrote:
quoted
On Mon, Sep 14, 2026 at 12:57 PM Eric Dumazet [off-list ref] wrote:
quoted
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?
Thanks Eric and Kuniyuki for the correction. I'll send a V2 with Fixes:
32607a332cfe.

Kuniyuki, good catch on the missing genid check. nh_saddr is only
meaningful when nh_saddr_genid matches dev_addr_genid, and
fib_select_multipath() currently skips that validation. I'll fold the
genid check into V2 along with the READ_ONCE annotations, unless you'd
prefer to send it as a separate patch. Let me know.
I (Eric) was the one who mentioned the genid thing :)

Send a V2 with both bugs fixed.
Thanks

Best regards,
Linkui Xiao
quoted
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 50e96f86ca59ab164f764f5d2185d158c14a4a6a..5a8bbcb65daaada1c3ae96339cabc2e801653dac
100644
--- 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++;




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help