git: 64f493e664cc - main - bridge: do not allow a bridge SVI in a bridge

The branch main has been updated by ivy:

URL: https://cgit.FreeBSD.org/src/commit/?id=64f493e664cc4bd52c67c6ba2c36a35f68c9fbe4

commit 64f493e664cc4bd52c67c6ba2c36a35f68c9fbe4
Author:     Lexi Winter <ivy@FreeBSD.org>
AuthorDate: 2025-07-28 17:38:02 +0000
Commit:     Lexi Winter <ivy@FreeBSD.org>
CommitDate: 2025-07-28 17:38:02 +0000

    bridge: do not allow a bridge SVI in a bridge
    
    Disallow this:
    
            ifconfig bridge0 create
            ifconfig bridge0.1 create
            ifconfig bridge0 addm bridge0.1
    
    Also disallow this:
    
            ifconfig vlan1 create
            ifconfig bridge0 create
            ifconfig bridge0 addm vlan1
            ifconfig vlan1 vlan 1 vlandev bridge0
    
    Firstly, this panics due to trying to take BRIDGE_LOCK recursively.
    Secondly, even if it worked, it could cause packet forwarding loops.
    
    Reviewed by:    des
    Differential Revision:  https://reviews.freebsd.org/D51310
---
 sys/net/if_bridge.c             | 23 +++++++++++++++++++++++
 sys/net/if_vlan.c               | 12 ++++++++++++
 tests/sys/net/if_bridge_test.sh | 24 ++++++++++++++++++++++++
 tests/sys/net/if_vlan.sh        | 27 +++++++++++++++++++++++++++
 4 files changed, 86 insertions(+)

diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
index 64aaecf6f029..0a35fb4095fb 100644
--- a/sys/net/if_bridge.c
+++ b/sys/net/if_bridge.c
@@ -1335,6 +1335,29 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
 	if (ifs->if_ioctl == NULL)	/* must be supported */
 		return (EXTERROR(EINVAL, "Interface must support ioctl(2)"));
 
+	/*
+	 * If the new interface is a vlan(4), it could be a bridge SVI.
+	 * Don't allow such things to be added to bridges.
+	 */
+	if (ifs->if_type == IFT_L2VLAN) {
+		struct ifnet *parent;
+		struct epoch_tracker et;
+		bool is_bridge;
+
+		/*
+		 * Entering NET_EPOCH with BRIDGE_LOCK held, but this is okay
+		 * since we don't sleep here.
+		 */
+		NET_EPOCH_ENTER(et);
+		parent = VLAN_TRUNKDEV(ifs);
+		is_bridge = (parent != NULL && parent->if_type == IFT_BRIDGE);
+		NET_EPOCH_EXIT(et);
+
+		if (is_bridge)
+			return (EXTERROR(EINVAL,
+			    "Bridge SVI cannot be added to a bridge"));
+	}
+
 	/* If it's in the span list, it can't be a member. */
 	CK_LIST_FOREACH(bif, &sc->sc_spanlist, bif_next)
 		if (ifs == bif->bif_ifp)
diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c
index 22fcb7bf7c64..61000018e5a4 100644
--- a/sys/net/if_vlan.c
+++ b/sys/net/if_vlan.c
@@ -2336,6 +2336,18 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 			error = ENOENT;
 			break;
 		}
+
+		/*
+		 * If the ifp is in a bridge, do not allow setting the device
+		 * to a bridge; this prevents having a bridge SVI as a bridge
+		 * member (which is not permitted).
+		 */
+		if (ifp->if_bridge != NULL && p->if_type == IFT_BRIDGE) {
+			if_rele(p);
+			error = EINVAL;
+			break;
+		}
+
 		if (vlr.vlr_proto == 0)
 			vlr.vlr_proto = ETHERTYPE_VLAN;
 		oldmtu = ifp->if_mtu;
diff --git a/tests/sys/net/if_bridge_test.sh b/tests/sys/net/if_bridge_test.sh
index cd38adea28ad..c0c085f22273 100755
--- a/tests/sys/net/if_bridge_test.sh
+++ b/tests/sys/net/if_bridge_test.sh
@@ -1221,6 +1221,29 @@ vlan_qinq_cleanup()
 	vnet_cleanup
 }
 
+# Adding a bridge SVI to a bridge should not be allowed.
+atf_test_case "bridge_svi_in_bridge" "cleanup"
+bridge_svi_in_bridge_head()
+{
+	atf_set descr 'adding a bridge SVI to a bridge is not allowed (1)'
+	atf_set require.user root
+}
+
+bridge_svi_in_bridge_body()
+{
+	vnet_init
+	vnet_init_bridge
+
+	bridge=$(vnet_mkbridge)
+	atf_check -s exit:0 ifconfig ${bridge}.1 create
+	atf_check -s exit:1 -e ignore ifconfig ${bridge} addm ${bridge}.1
+}
+
+bridge_svi_in_bridge_cleanup()
+{
+	vnet_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "bridge_transmit_ipv4_unicast"
@@ -1247,4 +1270,5 @@ atf_init_test_cases()
 	atf_add_test_case "vlan_ifconfig_tagged"
 	atf_add_test_case "vlan_svi"
 	atf_add_test_case "vlan_qinq"
+	atf_add_test_case "bridge_svi_in_bridge"
 }
diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh
index 424eac705b94..8122203337e2 100755
--- a/tests/sys/net/if_vlan.sh
+++ b/tests/sys/net/if_vlan.sh
@@ -333,6 +333,32 @@ conflict_id_cleanup()
 
 }
 
+# If a vlan interface is in a bridge, changing the vlandev to refer to
+# a bridge should not be allowed.
+atf_test_case "bridge_vlandev" "cleanup"
+bridge_vlandev_head()
+{
+	atf_set descr 'transforming a bridge member vlan into an SVI is not allowed'
+	atf_set require.user root
+}
+
+bridge_vlandev_body()
+{
+	vnet_init
+	vnet_init_bridge
+
+	bridge=$(vnet_mkbridge)
+	vlan=$(vnet_mkvlan)
+
+	atf_check -s exit:0 ifconfig ${bridge} addm ${vlan}
+	atf_check -s exit:1 -e ignore ifconfig ${vlan} vlan 1 vlandev ${bridge}
+}
+
+bridge_vlandev_cleanup()
+{
+	vnet_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "basic"
@@ -343,4 +369,5 @@ atf_init_test_cases()
 	atf_add_test_case "qinq_setflags"
 	atf_add_test_case "bpf_pcp"
 	atf_add_test_case "conflict_id"
+	atf_add_test_case "bridge_vlandev"
 }