Re: [RFC PATCH net 2/4] net: dsa: prevent hardware forwarding between unbridged 8021q uppers

On Tue, Mar 09, 2021 at 04:16, Vladimir Oltean [off-list ref] wrote:

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Tobias reports that the following set of commands, which bridge two
ports that have 8021q uppers with the same VID, is incorrectly accepted
by DSA as valid:

.100  br0  .100
   \  / \  /
   lan0 lan1

ip link add dev br0 type bridge vlan_filtering 1
ip link add dev lan0.100 link lan0 type vlan id 100
ip link add dev lan1.100 link lan1 type vlan id 100

If I move this line...

ip link set dev lan0 master br0
ip link set dev lan1 master br0 # This should fail but doesn't

...down here, the config is (erroneously) accepted.

Again, this is a variation of the same theme of 'all VLANs kinda smell
the same in hardware, you can't tell if they came from 8021q or from the
bridge'. When the base interfaces are bridged, the expectation of the
Linux network stack is that traffic received by other upper interfaces
except the bridge is not captured by the bridge rx_handler, therefore
not subject to forwarding. So the above setup should not do forwarding
for VLAN ID 100, but it does it nonetheless. So it should be denied.

Reported-by: Tobias Waldekranz <tobias@waldekranz.com>
Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---

This is what I meant by having bits and pieces of this validation
scattered in multiple places, some things being checked for certain
events but not for others, etc.

I took an initial stab at this to show what I mean:

https://lore.kernel.org/netdev/20210309184244.1970173-1-tobias@waldekranz.com (local)

I am sure there are holes in this as well, hence RFC, but I think it
will be much easier to make sure that we avoid ordering issues using a
structure like this.

What do you think?