netdev - Re: [PATCH v5 net] stmmac: 802.1ad tag stripping fix

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: <20180603.103313.1827081859745223157.davem@davemloft.net>
Date:   Sun, 03 Jun 2018 10:33:13 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     eladv6@...il.com
Cc:     makita.toshiaki@....ntt.co.jp, Jose.Abreu@...opsys.com,
        f.fainelli@...il.com, netdev@...r.kernel.org,
        peppe.cavallaro@...com, alexandre.torgue@...com
Subject: Re: [PATCH v5 net] stmmac: 802.1ad tag stripping fix

From: Elad Nachman <eladv6@...il.com>
Date: Wed, 30 May 2018 08:48:25 +0300

>  static void stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb)
>  {
> -	struct ethhdr *ehdr;
> +	struct vlan_ethhdr *veth;
>  	u16 vlanid;
> +	__be16 vlan_proto;

Please order local variables from longest to shortest line.

>  
> -	if ((dev->features & NETIF_F_HW_VLAN_CTAG_RX) ==
> -	    NETIF_F_HW_VLAN_CTAG_RX &&
> -	    !__vlan_get_tag(skb, &vlanid)) {
> +	if (!__vlan_get_tag(skb, &vlanid)) {
>  		/* pop the vlan tag */
> -		ehdr = (struct ethhdr *)skb->data;
> -		memmove(skb->data + VLAN_HLEN, ehdr, ETH_ALEN * 2);
> +		veth = (struct vlan_ethhdr *)skb->data;
> +		vlan_proto = veth->h_vlan_proto;
> +		memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2);
>  		skb_pull(skb, VLAN_HLEN);
> -		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlanid);
> +		__vlan_hwaccel_put_tag(skb, vlan_proto, vlanid);
>  	}
>  }

I can't see how it is valid to do an unconditional software VLAN
untagging even when VLAN is disabled in the kernel config or the
NETIF_F_* feature bits are not set.

At a minimum that feature test has to stay there, and when it's clear
we let the generic VLAN code untag the packet.