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>

diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index 95e47c97585e2e34635976d6a352a1484d5c091c..394a200f93c1f5b5338ccab70286545acad1ebda 100644 (file)
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -122,14 +122,18 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
        int err;
 
        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
                        + GENEVE_BASE_HLEN + opt_len + sizeof(struct iphdr)
                        + (vlan_tx_tag_present(skb) ? VLAN_HLEN : 0);
 
        err = skb_cow_head(skb, min_headroom);
-       if (unlikely(err))
+       if (unlikely(err)) {
+               kfree_skb(skb);
                return err;
+       }
 
        skb = vlan_hwaccel_push_inside(skb);
        if (unlikely(!skb))

diff --git a/net/openvswitch/vport-geneve.c b/net/openvswitch/vport-geneve.c
index 347fa2325b226309b8e3cfb239f2e1d0a6631a50..484864dd0e689290dfca8a2c204e984de1b33184 100644 (file)
--- a/net/openvswitch/vport-geneve.c
+++ b/net/openvswitch/vport-geneve.c
@@ -219,7 +219,10 @@ static int geneve_tnl_send(struct vport *vport, struct sk_buff *skb)
                              false);
        if (err < 0)
                ip_rt_put(rt);
+       return err;
+
 error:
+       kfree_skb(skb);
        return err;
 }
 

diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 6b69df545b1da3dba7ffedb92e106855106fd8d6..28f54e9a6b80618f1f28312d58eb58e222ea4426 100644 (file)
--- a/net/openvswitch/vport-gre.c
+++ b/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));
        if (IS_ERR(skb))
-               return NULL;
+               return skb;
 
        tpi.flags = filter_tnl_flags(tun_key->tun_flags);
        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)) {
                err = -EINVAL;
-               goto error;
+               goto err_free_skb;
        }
 
        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;
 
        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);
 
@@ -183,8 +185,9 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 
        /* Push Tunnel header. */
        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;
        }
 
@@ -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);
 err_free_rt:
        ip_rt_put(rt);
-error:
+err_free_skb:
+       kfree_skb(skb);
        return err;
 }
 

diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
index 38f95a52241bd53af8de06b76079e403de8413cf..d7c46b301024906cf748453d24b91d2790f2e272 100644 (file)
--- a/net/openvswitch/vport-vxlan.c
+++ b/net/openvswitch/vport-vxlan.c
@@ -187,7 +187,9 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
                             false);
        if (err < 0)
                ip_rt_put(rt);
+       return err;
 error:
+       kfree_skb(skb);
        return err;
 }
 

diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 9584526c077804f21d22726d4bb5bd7cc4260d36..53f3ebbfceabcb3d0b90cb1508c295c51da49eeb 100644 (file)
--- a/net/openvswitch/vport.c
+++ b/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);
        } else if (sent < 0) {
                ovs_vport_record_error(vport, VPORT_E_TX_ERROR);
-               kfree_skb(skb);
-       } else
+       } else {
                ovs_vport_record_error(vport, VPORT_E_TX_DROPPED);
-
+       }
        return sent;
 }