git: bdd12889eaa6 - main - if_vlan: handle VID conflicts

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Wed, 22 May 2024 07:41:24 UTC
The branch main has been updated by kp:

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

commit bdd12889eaa64032b3d09ef47e9a6f7081863378
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-05-21 11:31:13 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-05-22 07:08:02 +0000

    if_vlan: handle VID conflicts
    
    If we fail to change the vlan id we have to undo the removal (and vlan id
    change) in the error path. Otherwise we'll have removed the vlan object from the
    hash table, and have the wrong vlan id as well. Subsequent modification attempts
    will then try to remove an entry which doesn't exist, and panic.
    
    Undo the vlan id modification if the insertion in the hash table fails, and
    re-insert it under the original vlan id.
    
    PR:             279195
    Reviewed by:    zlei
    MFC atfer:      1 week
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D45285
---
 sys/net/if_vlan.c        | 10 ++++++++++
 tests/sys/net/if_vlan.sh | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c
index b69d8107e30d..788fdf787249 100644
--- a/sys/net/if_vlan.c
+++ b/sys/net/if_vlan.c
@@ -1715,10 +1715,20 @@ vlan_config(struct ifvlan *ifv, struct ifnet *p, uint16_t vid,
 		ifv->ifv_proto = proto;
 
 		if (ifv->ifv_vid != vid) {
+			int oldvid = ifv->ifv_vid;
+
 			/* Re-hash */
 			vlan_remhash(trunk, ifv);
 			ifv->ifv_vid = vid;
 			error = vlan_inshash(trunk, ifv);
+			if (error) {
+				int ret __diagused;
+
+				ifv->ifv_vid = oldvid;
+				/* Re-insert back where we found it. */
+				ret = vlan_inshash(trunk, ifv);
+				MPASS(ret == 0);
+			}
 		}
 		/* Will unlock */
 		goto done;
diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh
index 458e3cc36bc6..41dac6222cbb 100755
--- a/tests/sys/net/if_vlan.sh
+++ b/tests/sys/net/if_vlan.sh
@@ -297,6 +297,42 @@ bpf_pcp_cleanup()
 	vnet_cleanup
 }
 
+atf_test_case "conflict_id" "cleanup"
+conflict_id_head()
+{
+	atf_set descr 'Test conflicting VLAN IDs, PR #279195'
+	atf_set require.user root
+}
+
+conflict_id_body()
+{
+	vnet_init
+
+	epair=$(vnet_mkepair)
+
+	vnet_mkjail alcatraz ${epair}b
+	vlan_a=$(jexec alcatraz ifconfig vlan create)
+	vlan_b=$(jexec alcatraz ifconfig vlan create)
+
+	jexec alcatraz ifconfig ${vlan_a} vlan 100 vlandev ${epair}b
+	jexec alcatraz ifconfig ${vlan_b} vlan 101 vlandev ${epair}b
+
+	atf_check -s exit:1 -o ignore -e ignore \
+	    jexec alcatraz ifconfig ${vlan_a} vlan 101
+
+	atf_check -s exit:0 -o match:"vlan: 100" \
+	    jexec alcatraz ifconfig ${vlan_a}
+
+	atf_check -s exit:0 -o ignore -e ignore \
+	    jexec alcatraz ifconfig ${vlan_a} vlan 100
+}
+
+conflict_id_cleanup()
+{
+	vnet_cleanup
+
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "basic"
@@ -306,4 +342,5 @@ atf_init_test_cases()
 	atf_add_test_case "qinq_dot"
 	atf_add_test_case "qinq_setflags"
 	atf_add_test_case "bpf_pcp"
+	atf_add_test_case "conflict_id"
 }