mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry
@ 2026-10-20  3:45 Henry Martin
  2026-10-20  3:48 ` netdev-bot+sinfo
  2026-10-21 10:26 ` Ido Schimmel
  0 siblings, 2 replies; 3+ messages in thread
From: Henry Martin @ 2026-10-20  3:45 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Andrew Lunn, Simon Horman, Kuniyuki Iwashima, Ido Schimmel,
	Petr Machata, Ferruh Yigit, Antoine Tenart, Ilya Maximets,
	Seungwon Bae
  Cc: netdev, linux-kernel, Henry Martin, stable

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry
  2026-10-20  3:45 [PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry Henry Martin
@ 2026-10-20  3:48 ` netdev-bot+sinfo
  2026-10-21 10:26 ` Ido Schimmel
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sinfo @ 2026-10-20  3:48 UTC (permalink / raw)
  To: Henry Martin
  Cc: David S . Miller, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Andrew Lunn, Simon Horman, Kuniyuki Iwashima, Ido Schimmel,
	Petr Machata, Ferruh Yigit, Antoine Tenart, Ilya Maximets,
	Seungwon Bae, netdev, linux-kernel, stable

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry
  2026-10-20  3:45 [PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry Henry Martin
  2026-10-20  3:48 ` netdev-bot+sinfo
@ 2026-10-21 10:26 ` Ido Schimmel
  1 sibling, 0 replies; 3+ messages in thread
From: Ido Schimmel @ 2026-10-21 10:26 UTC (permalink / raw)
  To: Henry Martin
  Cc: David S . Miller, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Andrew Lunn, Simon Horman, Kuniyuki Iwashima, Petr Machata,
	Ferruh Yigit, Antoine Tenart, Ilya Maximets, Seungwon Bae,
	netdev, linux-kernel, stable

On Tue, Oct 20, 2026 at 11:45:31AM +0800, Henry Martin wrote:
> 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>

I agree that it doesn't make sense to append a remote to an FDB entry
that points to a nexthop, but what are the consequences of allowing it?
Does this patch fix a crash / memory leak? If so, the commit message
should describe it. Otherwise, this looks like net-next material
(currently closed).

Also, please add a test case in fib_nexthops.sh, in a similar fashion to
commit 98fc57d16744 ("vxlan: reject dynamic fdb entries that reference a
nexthop id").

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-21 10:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-20  3:45 [PATCH net v2] vxlan: reject adding a remote to a nexthop fdb entry Henry Martin
2026-10-20  3:48 ` netdev-bot+sinfo
2026-10-21 10:26 ` Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®