netdev - Re: [PATCH net-next] __netif_receive_skb_core: don't untag vlan from skb on DSA master

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] [day] [month] [year] [list]

Message-Id: <20200914.163431.1847224607563732054.davem@davemloft.net>
Date:   Mon, 14 Sep 2020 16:34:31 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     olteanv@...il.com
Cc:     kuba@...nel.org, f.fainelli@...il.com, andrew@...n.ch,
        willemdebruijn.kernel@...il.com, edumazet@...gle.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next] __netif_receive_skb_core: don't untag vlan
 from skb on DSA master

From: Vladimir Oltean <olteanv@...il.com>
Date: Sat, 12 Sep 2020 02:26:07 +0300

> A DSA master interface has upper network devices, each representing an
> Ethernet switch port attached to it. Demultiplexing the source ports and
> setting skb->dev accordingly is done through the catch-all ETH_P_XDSA
> packet_type handler. Catch-all because DSA vendors have various header
> implementations, which can be placed anywhere in the frame: before the
> DMAC, before the EtherType, before the FCS, etc. So, the ETH_P_XDSA
> handler acts like an rx_handler more than anything.
> 
> It is unlikely for the DSA master interface to have any other upper than
> the DSA switch interfaces themselves. Only maybe a bridge upper*, but it
> is very likely that the DSA master will have no 8021q upper. So
> __netif_receive_skb_core() will try to untag the VLAN, despite the fact
> that the DSA switch interface might have an 8021q upper. So the skb will
> never reach that.
> 
> So far, this hasn't been a problem because most of the possible
> placements of the DSA switch header mentioned in the first paragraph
> will displace the VLAN header when the DSA master receives the frame, so
> __netif_receive_skb_core() will not actually execute any VLAN-specific
> code for it. This only becomes a problem when the DSA switch header does
> not displace the VLAN header (for example with a tail tag).
> 
> What the patch does is it bypasses the untagging of the skb when there
> is a DSA switch attached to this net device. So, DSA is the only
> packet_type handler which requires seeing the VLAN header. Once skb->dev
> will be changed, __netif_receive_skb_core() will be invoked again and
> untagging, or delivery to an 8021q upper, will happen in the RX of the
> DSA switch interface itself.
> 
> *see commit 9eb8eff0cf2f ("net: bridge: allow enslaving some DSA master
> network devices". This is actually the reason why I prefer keeping DSA
> as a packet_type handler of ETH_P_XDSA rather than converting to an
> rx_handler. Currently the rx_handler code doesn't support chaining, and
> this is a problem because a DSA master might be bridged.
> 
> Signed-off-by: Vladimir Oltean <olteanv@...il.com>
> Reviewed-by: Florian Fainelli <f.fainelli@...il.com>

Applied, thanks.