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: <20250425114225.w24quv7gnp5vlcyd@skbuf>
Date: Fri, 25 Apr 2025 14:42:25 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jonas Gorski <jonas.gorski@...il.com>
Cc: Vladimir Oltean <olteanv@...il.com>,
	Florian Fainelli <f.fainelli@...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>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: dsa: fix VLAN 0 filter imbalance when toggling
 filtering

On Fri, Apr 25, 2025 at 09:52:13AM +0200, Jonas Gorski wrote:
> I gave it a test with a vlan_filtering bridge with no PVID / egress
> untagged vlan defined on a pure software bridge, and STP continued to
> work fine.

STP is not part of the bridge data path, it is control path. The PVID
rules don't apply to it.
In software terms, br_handle_frame() returns RX_HANDLER_PASS for it, it
doesn't go through br_handle_vlan().

> So in a sense, VLAN 0 is needed, as we still need to allow
> untagged traffic to be received regardless of a PVID egress untagged
> VLAN being defined.

When we are talking about the hardware data path of a switchdev port,
that is debatable as well, since many switchdevs have built-in packet
traps which again bypass the VLAN table (a function specific to the
switching layer, like learning, STP state etc). I would argue that the
presence of VID 0 in the RX filtering table is irrelevant for STP as far
as switchdevs are concerned.

> But we shouldn't forward it (except to the cpu port) unless it is part
> of a PVID egress untagged VLAN. This is the tricky part. If (dsa)
> switch drivers ensure that untagged traffic always reaches the cpu
> port, then we can ignore VLAN 0.
> 
> So I think this boils down to that dsa needs a way to pass on to
> drivers whether a VLAN should be forwarded to other members or not
> when adding it to a port.

That can be done (add a struct dsa_db argument to port_vlan_add(),
signifying whether it is a port VLAN or a bridge VLAN), but I haven't
come across switches which can make the distinction. It would require
mapping the same VID, coming from different ports, to different hardware
FIDs.

> Currently, from a dsa driver perspective, the following two scenarios
> would be indistinguishable:
> 
> $ ip link add br0 type bridge vlan_filtering 1
> $ ip link set sw1p1 master br0
> $ ip link set sw1p2 master br0
> $ bridge vlan add dev sw1p1 vid 10
> $ bridge vlan add dev sw2p1 vid 10
> 
> and
> 
> $ ip link add br0 type bridge vlan_filtering 1
> $ ip link set sw1p1 master br0
> $ ip link set sw1p2 master br0
> $ ip link add sw1p1.10 link sw1p1 type vlan id 10
> $ ip link add sw1p2.10 link sw1p2 type vlan id 10
> 
> But in the second case, swp1p1 and sw1p2 should be isolated.
> 
> This is because vlan filters and bridge vlans result in the same
> port_vlan_add() call, with no way of the driver to tell from where the
> call comes from.
> 
> And yes, this is something that is probably hard to configure for many
> smaller embedded switch chips. E.g. b53 supported switches do not have
> forward/flood/etc masks per VLAN, so some cheating/workaround is
> needed here. switchdev.rst says to fall back to software forwarding if
> there is no other way. I have some ideas, but I will first need to
> verify that they work ... .

We have insufficient coverage in dsa_user_prechangeupper_sanity_check()
and dsa_port_can_apply_vlan_filtering(), but we should add another
restriction for this: 8021q uppers with the same VID should not be
installed to ports spanning the same VLAN-aware bridge. And there should
be a new test for it in tools/testing/selftests/net/forwarding/no_forwarding.sh.

The restriction can be selectively lifted if there ever appear drivers
which can make the distinction you are talking about, but I don't think
that any of them can, at the moment.