From: Seungwon Bae <[email protected]>
An fdb entry that references a nexthop id cannot roam and therefore has no
reason to be aged out, even though vxlan_snoop() keeps refreshing its
timestamp. Nevertheless, such an entry can currently be created (or an
existing nexthop entry updated) with a dynamic state, which places it on
the aging list.
struct nexthop.fdb_list is a per-nexthop global list of the vxlan fdb
entries that reference an fdb-nexthop. It is manipulated by vxlan under
the per-device vxlan->hash_lock only. When two vxlan devices reference the
same fdb-nexthop, their entries share one nh->fdb_list, but each device
takes only its own hash_lock. vxlan_cleanup() - the aging timer - then
runs in softirq with only its device's hash_lock and calls
vxlan_fdb_destroy() -> list_del_rcu(&f->nh_list), racing a concurrent
list_add_tail_rcu()/list_del_rcu() driven from the other device. The
shared list is corrupted and a freed struct vxlan_fdb is left linked on
nh->fdb_list, a use-after-free later consumed by vxlan_fdb_nh_flush().
On CONFIG_DEBUG_LIST/KASAN this reproduces as:
list_del corruption. next->prev should be ..., but was dead000000000122.
__list_del_entry_valid_or_report <- vxlan_fdb_destroy <- vxlan_cleanup
<- run_timer_softirq
BUG: KASAN: slab-use-after-free in vxlan_fdb_destroy
Since a nexthop fdb entry must not be aged out in the first place, reject
making it dynamic, both when it is created and when an existing entry is
updated. vxlan_cleanup() skips NUD_PERMANENT/NUD_NOARP entries, so this
guarantees nexthop fdb entries are never touched by the softirq aging
path, and nh->fdb_list ends up manipulated under RTNL only, removing the
race entirely.
Extend the vxlan fdb-nexthop selftests to cover the rejection on add,
replace and append.
Verified with a KASAN + CONFIG_DEBUG_LIST kernel and an unprivileged
(userns+netns) reproducer that previously ran two vxlan devices churning
dynamic add + aging-delete on a shared fdb-nexthop: before the change the
dynamic nexthop adds succeed and produce hundreds of list_del corruptions
plus a slab-use-after-free; after the change the adds are rejected with
-EINVAL and the run is clean (0 corruptions, 0 KASAN reports).
Fixes: 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries")
Suggested-by: Ido Schimmel <[email protected]>
Signed-off-by: Seungwon Bae <[email protected]>
---
Found with AI assistance; treated as public per
Documentation/process/security-bugs.rst. A reproducer is available
privately on request.
Changes in v2:
- Reject making a nexthop fdb dynamic (on add and update) instead of
adding a lock around nh->fdb_list, per review feedback: a nexthop fdb
cannot roam, so it should never be aged out; this removes the softirq
racer and leaves nh->fdb_list manipulated under RTNL only.
- Add fib_nexthops.sh selftest coverage (add/replace/append rejection).
- Add Fixes tag.
v1: https://lore.kernel.org/netdev/[email protected]/
drivers/net/vxlan/vxlan_core.c | 11 ++++++++
tools/testing/selftests/net/fib_nexthops.sh | 28 +++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index ac88d1c85..93bf7c535 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -996,6 +996,12 @@ static int vxlan_fdb_update_existing(struct vxlan_dev
*vxlan,
return -EOPNOTSUPP;
}
+ if (rcu_access_pointer(f->nh) &&
+ !(state & (NUD_PERMANENT | NUD_NOARP))) {
+ NL_SET_ERR_MSG(extack, "Cannot make a nexthop fdb dynamic");
+ return -EOPNOTSUPP;
+ }
+
/* Do not allow an externally learned entry to take over an entry added
* by the user.
*/
@@ -1257,6 +1263,11 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct
nlattr *tb[],
if (err)
return err;
+ if (nhid && !(ndm->ndm_state & (NUD_PERMANENT | NUD_NOARP))) {
+ NL_SET_ERR_MSG(extack, "A nexthop fdb cannot be dynamic");
+ return -EINVAL;
+ }
+
if (vxlan->default_dst.remote_ip.sa.sa_family != ip.sa.sa_family)
return -EAFNOSUPPORT;
diff --git a/tools/testing/selftests/net/fib_nexthops.sh
b/tools/testing/selftests/net/fib_nexthops.sh
index 3d3471267..431d7bed7 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -533,6 +533,20 @@ ipv6_fdb_grp_fcnal()
run_cmd "$BRIDGE fdb add 02:02:00:00:00:14 dev vx10 nhid 61 self"
log_test $? 255 "Fdb mac add with nexthop"
+ # fdb entries with a nexthop group cannot be aged out
+ run_cmd "$BRIDGE fdb add 02:02:00:00:00:15 dev vx10 nhid 102 self
static"
+ log_test $? 0 "Fdb mac add with nexthop group and static state"
+
+ run_cmd "$BRIDGE fdb add 02:02:00:00:00:16 dev vx10 nhid 102 self
dynamic"
+ log_test $? 255 "Fdb mac add with nexthop group and dynamic state"
+
+ run_cmd "$BRIDGE fdb add 02:02:00:00:00:17 dev vx10 nhid 102 self"
+ run_cmd "$BRIDGE fdb replace 02:02:00:00:00:17 dev vx10 dst
2001:db8:91::11 self dynamic"
+ log_test $? 255 "Fdb mac replace with nexthop group and dynamic state"
+
+ run_cmd "$BRIDGE fdb append 02:02:00:00:00:17 dev vx10 dst
2001:db8:91::11 self dynamic"
+ log_test $? 255 "Fdb mac append with nexthop group and dynamic state"
+
run_cmd "$IP -6 ro add 2001:db8:101::1/128 nhid 66"
log_test $? 2 "Route add with fdb nexthop"
@@ -669,6 +683,20 @@ ipv4_fdb_grp_fcnal()
run_cmd "$BRIDGE fdb add 02:02:00:00:00:14 dev vx10 nhid 12 self"
log_test $? 255 "Fdb mac add with nexthop"
+ # fdb entries with a nexthop group cannot be aged out
+ run_cmd "$BRIDGE fdb add 02:02:00:00:00:15 dev vx10 nhid 102 self
static"
+ log_test $? 0 "Fdb mac add with nexthop group and static state"
+
+ run_cmd "$BRIDGE fdb add 02:02:00:00:00:16 dev vx10 nhid 102 self
dynamic"
+ log_test $? 255 "Fdb mac add with nexthop group and dynamic state"
+
+ run_cmd "$BRIDGE fdb add 02:02:00:00:00:17 dev vx10 nhid 102 self"
+ run_cmd "$BRIDGE fdb replace 02:02:00:00:00:17 dev vx10 dst 10.0.0.3
self dynamic"
+ log_test $? 255 "Fdb mac replace with nexthop group and dynamic state"
+
+ run_cmd "$BRIDGE fdb append 02:02:00:00:00:17 dev vx10 dst 10.0.0.3
self dynamic"
+ log_test $? 255 "Fdb mac append with nexthop group and dynamic state"
+
run_cmd "$IP ro add 172.16.0.0/22 nhid 16"
log_test $? 2 "Route add with fdb nexthop"
--
2.43.0
