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: <CAOiHx==5p2O6wVa42YtR-d=Sufbb2Ljy64mFSHavX2bguVXPWg@mail.gmail.com>
Date: Thu, 24 Apr 2025 15:58:50 +0200
From: Jonas Gorski <jonas.gorski@...il.com>
To: Florian Fainelli <f.fainelli@...il.com>
Cc: Vladimir Oltean <olteanv@...il.com>, 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 Thu, Apr 24, 2025 at 2:34 PM Florian Fainelli <f.fainelli@...il.com> wrote:
>
>
>
> 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?

config BRIDGE_VLAN_FILTERING
        bool "VLAN filtering"
        depends on BRIDGE
        depends on VLAN_8021Q

without 8021Q there is no vlan filtering bridge, so filtering can
never be 1, so NETIF_F_HW_VLAN_CTAG_FILTER is never set, so HW filters
for VLAN 0 are never installed or removed, therefore the issue can
never happen.

The issue is only if a vlan filtering bridge was there, and now isn't
anymore, and a previously VLAN 0 HW filter is left intact. This causes
an incomplete vlan entry left programmed in the vlan table of the chip
with just this port as a member, which breaks forwarding for that
VLAN, which is incidentally also the VLAN used for untagged traffic in
the non filtering case.

There are a several issues currently in how b53 handles VLANs,
especially on non filtering bridges. E.g. switchdev.rst says all vlan
configuration should be ignored, but currently it isn't, as b53
dutifully configures any vlans to the hardware passed on from DSA. I
will attempt to fix at a later time, but first I wanted to make sure
that switchdev/dsa/vlan/etc subsystems are working correctly.

Best regards,
Jonas