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

[Eric Dumazet]

On Wed, Sep 16, 2026 at 7:35 AM Ido Schimmel <idosch@xxxxxxxxxx> wrote:
>
> 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>

Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>