netdev - Re: [PATCH RESEND net] net: handle 802.1P vlan 0 packets properly

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: <13332a7b-bd3d-e546-27d1-402ed8013f41@gmail.com>
Date:   Tue, 11 Jun 2019 13:34:33 +0900
From:   Toshiaki Makita <toshiaki.makita1@...il.com>
To:     Govindarajulu Varadarajan <gvaradar@...co.com>, benve@...co.com,
        davem@...emloft.net, netdev@...r.kernel.org,
        govind.varadar@...il.com
Cc:     ssuryaextr@...il.com
Subject: Re: [PATCH RESEND net] net: handle 802.1P vlan 0 packets properly

On 2019/06/11 3:31, Govindarajulu Varadarajan wrote:
> When stack receives pkt: [802.1P vlan 0][802.1AD vlan 100][IPv4],
> vlan_do_receive() returns false if it does not find vlan_dev. Later
> __netif_receive_skb_core() fails to find packet type handler for
> skb->protocol 801.1AD and drops the packet.
> 
> 801.1P header with vlan id 0 should be handled as untagged packets.
> This patch fixes it by checking if vlan_id is 0 and processes next vlan
> header.
> 
> Signed-off-by: Govindarajulu Varadarajan <gvaradar@...co.com>
> ---
>   net/8021q/vlan_core.c | 24 +++++++++++++++++++++---
>   1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index a313165e7a67..0cde54c02c3f 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -9,14 +9,32 @@
>   bool vlan_do_receive(struct sk_buff **skbp)
>   {
>   	struct sk_buff *skb = *skbp;
> -	__be16 vlan_proto = skb->vlan_proto;
> -	u16 vlan_id = skb_vlan_tag_get_id(skb);
> +	__be16 vlan_proto;
> +	u16 vlan_id;
>   	struct net_device *vlan_dev;
>   	struct vlan_pcpu_stats *rx_stats;
>   
> +again:
> +	vlan_proto = skb->vlan_proto;
> +	vlan_id = skb_vlan_tag_get_id(skb);
>   	vlan_dev = vlan_find_dev(skb->dev, vlan_proto, vlan_id);
> -	if (!vlan_dev)
> +	if (!vlan_dev) {
> +		/* Incase of 802.1P header with vlan id 0, continue if
> +		 * vlan_dev is not found.
> +		 */
> +		if (unlikely(!vlan_id)) {
> +			__vlan_hwaccel_clear_tag(skb);

Looks like this changes existing behavior. Priority-tagged packets will be untagged
before bridge, etc. I think priority-tagged packets should be forwarded as priority-tagged
(iff bridge is not vlan-aware), not untagged.

> +			if (skb->protocol == cpu_to_be16(ETH_P_8021Q) ||
> +			    skb->protocol == cpu_to_be16(ETH_P_8021AD)) {
> +				skb = skb_vlan_untag(skb);
> +				*skbp = skb;
> +				if (unlikely(!skb))
> +					return false;
> +				goto again;
> +			}
> +		}
>   		return false;
> +	}
>   
>   	skb = *skbp = skb_share_check(skb, GFP_ATOMIC);
>   	if (unlikely(!skb))
> 

Toshiaki Makita