[PATCH net v2] vxlan: reject dynamic fdb entries that reference a nexthop id

[PATCH net v2] vxlan: reject dynamic fdb entries that reference a nexthop id

[qotmddnjs]

From: Seungwon Bae <qotmddnjs@ajou.ac.kr>

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 <idosch@nvidia.com>
Signed-off-by: Seungwon Bae <qotmddnjs@ajou.ac.kr>
---
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/20260901050253.47197-1-qotmddnjs@ajou.ac.kr/
 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

Re: [PATCH net v2] vxlan: reject dynamic fdb entries that reference a nexthop id

[Ido Schimmel]

I don't understand.

v1 was submitted at 5:02 UTC. I replied at 11:36 UTC asking:

"Please read [2] and run [3] and [4] on v2 and future submissions"

Where the fourth line in [2] is "don’t repost your patches within one
24h period".

31 minutes after my reply you posted v2. So, did you ignore everything I
wrote and just took the diff?

[2] https://docs.kernel.org/next/process/maintainer-netdev.html

On Tue, Sep 01, 2026 at 09:07:40PM +0900, qotmddnjs wrote:
> From: Seungwon Bae <qotmddnjs@ajou.ac.kr>
> 
> 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).

The commit message needs to better explain why it's safe to prevent
dynamic FDB entries from pointing to FDB nexthops. Something like:

"Cited commit in the Fixes tag allowed VXLAN FDB entries to point to FDB
nexthops so that overlay traffic could be load balanced across multiple
VTEPs. Such entries can only be configured from user space, cannot be
learned and cannot roam. They only make sense with a user space control
plane such as E-VPN where data plane learning is disabled.

Despite that, the VXLAN driver does not currently prevent such entries
from being configured with the "dynamic" flag. The per-nexthop FDB list
is only protected by the per-device hash lock, which is not sufficient
when two VXLAN devices point to the same FDB nexthop and therefore share
the list. Aging runs in softirq context without RTNL, so an entry
deleted by one device can race with an addition or deletion from the
other, leading to list corruption:

<put the trace here>

Fix this by rejecting the bogus configuration of dynamic FDB entries
that point to FDB nexthops. As such, the per-nexthop FDB list is only
ever mutated under the RTNL lock. Add test cases to make sure that this
does not regress in the future."