mirror of
https://github.com/torvalds/linux.git
synced 2026-10-14 11:54:03 +02:00
ipv4: fib: fix data-race and stale genid check around nh->nh_saddr
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. Commit195374d893("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. Moreover, nh_saddr is only meaningful when nh_saddr_genid matches dev_addr_genid, as established by commit436c3b66ec("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:32607a332c("ipv4: prefer multipath nexthop that matches source address") Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20260916125316.988044-1-xiaolinkui@126.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
parent
651010592b
commit
46bc52d135
|
|
@ -2176,6 +2176,15 @@ static bool fib_good_nh(const struct fib_nh *nh)
|
|||
return !!(state & NUD_VALID);
|
||||
}
|
||||
|
||||
static __be32 fib_nh_saddr(struct net *net, const struct fib_info *fi,
|
||||
struct fib_nh *nh, int genid)
|
||||
{
|
||||
if (READ_ONCE(nh->nh_saddr_genid) == genid)
|
||||
return READ_ONCE(nh->nh_saddr);
|
||||
|
||||
return fib_info_update_nhc_saddr(net, &nh->nh_common, fi->fib_scope);
|
||||
}
|
||||
|
||||
void fib_select_multipath(struct fib_result *res, int hash,
|
||||
const struct flowi4 *fl4)
|
||||
{
|
||||
|
|
@ -2184,6 +2193,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);
|
||||
|
|
@ -2192,6 +2202,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;
|
||||
|
|
@ -2204,7 +2215,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 && fib_nh_saddr(net, fi, nexthop_nh, genid) == saddr)
|
||||
nh_score += 2;
|
||||
if (hash <= nh_upper_bound)
|
||||
nh_score++;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user