Przeglądaj źródła

openvswitch: Fix vport_send double free

Today vport-send has complex error handling because it involves
freeing skb and updating stats depending on return value from
vport send implementation.
This can be simplified by delegating responsibility of freeing
skb to the vport implementation for all cases. So that
vport-send needs just update stats.

Fixes: 91b7514cdf ("openvswitch: Unify vport error stats
handling")
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Pravin B Shelar 10 lat temu
rodzic
commit
997e068ebc

+ 5 - 1
net/ipv4/geneve.c

@@ -122,14 +122,18 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
 	int err;
 	int err;
 
 
 	skb = udp_tunnel_handle_offloads(skb, !gs->sock->sk->sk_no_check_tx);
 	skb = udp_tunnel_handle_offloads(skb, !gs->sock->sk->sk_no_check_tx);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
 
 
 	min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
 	min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
 			+ GENEVE_BASE_HLEN + opt_len + sizeof(struct iphdr)
 			+ GENEVE_BASE_HLEN + opt_len + sizeof(struct iphdr)
 			+ (vlan_tx_tag_present(skb) ? VLAN_HLEN : 0);
 			+ (vlan_tx_tag_present(skb) ? VLAN_HLEN : 0);
 
 
 	err = skb_cow_head(skb, min_headroom);
 	err = skb_cow_head(skb, min_headroom);
-	if (unlikely(err))
+	if (unlikely(err)) {
+		kfree_skb(skb);
 		return err;
 		return err;
+	}
 
 
 	skb = vlan_hwaccel_push_inside(skb);
 	skb = vlan_hwaccel_push_inside(skb);
 	if (unlikely(!skb))
 	if (unlikely(!skb))

+ 3 - 0
net/openvswitch/vport-geneve.c

@@ -219,7 +219,10 @@ static int geneve_tnl_send(struct vport *vport, struct sk_buff *skb)
 			      false);
 			      false);
 	if (err < 0)
 	if (err < 0)
 		ip_rt_put(rt);
 		ip_rt_put(rt);
+	return err;
+
 error:
 error:
+	kfree_skb(skb);
 	return err;
 	return err;
 }
 }
 
 

+ 11 - 7
net/openvswitch/vport-gre.c

@@ -73,7 +73,7 @@ static struct sk_buff *__build_header(struct sk_buff *skb,
 
 
 	skb = gre_handle_offloads(skb, !!(tun_key->tun_flags & TUNNEL_CSUM));
 	skb = gre_handle_offloads(skb, !!(tun_key->tun_flags & TUNNEL_CSUM));
 	if (IS_ERR(skb))
 	if (IS_ERR(skb))
-		return NULL;
+		return skb;
 
 
 	tpi.flags = filter_tnl_flags(tun_key->tun_flags);
 	tpi.flags = filter_tnl_flags(tun_key->tun_flags);
 	tpi.proto = htons(ETH_P_TEB);
 	tpi.proto = htons(ETH_P_TEB);
@@ -144,7 +144,7 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 
 
 	if (unlikely(!OVS_CB(skb)->egress_tun_info)) {
 	if (unlikely(!OVS_CB(skb)->egress_tun_info)) {
 		err = -EINVAL;
 		err = -EINVAL;
-		goto error;
+		goto err_free_skb;
 	}
 	}
 
 
 	tun_key = &OVS_CB(skb)->egress_tun_info->tunnel;
 	tun_key = &OVS_CB(skb)->egress_tun_info->tunnel;
@@ -157,8 +157,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 	fl.flowi4_proto = IPPROTO_GRE;
 	fl.flowi4_proto = IPPROTO_GRE;
 
 
 	rt = ip_route_output_key(net, &fl);
 	rt = ip_route_output_key(net, &fl);
-	if (IS_ERR(rt))
-		return PTR_ERR(rt);
+	if (IS_ERR(rt)) {
+		err = PTR_ERR(rt);
+		goto err_free_skb;
+	}
 
 
 	tunnel_hlen = ip_gre_calc_hlen(tun_key->tun_flags);
 	tunnel_hlen = ip_gre_calc_hlen(tun_key->tun_flags);
 
 
@@ -183,8 +185,9 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 
 
 	/* Push Tunnel header. */
 	/* Push Tunnel header. */
 	skb = __build_header(skb, tunnel_hlen);
 	skb = __build_header(skb, tunnel_hlen);
-	if (unlikely(!skb)) {
-		err = 0;
+	if (IS_ERR(skb)) {
+		err = PTR_ERR(rt);
+		skb = NULL;
 		goto err_free_rt;
 		goto err_free_rt;
 	}
 	}
 
 
@@ -198,7 +201,8 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 			     tun_key->ipv4_tos, tun_key->ipv4_ttl, df, false);
 			     tun_key->ipv4_tos, tun_key->ipv4_ttl, df, false);
 err_free_rt:
 err_free_rt:
 	ip_rt_put(rt);
 	ip_rt_put(rt);
-error:
+err_free_skb:
+	kfree_skb(skb);
 	return err;
 	return err;
 }
 }
 
 

+ 2 - 0
net/openvswitch/vport-vxlan.c

@@ -187,7 +187,9 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
 			     false);
 			     false);
 	if (err < 0)
 	if (err < 0)
 		ip_rt_put(rt);
 		ip_rt_put(rt);
+	return err;
 error:
 error:
+	kfree_skb(skb);
 	return err;
 	return err;
 }
 }
 
 

+ 2 - 3
net/openvswitch/vport.c

@@ -519,10 +519,9 @@ int ovs_vport_send(struct vport *vport, struct sk_buff *skb)
 		u64_stats_update_end(&stats->syncp);
 		u64_stats_update_end(&stats->syncp);
 	} else if (sent < 0) {
 	} else if (sent < 0) {
 		ovs_vport_record_error(vport, VPORT_E_TX_ERROR);
 		ovs_vport_record_error(vport, VPORT_E_TX_ERROR);
-		kfree_skb(skb);
-	} else
+	} else {
 		ovs_vport_record_error(vport, VPORT_E_TX_DROPPED);
 		ovs_vport_record_error(vport, VPORT_E_TX_DROPPED);
-
+	}
 	return sent;
 	return sent;
 }
 }