Skip to content

Commit

Permalink
openvswitch: Fix vport_send double free
Browse files Browse the repository at this point in the history
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: 91b7514 ("openvswitch: Unify vport error stats
handling")
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Pravin B Shelar authored and David S. Miller committed Dec 24, 2014
1 parent cbe7e76 commit 997e068
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 11 deletions.
6 changes: 5 additions & 1 deletion net/ipv4/geneve.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
3 changes: 3 additions & 0 deletions net/openvswitch/vport-geneve.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
18 changes: 11 additions & 7 deletions net/openvswitch/vport-gre.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions net/openvswitch/vport-vxlan.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 2 additions & 3 deletions net/openvswitch/vport.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 997e068

Please sign in to comment.