netdev - Re: [PATCH net-next 1/2] openvswitch: remove nonreachable code in vlan parsing

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: <20161005000618.GN25403@egarver>
Date:   Tue, 4 Oct 2016 20:06:18 -0400
From:   Eric Garver <e@...g.me>
To:     Jiri Benc <jbenc@...hat.com>
Cc:     netdev@...r.kernel.org, pravin shelar <pshelar@....org>
Subject: Re: [PATCH net-next 1/2] openvswitch: remove nonreachable code in
 vlan parsing

Hi Jiri,

On Tue, Oct 04, 2016 at 02:30:01PM +0200, Jiri Benc wrote:
> It can never happen that there's a vlan tag in the packet but not in
> skb->vlan_tci. This is ensured in __netif_receive_skb_core and honored by
> skb_vlan_push and skb_vlan_pop. The code dealing with such case is a dead
> code.
>

This code is also called for packets passed back down from userspace
(after the flow key miss and upcall). So it does happen that we have a
skb without skb->vlan_tci set.

See the chain:
ovs_packet_cmd_execute()
  ovs_flow_key_extract_userspace()
    key_extract()
      parse_vlan()

> Moreover, the likely() statement around skb_vlan_tag_present is bogus. This
> code is called whenever flow key is being extracted from the packet. The
> packet may be as likely vlan tagged as not.
> 

I guess the unlikely scenario is the one I mention above.