mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Linkui Xiao <xiaolinkui@126.com>
Cc: dsahern@kernel.org, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linkui Xiao <xiaolinkui@kylinos.cn>
Subject: Re: [PATCH net v3] ipv4: fib: fix data-race and stale genid check around nh->nh_saddr
Date: Wed, 16 Sep 2026 17:35:20 +0300	[thread overview]
Message-ID: <20260916143520.GA1046072@shredder> (raw)
In-Reply-To: <20260916125316.988044-1-xiaolinkui@126.com>

On Wed, Sep 16, 2026 at 08:53:16PM +0800, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> 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.

Nit (not worth a v4): This reads as if 195374d89368 missed
fib_select_multipath(), but back then this function didn't use nh_saddr.

> 
> Moreover, nh_saddr is only meaningful when nh_saddr_genid matches
> dev_addr_genid, as established by commit 436c3b66ec98 ("ipv4: Invalidate
> nexthop cache nh_saddr more correctly."). fib_select_multipath()
> skips that validation, so it can score a nexthop using a stale source
> address and skew the ECMP selection.
> 
> Annotate both reads with READ_ONCE() and refresh the cached source
> address via fib_info_update_nhc_saddr() when the genid does not match,
> mirroring fib_result_prefsrc().
> 
> Fixes: 32607a332cfe ("ipv4: prefer multipath nexthop that matches source address")
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

  reply	other threads:[~2026-09-16 14:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16 12:53 Linkui Xiao
2026-09-16 14:35 ` Ido Schimmel [this message]
2026-09-16 14:40   ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260916143520.GA1046072@shredder \
    --to=idosch@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=xiaolinkui@126.com \
    --cc=xiaolinkui@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®