[ovs-dev] [v2] datapath: Fix recirc bug where skb is double freed.

[Andy Zhou]

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 <jpet...@nicira.com>
Signed-off-by: Andy Zhou <az...@nicira.com>
---
 datapath/actions.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index ad22467..3b39d6f 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
@@ -822,15 +831,6 @@ static int execute_recirc(struct datapath *dp, struct 
sk_buff *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);
 
@@ -897,6 +897,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:
-- 
1.9.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev