[ovs-discuss] one patch was omitted to be pushed to branch-2.3---datapath: Fix recirc bug where skb is double freed

zhangzhiming zhangzhiming at yunshan.net.cn
Sun May 17 11:29:27 UTC 2015


Hi,

I found one patch was omitted to be pushed to branch-2.3, which leads to double freed skb.
Could someone to confirm the patch and submit it to branch-2.3? 
Thanks!

Here is the patch information:


commit 867e37ba00091b3e319c4c47c1598f1ae84dd32e
Author: Andy Zhou <azhou at nicira.com>
Date:   Mon Aug 25 15:18:19 2014 -0700

    datapath: Fix recirc bug where skb is double freed.
    
    If recirc action is the last action of a action list, the SKB triggers
    the recirc will be freed twice. This patch fixes this bug.
    
    Reported-by: Justin Pettit <jpettit at nicira.com>
    Signed-off-by: Andy Zhou <azhou at nicira.com>

diff --git a/datapath/actions.c b/datapath/actions.c
index ad22467..7f25553 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -809,7 +809,16 @@ static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
                          const struct nlattr *a, int rem)
 {
        struct sw_flow_key recirc_key;
-       int err;
+
+       if (!is_skb_flow_key_valid(skb)) {
+               int err;
+
+               err = ovs_flow_key_update(skb, OVS_CB(skb)->pkt_key);
+               if (err)
+                       return err;
+
+       }
+       BUG_ON(!is_skb_flow_key_valid(skb));

        if (!last_action(a, rem)) {
                /* Recirc action is the not the last action
@@ -820,19 +829,9 @@ static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
                 * continue on with the rest of the action list. */
                if (!skb)
                        return 0;
-       }

-       if (!is_skb_flow_key_valid(skb)) {
-               err = ovs_flow_key_update(skb, OVS_CB(skb)->pkt_key);
-               if (err) {
-                       kfree_skb(skb);
-                       return err;
-               }
-       }
-       BUG_ON(!is_skb_flow_key_valid(skb));
-
-       if (!last_action(a, rem))
                flow_key_clone(skb, &recirc_key);
+       }

        flow_key_set_recirc_id(skb, nla_get_u32(a));
        ovs_dp_process_packet(skb, true);
@@ -897,6 +896,12 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,

                case OVS_ACTION_ATTR_RECIRC:
                        err = execute_recirc(dp, skb, a, rem);
+                       if (last_action(a, rem)) {
+                               /* If this is the last action, the skb has
+                                * been consumed or freed.
+                                * Return immediately. */
+                               return err;
+                       }
                        break;

                case OVS_ACTION_ATTR_SET:




Jeremy Zhang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://openvswitch.org/pipermail/ovs-discuss/attachments/20150517/43d54250/attachment-0002.html>


More information about the discuss mailing list