On 11/4/25 19:15, Petr Machata wrote:
>
> Nikolay Aleksandrov <[email protected]> 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 <[email protected]>
>> ---
>> .../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?
>
ack
> 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".
>
Oh haven't written selftests in awhile, I've missed the shellcheck stuff. :)
>> + 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
>
Thanks for the hint. I'll add this to my patch review process as well.
Cheers,
Nik
>> + 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
>
