d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

vlan: fix setting flags on a QinQ interface

Browse Source 
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")
This commit is contained in:
Kristof Provost 2023-05-12 10:42:48 +02:00
parent cbe88c6f3c
commit 92c23f6d9c
2 changed files with 31 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
sys/net/if_vlan.c
View File

@ -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:

29
tests/sys/net/if_vlan.sh
View File

@ -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"
}