git: 92c23f6d9c20 - main - vlan: fix setting flags on a QinQ interface

The branch main has been updated by kp:

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

commit 92c23f6d9c2074f6deb0029d13a8c92b32797059
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2023-05-12 08:42:48 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-05-12 09:12:51 +0000

    vlan: fix setting flags on a QinQ interface
    
    Setting vlan flags needlessly takes the exclusive VLAN_XLOCK().
    
    If we have stacked vlan devices (i.e. QinQ) and we set vlan flags (e.g.
    IFF_PROMISC) we call rtnl_handle_ifevent() to send a notification about
    the interface.
    This ends up calling SIOCGIFMEDIA, which requires the VLAN_SLOCK().
    Trying to take that one with the VLAN_XLOCK() held deadlocks us.
    
    There's no need for the exclusive lock though, as we're only accessing
    parent/trunk information, not modifying it, so a shared lock is
    sufficient.
    
    While here also add a test case for this issue.
    
    Backtrace:
            shared lock of (sx) vlan_sx @ /usr/src/sys/net/if_vlan.c:2192
            while exclusively locked from /usr/src/sys/net/if_vlan.c:2307
            panic: excl->share
            cpuid = 29
            time = 1683873033
            KDB: stack backtrace:
            db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe015d4ad4b0
            vpanic() at vpanic+0x152/frame 0xfffffe015d4ad500
            panic() at panic+0x43/frame 0xfffffe015d4ad560
            witness_checkorder() at witness_checkorder+0xcb5/frame 0xfffffe015d4ad720
            _sx_slock_int() at _sx_slock_int+0x67/frame 0xfffffe015d4ad760
            vlan_ioctl() at vlan_ioctl+0xf8/frame 0xfffffe015d4ad7c0
            dump_iface() at dump_iface+0x12f/frame 0xfffffe015d4ad840
            rtnl_handle_ifevent() at rtnl_handle_ifevent+0xab/frame 0xfffffe015d4ad8c0
            if_setflag() at if_setflag+0xf6/frame 0xfffffe015d4ad930
            ifpromisc() at ifpromisc+0x2a/frame 0xfffffe015d4ad960
            vlan_setflags() at vlan_setflags+0x60/frame 0xfffffe015d4ad990
            vlan_ioctl() at vlan_ioctl+0x216/frame 0xfffffe015d4ad9f0
            if_setflag() at if_setflag+0xe4/frame 0xfffffe015d4ada60
            ifpromisc() at ifpromisc+0x2a/frame 0xfffffe015d4ada90
            bridge_ioctl_add() at bridge_ioctl_add+0x499/frame 0xfffffe015d4adb10
            bridge_ioctl() at bridge_ioctl+0x328/frame 0xfffffe015d4adbc0
            ifioctl() at ifioctl+0x972/frame 0xfffffe015d4adcc0
            kern_ioctl() at kern_ioctl+0x1fe/frame 0xfffffe015d4add30
            sys_ioctl() at sys_ioctl+0x154/frame 0xfffffe015d4ade00
            amd64_syscall() at amd64_syscall+0x140/frame 0xfffffe015d4adf30
            fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe015d4adf30
            --- syscall (54, FreeBSD ELF64, ioctl), rip = 0x22b0f0ef8d8a, rsp = 0x22b0ec63f2c8, rbp = 0x22b0ec63f380 ---
            KDB: enter: panic
            [ thread pid 5715 tid 101132 ]
    
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
---
 sys/net/if_vlan.c        |  4 ++--
 tests/sys/net/if_vlan.sh | 29 +++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c
index f5b401c446ed..6aa872a19364 100644
--- a/sys/net/if_vlan.c
+++ b/sys/net/if_vlan.c
@@ -2304,10 +2304,10 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 		 * We should propagate selected flags to the parent,
 		 * e.g., promiscuous mode.
 		 */
-		VLAN_XLOCK();
+		VLAN_SLOCK();
 		if (TRUNK(ifv) != NULL)
 			error = vlan_setflags(ifp, 1);
-		VLAN_XUNLOCK();
+		VLAN_SUNLOCK();
 		break;
 
 	case SIOCADDMULTI:
diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh
index 96426254bb6d..e4c737121bdb 100755
--- a/tests/sys/net/if_vlan.sh
+++ b/tests/sys/net/if_vlan.sh
@@ -221,6 +221,34 @@ qinq_dot_cleanup()
 	vnet_cleanup
 }
 
+atf_test_case "qinq_setflags" "cleanup"
+qinq_setflags_head()
+{
+	atf_set descr 'Test setting flags on a QinQ device'
+	atf_set require.user root
+}
+
+qinq_setflags_body()
+{
+	vnet_init
+
+	epair=$(vnet_mkepair)
+
+	ifconfig ${epair}a up
+	vlan1=$(ifconfig vlan create)
+	ifconfig $vlan1 vlan 1 vlandev ${epair}a
+	vlan2=$(ifconfig vlan create)
+	ifconfig $vlan2 vlan 2 vlandev $vlan1
+
+	# This panics, incorrect locking
+	ifconfig $vlan2 promisc
+}
+
+qinq_setflags_cleanup()
+{
+	vnet_cleanup
+}
+
 atf_test_case "bpf_pcp" "cleanup"
 bpf_pcp_head()
 {
@@ -273,5 +301,6 @@ atf_init_test_cases()
 	atf_add_test_case "qinq_deep"
 	atf_add_test_case "qinq_legacy"
 	atf_add_test_case "qinq_dot"
+	atf_add_test_case "qinq_setflags"
 	atf_add_test_case "bpf_pcp"
 }