Re: [PATCH net v3] ipv4: fib: fix data-race and stale genid check around nh->nh_saddr

[Ido Schimmel]

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

Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx>