netdev - Re: [PATCH 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]

Date:   Mon, 10 Jun 2019 23:09:58 +0000
From:   "Govindarajulu Varadarajan (gvaradar)" <gvaradar@...co.com>
To:     "davem@...emloft.net" <davem@...emloft.net>
CC:     "Christian Benvenuti (benve)" <benve@...co.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net] net: handle 802.1P vlan 0 packets properly

On Mon, 2019-06-10 at 14:28 -0700, David Miller wrote:
> From: Govindarajulu Varadarajan <gvaradar@...co.com>
> Date: Mon, 10 Jun 2019 07:27:02 -0700
> 
> > 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>
> 
> Under Linux we absolutely do not decapsulate the VLAN protocol unless
> a VLAN device is configured on that interface.

Can you clarify on if 802.1P header (with vlan id 0) not supported with inner
802.1Q or 802.1AD header?

We already decapsulate vlan 0 header (802.1P) for all other inner protocols if
vlan 0 device is not present.

For example: pkt - [802.1P vlan 0][IPv4]

In __netif_receive_skb_core():

	if (vlan_do_receive(&skb))
		goto another_round;

vlan_do_receive() returns false since it did not find VLAN 0 device.

Later, we decapsulate vlan 0 header (802.1P) here:

        if (unlikely(skb_vlan_tag_present(skb))) {
                if (skb_vlan_tag_get_id(skb))	//False, because vlan id is 0
                        skb->pkt_type = PACKET_OTHERHOST;
                /* Note: we might in the future use prio bits
                 * and set skb->priority like in vlan_do_receive()
                 * For the time being, just ignore Priority Code Point
                 */
                __vlan_hwaccel_clear_tag(skb);
        }

Then we deliver packet to packet type handler (IPv4) here:

        /* deliver only exact match when indicated */
        if (likely(!deliver_exact)) {
                deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
                                       &ptype_base[ntohs(type) &
                                                   PTYPE_HASH_MASK]);
        }

        deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
                               &orig_dev->ptype_specific);

This does not work if skb->protocol (inner) is 802.1Q or 802.1AD. Since they do
not have packet type handler, pkt is dropped.

Default configuration of our hardware is to priority tag all packets. IMO 802.1P
header with (vlan id 0) should be treated as priority tagged packets, and not as
a vlan tagged packet.

Alternative is, enic driver not setting skb->vlan_tci if vlan_id is 0. IMO driver
should forward pkt as it sees on wire. And stack should handle the headers
properly.

Is there any other way to solve the problem?