Re: [PATCH net] vxlan: reject appending a remote to a nexthop fdb entry

Both findings are correct, thanks.

The Medium one matters: placed in the append block, the new check
returned -EOPNOTSUPP only after the entry state and flags had been
rewritten, skipping the notification pass and the err_notify
rollback and leaving the entry half-updated (f->flags losing
NTF_VXLAN_ADDED_BY_USER for one).

v2 moves the check next to the existing nexthop guards at the top
of vxlan_fdb_update_existing(), before anything is changed, and
rewords the extack message to "Cannot add a remote to a nexthop fdb
entry" so it also reads sanely for the vxlan_changelink() and
vnifilter callers; the changelog now documents that these two
rtnetlink paths change behavior (silent wrong-remote attachment ->
explicit failure).

pw-bot: cr

Regards,
Henry

[off-list ref] 于2026年10月11日周日 15:03写道：

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In vxlan_fdb_update_existing()
  (drivers/net/vxlan/vxlan_core.c), the new `if…
- [Low] The commit message presents the change only as rejecting a
  userspace `bridge fdb append` onto a nexthop entry.

--

vxlan: reject appending a remote to a nexthop fdb entry

This patch adds a check to vxlan_fdb_update_existing(). An NLM_F_APPEND
request on a multicast or zero MAC fdb entry now fails with -EOPNOTSUPP
when the entry is backed by a nexthop group. Before this,
vxlan_fdb_append() would link a remote onto such an entry, which breaks
the rule that nexthop entries have no remote list.

quoted

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 278d41f58d895..e9c5ba7a74172 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c

@@ -1047,6 +1047,11 @@ static int vxlan_fdb_update_existing(struct vxlan_dev *vxlan,
      if ((flags & NLM_F_APPEND) &&
          (is_multicast_ether_addr(f->key.eth_addr) ||
           is_zero_ether_addr(f->key.eth_addr))) {
+             if (rcu_access_pointer(f->nh)) {
+                     NL_SET_ERR_MSG(extack,
+                                    "Cannot append a remote to a nexthop fdb");
+                     return -EOPNOTSUPP;
+             }

[Severity: Medium]
Can this early return leave the entry partly updated? When this check
runs, vxlan_fdb_update_existing() has already written the new state and
flags:

        if (!(fdb_flags & NTF_EXT_LEARNED) ||
            !(f->flags & NTF_VXLAN_ADDED_BY_USER)) {
                if (f->state != state) {
                        f->state = state;
                        notify = 1;
                }
                if (f->flags != fdb_flags) {
                        f->flags = fdb_flags;
                        notify = 1;
                }
        }

Returning -EOPNOTSUPP here skips the if (notify) block, so no
RTM_NEWNEIGH or switchdev notification goes out. It also skips the
err_notify rollback.

For example, create a zero MAC nexthop entry with:

  bridge fdb add 00:00:00:00:00:00 dev vx0 nhid N self permanent

Then run:

  bridge fdb append 00:00:00:00:00:00 dev vx0 dst X self permanent router

The append passes the earlier nexthop checks, because nhid is 0 and the
state includes NUD_PERMANENT. It changes f->flags and then fails.
vxlan_fdb_add() returns the error and restores nothing.

The in-kernel callers vxlan_changelink() and
vxlan_update_default_fdb_entry() also reach this point. They pass
ndm_flags = NTF_SELF and state = NUD_REACHABLE | NUD_PERMANENT.

On a nexthop default entry created by the user, f->flags is overwritten
with NTF_SELF, which clears NTF_VXLAN_ADDED_BY_USER, and then the call
fails. vxlan_changelink() treats the failure as if nothing changed and
frees new_cfg.

The other three nexthop checks near the top of this function all return
before anything is changed. Could this check move up next to them, so
that the failed request has no side effects?

[Severity: Low]
The commit message presents this as rejecting a user's bridge fdb append
onto a nexthop entry. Two in-kernel paths also reach this check with
NLM_F_APPEND | NLM_F_CREATE on all_zeros_mac and nhid = 0:

  vxlan_changelink()
    vxlan_fdb_update()
      vxlan_fdb_update_existing()

  vxlan_update_default_fdb_entry()
    vxlan_fdb_update()
      vxlan_fdb_update_existing()

The first runs on "ip link set dev vx0 type vxlan remote <ip>". The
second runs on vnifilter group add or change, for example
"bridge vni add ... group ...".

On a non-metadata device, the zero MAC lookup is keyed on cfg->vni. A
nexthop entry created with "bridge fdb add 00:00:00:00:00:00 dev vx0
nhid N self permanent" is therefore the same entry these paths update.

Before this patch, these link and VNI changes succeeded, though they left
an unused remote on the entry. With this patch they fail with
-EOPNOTSUPP and the extack "Cannot append a remote to a nexthop fdb".
That message describes an fdb operation the user never issued.

Rejecting these requests is probably the right result. Since the patch
is tagged for stable, should the commit message mention this change in
rtnetlink link and vnifilter behavior? Could the error message also be
made accurate for these callers?

--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261010064802.2741221-1-bsdhenrymartin%40gmail.com