[PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry

[PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry

[Henry Martin]

FDB entries backed by a nexthop group must not carry a remote
destination list.  vxlan_fdb_update_existing() currently appends a
remote via vxlan_fdb_append() on an NLM_F_APPEND request with a
multicast or zero MAC even when the entry is backed by a nexthop,
breaking that invariant through three different callers:

- "bridge fdb append" with a dst on a nexthop entry;
- "ip link set dev vx0 type vxlan remote <ip>" (vxlan_changelink());
- "bridge vni add/change ... group <mcast>" on a vnifilter device
  (vxlan_update_default_fdb_entry()).

All three reach the append block with nhid == 0 and a PERMANENT
state, so they pass the earlier nexthop checks.

Reject the combination at the top of vxlan_fdb_update_existing(),
next to the existing nexthop guards, so a refused request has no
side effects: placing the check in the append block instead would
return -EOPNOTSUPP only after the entry state and flags have been
rewritten, skipping both the notification pass and the err_notify
rollback.

Note the resulting behavior change for the two rtnetlink callers:
operations that previously succeeded while silently attaching an
unused remote to a nexthop entry now fail with extack
"Cannot add a remote to a nexthop fdb entry".

This issue was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v2: Move the check before the entry state/flags update, next to the
    other nexthop guards, so a rejected request cannot leave the
    entry half-updated with notifications skipped (reported by
    sashiko, confirmed by Jakub); reword the extack message and
    document the behavior change for the vxlan_changelink() and
    vnifilter callers.

 drivers/net/vxlan/vxlan_core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 3390e341d1e58..83dcede2ca62f 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -1002,6 +1002,15 @@ static int vxlan_fdb_update_existing(struct vxlan_dev *vxlan,
 		return -EOPNOTSUPP;
 	}
 
+	if (rcu_access_pointer(f->nh) &&
+	    (flags & NLM_F_APPEND) &&
+	    (is_multicast_ether_addr(f->key.eth_addr) ||
+	     is_zero_ether_addr(f->key.eth_addr))) {
+		NL_SET_ERR_MSG(extack,
+			       "Cannot add a remote to a nexthop fdb entry");
+		return -EOPNOTSUPP;
+	}
+
 	/* Do not allow an externally learned entry to take over an entry added
 	 * by the user.
 	 */
-- 
2.43.7

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.