[ovs-dev] [megaflow masklist fix V2] datapath: Fix a kernel crash caused by corrupted mask list.

Andy Zhou azhou at nicira.com
Fri Jun 21 23:18:42 UTC 2013


When flow table is copied, the mask list from the old table
is not properly copied into the new table. The corrupted mask
list in the new table will lead to kernel crash. This patch
fixes this bug.

Bug #18110
Reported-by: Justin Pettit <jpettit at nicira.com>

-------
v1 -> v2:
	Jesse pointed out the race condition in v1. Fastpath can be
        walking the list while the head is being replaced.

	In V2, instead of have mask_list head as part of the flow
	table data structure, it only has a pointer to the head. The
        actual head is allocated at run time, and stay consistent with
	the mask list.

Signed-off-by: Andy Zhou <azhou at nicira.com>
---
 datapath/flow.c |   36 ++++++++++++++++++++++++++++--------
 datapath/flow.h |    2 +-
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index 38b9502..864c31c 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -461,7 +461,7 @@ struct flow_table *ovs_flow_tbl_alloc(int new_size)
 	table->node_ver = 0;
 	table->keep_flows = false;
 	get_random_bytes(&table->hash_seed, sizeof(u32));
-	INIT_LIST_HEAD(&table->mask_list);
+	table->mask_list = NULL;
 
 	return table;
 }
@@ -485,6 +485,11 @@ static void __flow_tbl_destroy(struct flow_table *table)
 		}
 	}
 
+	if (table->mask_list) {
+		BUG_ON(!list_empty(table->mask_list));
+		kfree(table->mask_list);
+	}
+
 skip_flows:
 	free_buckets(table->buckets);
 	kfree(table);
@@ -1028,12 +1033,15 @@ struct sw_flow *ovs_flow_lookup(struct flow_table *tbl,
 				const struct sw_flow_key *key)
 {
 	struct sw_flow *flow = NULL;
-	struct sw_flow_mask *mask;
 
-	list_for_each_entry_rcu(mask, &tbl->mask_list, list) {
-		flow = ovs_masked_flow_lookup(tbl, key, mask);
-		if (flow)  /* Found */
-			break;
+	if (tbl->mask_list) {
+		struct sw_flow_mask *mask;
+
+		list_for_each_entry_rcu(mask, tbl->mask_list, list) {
+			flow = ovs_masked_flow_lookup(tbl, key, mask);
+			if (flow)  /* Found */
+				break;
+		}
 	}
 
 	return flow;
@@ -1843,7 +1851,10 @@ struct sw_flow_mask *ovs_sw_flow_mask_find(const struct flow_table *tbl,
 {
 	struct list_head *ml;
 
-	list_for_each(ml, &tbl->mask_list) {
+	if (!tbl->mask_list)
+		return NULL;
+
+	list_for_each(ml, tbl->mask_list) {
 		struct sw_flow_mask *m;
 		m = container_of(ml, struct sw_flow_mask, list);
 		if (ovs_sw_flow_mask_equal(mask, m))
@@ -1860,7 +1871,16 @@ struct sw_flow_mask *ovs_sw_flow_mask_find(const struct flow_table *tbl,
  */
 void ovs_sw_flow_mask_insert(struct flow_table *tbl, struct sw_flow_mask *mask)
 {
-	list_add_rcu(&mask->list, &tbl->mask_list);
+	if (!tbl->mask_list) {
+		tbl->mask_list = (struct list_head *)
+			kmalloc(sizeof(struct list_head), GFP_KERNEL);
+
+		if (tbl->mask_list)
+			INIT_LIST_HEAD(tbl->mask_list);
+	}
+
+	if (tbl->mask_list)
+		list_add_rcu(&mask->list, tbl->mask_list);
 }
 
 /**
diff --git a/datapath/flow.h b/datapath/flow.h
index 45eba03..a31dab0 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -191,7 +191,7 @@ struct flow_table {
 	struct flex_array *buckets;
 	unsigned int count, n_buckets;
 	struct rcu_head rcu;
-	struct list_head mask_list;
+	struct list_head *mask_list;
 	int node_ver;
 	u32 hash_seed;
 	bool keep_flows;
-- 
1.7.9.5




More information about the dev mailing list