From f4bece52d237105d7645d8cf63af5ca92e632c9b Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Fri, 21 Jun 2013 16:07:08 -0700 Subject: [PATCH] datapath: Fix a kernel crash caused by corrupted mask list. 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 ------- 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. v2 -> v3: Make sure the V1 flow table allocation always allocate a mask list. This change makes most of the make_list NULL check Unnecessary. Signed-off-by: Andy Zhou --- datapath/flow.c | 33 +++++++++++++++++++++++++++------ datapath/flow.h | 2 +- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/datapath/flow.c b/datapath/flow.c index 3a7bc9b..9730cb9 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -443,7 +443,7 @@ static void free_buckets(struct flex_array *buckets) flex_array_free(buckets); } -struct flow_table *ovs_flow_tbl_alloc(int new_size) +static struct flow_table *__flow_tbl_alloc(int new_size) { struct flow_table *table = kmalloc(sizeof(*table), GFP_KERNEL); @@ -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,11 +485,32 @@ static void __flow_tbl_destroy(struct flow_table *table) } } + BUG_ON(!list_empty(table->mask_list)); + kfree(table->mask_list); + skip_flows: free_buckets(table->buckets); kfree(table); } +struct flow_table *ovs_flow_tbl_alloc(int new_size) +{ + struct flow_table *table = __flow_tbl_alloc(new_size); + + if (!table) + return NULL; + + table->mask_list = kmalloc(sizeof(struct list_head), GFP_KERNEL); + if (!table->mask_list) { + table->keep_flows = true; + __flow_tbl_destroy(table); + return NULL; + } + INIT_LIST_HEAD(table->mask_list); + + return table; +} + static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu) { struct flow_table *table = container_of(rcu, struct flow_table, rcu); @@ -571,7 +592,7 @@ static struct flow_table *__flow_tbl_rehash(struct flow_table *table, int n_buck { struct flow_table *new_table; - new_table = ovs_flow_tbl_alloc(n_buckets); + new_table = __flow_tbl_alloc(n_buckets); if (!new_table) return ERR_PTR(-ENOMEM); @@ -1030,7 +1051,7 @@ struct sw_flow *ovs_flow_lookup(struct flow_table *tbl, struct sw_flow *flow = NULL; struct sw_flow_mask *mask; - list_for_each_entry_rcu(mask, &tbl->mask_list, list) { + list_for_each_entry_rcu(mask, tbl->mask_list, list) { flow = ovs_masked_flow_lookup(tbl, key, mask); if (flow) /* Found */ break; @@ -1843,7 +1864,7 @@ 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) { + 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 +1881,7 @@ 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); + list_add_rcu(&mask->list, tbl->mask_list); } /** diff --git a/datapath/flow.h b/datapath/flow.h index 2b6f217..2366039 100644 --- a/datapath/flow.h +++ b/datapath/flow.h @@ -194,7 +194,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