Re: [PATCH net 2/2] selftests: forwarding: bridge: add a state bypass with disabled VLAN filtering test

[Petr Machata <petrm@nvidia.com>]

Nikolay Aleksandrov <razor@blackwall.org> writes:

> Add a test which checks that port state bypass cannot happen if we have
> VLAN filtering disabled and MST enabled. Such bypass could lead to race
> condition when deleting a port because learning may happen after its
> state has been toggled to disabled while it's being deleted, leading to
> a use after free.
>
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  .../net/forwarding/bridge_vlan_unaware.sh     | 35 ++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/forwarding/bridge_vlan_unaware.sh b/tools/testing/selftests/net/forwarding/bridge_vlan_unaware.sh
> index 2b5700b61ffa..20769793310e 100755
> --- a/tools/testing/selftests/net/forwarding/bridge_vlan_unaware.sh
> +++ b/tools/testing/selftests/net/forwarding/bridge_vlan_unaware.sh
> @@ -1,7 +1,7 @@
>  #!/bin/bash
>  # SPDX-License-Identifier: GPL-2.0
>  
> -ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding pvid_change"
> +ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding pvid_change mst_state_no_bypass"

I think you'll need to adjust this test for v2, can you please change
the above line to the following while at it?

ALL_TESTS="
	ping_ipv4
	ping_ipv6
	learning
	flooding
	pvid_change
	mst_state_no_bypass
"

>  NUM_NETIFS=4
>  source lib.sh
>  
> @@ -114,6 +114,39 @@ pvid_change()
>  	ping_ipv6 " with bridge port $swp1 PVID deleted"
>  }
>  
> +mst_state_no_bypass()
> +{
> +	local mac=de:ad:be:ef:13:37
> +
> +	# Test that port state isn't bypassed when MST is enabled and VLAN
> +	# filtering is disabled
> +	RET=0
> +
> +	# MST can be enabled only when there are no VLANs
> +	bridge vlan del vid 1 dev $swp1
> +	bridge vlan del vid 1 dev $swp2

Pretty sure these naked references will explode in the CI's shellcheck.
I expect they'll have to be quoted as "$swp1".

> +	bridge vlan del vid 1 dev br0 self
> +
> +	ip link set br0 type bridge mst_enabled 1
> +	check_err $? "Could not enable MST"
> +
> +	bridge link set dev $swp1 state disabled

Here as well. And more cases are below.

I've got this in my bash history. Might come in handy.

    cat files | while read file; do git show net-next/main:$file > file; shellcheck file > sc-old; git show HEAD:$file > file; shellcheck file > sc-new; echo $file; diff -u sc-old sc-new; done | less

> +	check_err $? "Could not set port state"
> +
> +	$MZ $h1 -c 1 -p 64 -a $mac -t ip -q
> +
> +	bridge fdb show brport $swp1 | grep -q de:ad:be:ef:13:37
> +	check_fail $? "FDB entry found when it shouldn't be"
> +
> +	log_test "VLAN filtering disabled and MST enabled port state no bypass"
> +
> +	ip link set br0 type bridge mst_enabled 0
> +	bridge link set dev $swp1 state forwarding
> +	bridge vlan add vid 1 dev $swp1 pvid untagged
> +	bridge vlan add vid 1 dev $swp2 pvid untagged
> +	bridge vlan add vid 1 dev br0 self
> +}
> +
>  trap cleanup EXIT
>  
>  setup_prepare