linux-kernel - Re: [PATCH net] net: dsa: tag_brcm: legacy: fix untagged rx on unbridged ports for bcm63xx

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: <aO_H6187Oahh24IX@horms.kernel.org>
Date: Wed, 15 Oct 2025 17:12:27 +0100
From: Simon Horman <horms@...nel.org>
To: Jonas Gorski <jonas.gorski@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Álvaro Fernández Rojas <noltari@...il.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: dsa: tag_brcm: legacy: fix untagged rx on
 unbridged ports for bcm63xx

On Wed, Oct 15, 2025 at 09:08:54AM +0200, Jonas Gorski wrote:
> The internal switch on BCM63XX SoCs will unconditionally add 802.1Q VLAN
> tags on egress to CPU when 802.1Q mode is enabled. We do this
> unconditionally since commit ed409f3bbaa5 ("net: dsa: b53: Configure
> VLANs while not filtering").
> 
> This is fine for VLAN aware bridges, but for standalone ports and vlan
> unaware bridges this means all packets are tagged with the default VID,
> which is 0.
> 
> While the kernel will treat that like untagged, this can break userspace
> applications processing raw packets, expecting untagged traffic, like
> STP daemons.
> 
> This also breaks several bridge tests, where the tcpdump output then
> does not match the expected output anymore.
> 
> Since 0 isn't a valid VID, just strip out the VLAN tag if we encounter
> it, unless the priority field is set, since that would be a valid tag
> again.
> 
> Fixes: 964dbf186eaa ("net: dsa: tag_brcm: add support for legacy tags")
> Signed-off-by: Jonas Gorski <jonas.gorski@...il.com>

...

> @@ -237,8 +239,14 @@ static struct sk_buff *brcm_leg_tag_rcv(struct sk_buff *skb,
>  	if (!skb->dev)
>  		return NULL;
>  
> -	/* VLAN tag is added by BCM63xx internal switch */
> -	if (netdev_uses_dsa(skb->dev))
> +	/* The internal switch in BCM63XX SoCs will add a 802.1Q VLAN tag on
> +	 * egress to the CPU port for all packets, regardless of the untag bit
> +	 * in the VLAN table.  VID 0 is used for untagged traffic on unbridged
> +	 * ports and vlan unaware bridges. If we encounter a VID 0 tagged
> +	 * packet, we know it is supposed to be untagged, so strip the VLAN
> +	 * tag as well in that case.

Maybe it isn't important, but here it is a TCI 0 that is being checked:
VID 0, PCP 0, and DEI 0.

> +	 */
> +	if (proto[0] == htons(ETH_P_8021Q) && proto[1] == 0)
>  		len += VLAN_HLEN;
>  
>  	/* Remove Broadcom tag and update checksum */
> 
> base-commit: 7f0fddd817ba6daebea1445ae9fab4b6d2294fa8
> -- 
> 2.43.0
>