[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 <jpettit at nicira.com>
Signed-off-by: Andy Zhou <azhou at 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