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

From: Tobias Waldekranz <[email-masked]>
Date: Tue, 09 Mar 2021 19:56:11 +0100

On Tue, Mar 09, 2021 at 04:16, Vladimir Oltean <[email-masked]> wrote:
> From: Vladimir Oltean <[email-masked]>
>
> 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 <[email-masked]>
> Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")
> Signed-off-by: Vladimir Oltean <[email-masked]>
> ---

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/[email-masked]

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?