netdev - Re: [PATCH net] openvswitch: Fix pop_vlan action for double tagged frames

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: <20171219203929.12c1cb93@redhat.com>
Date:   Tue, 19 Dec 2017 20:39:29 +0100
From:   Jiri Benc <jbenc@...hat.com>
To:     Eric Garver <e@...g.me>
Cc:     netdev@...r.kernel.org, ovs-dev@...nvswitch.org
Subject: Re: [PATCH net] openvswitch: Fix pop_vlan action for double tagged
 frames

On Tue, 19 Dec 2017 13:57:53 -0500, Eric Garver wrote:
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -559,8 +559,9 @@ static int parse_nsh(struct sk_buff *skb, struct sw_flow_key *key)
>   *      of a correct length, otherwise the same as skb->network_header.
>   *      For other key->eth.type values it is left untouched.
>   *
> - *    - skb->protocol: the type of the data starting at skb->network_header.
> - *      Equals to key->eth.type.
> + *    - skb->protocol: For Ethernet, the ethertype or VLAN TPID.
> + *      For non-Ethernet, the type of the data starting at skb->network_header
> + *      (also equal to key->eth.type).
>   */
>  static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
>  {
> @@ -579,6 +580,7 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
>  			return -EINVAL;
>  
>  		skb_reset_network_header(skb);
> +		key->eth.type = skb->protocol;
>  	} else {
>  		eth = eth_hdr(skb);
>  		ether_addr_copy(key->eth.src, eth->h_source);
> @@ -592,15 +594,14 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
>  		if (unlikely(parse_vlan(skb, key)))
>  			return -ENOMEM;
>  
> -		skb->protocol = parse_ethertype(skb);
> -		if (unlikely(skb->protocol == htons(0)))
> +		key->eth.type = parse_ethertype(skb);
> +		if (unlikely(key->eth.type == htons(0)))
>  			return -ENOMEM;
>  
>  		skb_reset_network_header(skb);
>  		__skb_push(skb, skb->data - skb_mac_header(skb));
>  	}
>  	skb_reset_mac_len(skb);
> -	key->eth.type = skb->protocol;
>  
>  	/* Network layer. */
>  	if (key->eth.type == htons(ETH_P_IP)) {

Unfortunately, this does not work. key_extract must set skb->protocol
even for Ethernet frames that come from a mixed L2/L3 tunnel. Such
packets will have key->mac_proto set to MAC_PROTO_ETHERNET and
skb->protocol set to ETH_P_TEB (see key_extract_mac_proto). In
key_extract, skb->protocol has to be correctly set to the dissected
value.

Which means that we have to check for the existence of inner vlan tag
(by checking key->eth.cvlan.tci or, perhaps better, by returning it
from parse_vlan) and set skb->protocol accordingly.

 Jiri