linux-kernel - Re: [PATCH net] net: dsa: fix VLAN 0 filter imbalance when toggling filtering

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <04ac4aec-e6cd-4432-a31d-73088e762565@gmail.com>
Date: Thu, 24 Apr 2025 14:34:41 +0200
From: Florian Fainelli <f.fainelli@...il.com>
To: Vladimir Oltean <olteanv@...il.com>, Jonas Gorski <jonas.gorski@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: dsa: fix VLAN 0 filter imbalance when toggling
 filtering



On 4/24/2025 12:25 PM, Vladimir Oltean wrote:
> On Tue, Apr 22, 2025 at 08:49:13PM +0200, Jonas Gorski wrote:
>> When a net device has NETIF_F_HW_VLAN_CTAG_FILTER set, the 8021q code
>> will add VLAN 0 when enabling the device, and remove it on disabling it
>> again.
>>
>> But since we are changing NETIF_F_HW_VLAN_CTAG_FILTER during runtime in
>> dsa_user_manage_vlan_filtering(), user ports that are already enabled
>> may end up with no VLAN 0 configured, or VLAN 0 left configured.
>>
>> E.g.the following sequence would leave sw1p1 without VLAN 0 configured:
>>
>> $ ip link add br0 type bridge vlan_filtering 1
>> $ ip link set br0 up
>> $ ip link set sw1p1 up (filtering is 0, so no HW filter added)
>> $ ip link set sw1p1 master br0 (filtering gets set to 1, but already up)
>>
>> while the following sequence would work:
>>
>> $ ip link add br0 type bridge vlan_filtering 1
>> $ ip link set br0 up
>> $ ip link set sw1p1 master br0 (filtering gets set to 1)
>> $ ip link set sw1p1 up (filtering is 1, HW filter is added)
>>
>> Likewise, the following sequence would leave sw1p2 with a VLAN 0 filter
>> enabled on a vlan_filtering_is_global dsa switch:
>>
>> $ ip link add br0 type bridge vlan_filtering 1
>> $ ip link set br0 up
>> $ ip link set sw1p1 master br0 (filtering set to 1 for all devices)
>> $ ip link set sw1p2 up (filtering is 1, so VLAN 0 filter is added)
>> $ ip link set sw1p1 nomaster (filtering is reset to 0 again)
>> $ ip link set sw1p2 down (VLAN 0 filter is left configured)
>>
>> This even causes untagged traffic to break on b53 after undoing the
>> bridge (though this is partially caused by b53's own doing).
>>
>> Fix this by emulating 8021q's vlan_device_event() behavior when changing
>> the NETIF_F_HW_VLAN_CTAG_FILTER flag, including the printk, so that the
>> absence of it doesn't become a red herring.
>>
>> While vlan_vid_add() has a return value, vlan_device_event() does not
>> check its return value, so let us do the same.
>>
>> Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
>> Signed-off-by: Jonas Gorski <jonas.gorski@...il.com>
>> ---
> 
> Why does the b53 driver depend on VID 0? CONFIG_VLAN_8021Q can be
> disabled or be an unloaded module, how does it work in that case?

This is explained in this commit:

https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=64a81b24487f0d2fba0f033029eec2abc7d82cee

however the case of starting up with CONFIG_VLAN_8021Q and then loading 
the 8021q module was not thought about, arguably I am not sure what sort 
of notification or event we can hook onto in order to react properly to 
that module being loaded. Do you know?
-- 
Florian