netdev - Re: [PATCH net-next v2 1/3] openvswitch: normalize vlan rx path

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: <20161005192132.GR25403@egarver>
Date:   Wed, 5 Oct 2016 15:21:32 -0400
From:   Eric Garver <e@...g.me>
To:     Jiri Benc <jbenc@...hat.com>
Cc:     Eyal Birger <eyal.birger@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        pravin shelar <pshelar@....org>
Subject: Re: [PATCH net-next v2 1/3] openvswitch: normalize vlan rx path

On Wed, Oct 05, 2016 at 09:07:09PM +0200, Jiri Benc wrote:
> On Wed, 5 Oct 2016 14:44:26 -0400, Eric Garver wrote:
> > On Wed, Oct 05, 2016 at 08:31:52PM +0300, Eyal Birger wrote:
> > > Just seemed less future safe to keep a pointer to an old packet lying around.
> > 
> > I agree. Alternatively refresh the eth pointer.
> 
> Sorry guys, that just doesn't make sense. Everyone should know that
> reloading of skb pointer means the former pointers to its data may
> become invalid. Please point me to any place in the kernel where we
> reload the data pointer "just because" even when not used.
> 

How about this incremental change?


diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 7ef02752d4ba..0dd36f353c53 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -562,7 +562,6 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	struct sw_flow *flow;
 	struct sw_flow_actions *sf_acts;
 	struct datapath *dp;
-	struct ethhdr *eth;
 	struct vport *input_vport;
 	u16 mru = 0;
 	int len;
@@ -584,14 +583,12 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	nla_memcpy(__skb_put(packet, len), a[OVS_PACKET_ATTR_PACKET], len);
 
 	skb_reset_mac_header(packet);
-	eth = eth_hdr(packet);
 
 	/* Normally, setting the skb 'protocol' field would be handled by a
 	 * call to eth_type_trans(), but it assumes there's a sending
 	 * device, which we may not have. */
-	if (eth_proto_is_802_3(eth->h_proto))
-		packet->protocol = eth->h_proto;
-	else
+	packet->protocol = eth_hdr(packet)->h_proto;
+	if (!eth_proto_is_802_3(packet->protocol))
 		packet->protocol = htons(ETH_P_802_2);
 
 	if (eth_type_vlan(packet->protocol)) {