From 9ca99eeca0cfe839c481f3350988e9ed94188567 Mon Sep 17 00:00:00 2001 From: Javier Cardona Date: Tue, 3 May 2011 16:57:15 -0700 Subject: mac80211: Check size of a new mesh path table for changes since allocation. Not sure if I'm chasing a ghost here, seems like the mesh_path->size_order needs to be inside an RCU-read section to prevent that value from changing between table allocation and copying. We have observed crashes that might be caused by this. Signed-off-by: Javier Cardona Signed-off-by: John W. Linville --- net/mac80211/mesh_pathtbl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'net/mac80211/mesh_pathtbl.c') diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c index 7776ae5..c1a2bf2 100644 --- a/net/mac80211/mesh_pathtbl.c +++ b/net/mac80211/mesh_pathtbl.c @@ -76,7 +76,6 @@ static int mesh_table_grow(struct mesh_table *oldtbl, < oldtbl->mean_chain_len * (oldtbl->hash_mask + 1)) return -EAGAIN; - newtbl->free_node = oldtbl->free_node; newtbl->mean_chain_len = oldtbl->mean_chain_len; newtbl->copy_node = oldtbl->copy_node; @@ -329,7 +328,8 @@ void mesh_mpath_table_grow(void) { struct mesh_table *oldtbl, *newtbl; - newtbl = mesh_table_alloc(mesh_paths->size_order + 1); + rcu_read_lock(); + newtbl = mesh_table_alloc(rcu_dereference(mesh_paths)->size_order + 1); if (!newtbl) return; write_lock(&pathtbl_resize_lock); @@ -339,6 +339,7 @@ void mesh_mpath_table_grow(void) write_unlock(&pathtbl_resize_lock); return; } + rcu_read_unlock(); rcu_assign_pointer(mesh_paths, newtbl); write_unlock(&pathtbl_resize_lock); @@ -350,7 +351,8 @@ void mesh_mpp_table_grow(void) { struct mesh_table *oldtbl, *newtbl; - newtbl = mesh_table_alloc(mpp_paths->size_order + 1); + rcu_read_lock(); + newtbl = mesh_table_alloc(rcu_dereference(mpp_paths)->size_order + 1); if (!newtbl) return; write_lock(&pathtbl_resize_lock); @@ -360,6 +362,7 @@ void mesh_mpp_table_grow(void) write_unlock(&pathtbl_resize_lock); return; } + rcu_read_unlock(); rcu_assign_pointer(mpp_paths, newtbl); write_unlock(&pathtbl_resize_lock); -- cgit v1.1 From 9b84b80891e5e25ae21c855bb135b05274125a29 Mon Sep 17 00:00:00 2001 From: Javier Cardona Date: Tue, 3 May 2011 16:57:16 -0700 Subject: mac80211: Fix locking bug on mesh path table access The mesh and mpp path tables are accessed from softirq and workqueue context so non-irq locking cannot be used. Or at least that's what PROVE_RCU seems to tell us here: [ 431.240946] ================================= [ 431.241061] [ INFO: inconsistent lock state ] [ 431.241061] 2.6.39-rc3-wl+ #354 [ 431.241061] --------------------------------- [ 431.241061] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage. [ 431.241061] kworker/u:1/1423 [HC0[0]:SC0[0]:HE1:SE1] takes: [ 431.241061] (&(&newtbl->hashwlock[i])->rlock){+.?...}, at: [] mesh_path_add+0x167/0x257 Signed-off-by: Javier Cardona Signed-off-by: John W. Linville --- net/mac80211/mesh_pathtbl.c | 54 +++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 26 deletions(-) (limited to 'net/mac80211/mesh_pathtbl.c') diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c index c1a2bf2..c7cb3cc 100644 --- a/net/mac80211/mesh_pathtbl.c +++ b/net/mac80211/mesh_pathtbl.c @@ -55,12 +55,12 @@ void mesh_table_free(struct mesh_table *tbl, bool free_leafs) mesh_hash = tbl->hash_buckets; for (i = 0; i <= tbl->hash_mask; i++) { - spin_lock(&tbl->hashwlock[i]); + spin_lock_bh(&tbl->hashwlock[i]); hlist_for_each_safe(p, q, &mesh_hash[i]) { tbl->free_node(p, free_leafs); atomic_dec(&tbl->entries); } - spin_unlock(&tbl->hashwlock[i]); + spin_unlock_bh(&tbl->hashwlock[i]); } __mesh_table_free(tbl); } @@ -274,7 +274,7 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata) if (!new_node) goto err_node_alloc; - read_lock(&pathtbl_resize_lock); + read_lock_bh(&pathtbl_resize_lock); memcpy(new_mpath->dst, dst, ETH_ALEN); new_mpath->sdata = sdata; new_mpath->flags = 0; @@ -289,7 +289,7 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata) hash_idx = mesh_table_hash(dst, sdata, mesh_paths); bucket = &mesh_paths->hash_buckets[hash_idx]; - spin_lock(&mesh_paths->hashwlock[hash_idx]); + spin_lock_bh(&mesh_paths->hashwlock[hash_idx]); err = -EEXIST; hlist_for_each_entry(node, n, bucket, list) { @@ -305,8 +305,8 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata) mesh_paths_generation++; - spin_unlock(&mesh_paths->hashwlock[hash_idx]); - read_unlock(&pathtbl_resize_lock); + spin_unlock_bh(&mesh_paths->hashwlock[hash_idx]); + read_unlock_bh(&pathtbl_resize_lock); if (grow) { set_bit(MESH_WORK_GROW_MPATH_TABLE, &ifmsh->wrkq_flags); ieee80211_queue_work(&local->hw, &sdata->work); @@ -314,8 +314,8 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data *sdata) return 0; err_exists: - spin_unlock(&mesh_paths->hashwlock[hash_idx]); - read_unlock(&pathtbl_resize_lock); + spin_unlock_bh(&mesh_paths->hashwlock[hash_idx]); + read_unlock_bh(&pathtbl_resize_lock); kfree(new_node); err_node_alloc: kfree(new_mpath); @@ -332,16 +332,17 @@ void mesh_mpath_table_grow(void) newtbl = mesh_table_alloc(rcu_dereference(mesh_paths)->size_order + 1); if (!newtbl) return; - write_lock(&pathtbl_resize_lock); + write_lock_bh(&pathtbl_resize_lock); oldtbl = mesh_paths; if (mesh_table_grow(mesh_paths, newtbl) < 0) { + rcu_read_unlock(); __mesh_table_free(newtbl); - write_unlock(&pathtbl_resize_lock); + write_unlock_bh(&pathtbl_resize_lock); return; } rcu_read_unlock(); rcu_assign_pointer(mesh_paths, newtbl); - write_unlock(&pathtbl_resize_lock); + write_unlock_bh(&pathtbl_resize_lock); synchronize_rcu(); mesh_table_free(oldtbl, false); @@ -355,16 +356,17 @@ void mesh_mpp_table_grow(void) newtbl = mesh_table_alloc(rcu_dereference(mpp_paths)->size_order + 1); if (!newtbl) return; - write_lock(&pathtbl_resize_lock); + write_lock_bh(&pathtbl_resize_lock); oldtbl = mpp_paths; if (mesh_table_grow(mpp_paths, newtbl) < 0) { + rcu_read_unlock(); __mesh_table_free(newtbl); - write_unlock(&pathtbl_resize_lock); + write_unlock_bh(&pathtbl_resize_lock); return; } rcu_read_unlock(); rcu_assign_pointer(mpp_paths, newtbl); - write_unlock(&pathtbl_resize_lock); + write_unlock_bh(&pathtbl_resize_lock); synchronize_rcu(); mesh_table_free(oldtbl, false); @@ -398,7 +400,7 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata) if (!new_node) goto err_node_alloc; - read_lock(&pathtbl_resize_lock); + read_lock_bh(&pathtbl_resize_lock); memcpy(new_mpath->dst, dst, ETH_ALEN); memcpy(new_mpath->mpp, mpp, ETH_ALEN); new_mpath->sdata = sdata; @@ -411,7 +413,7 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata) hash_idx = mesh_table_hash(dst, sdata, mpp_paths); bucket = &mpp_paths->hash_buckets[hash_idx]; - spin_lock(&mpp_paths->hashwlock[hash_idx]); + spin_lock_bh(&mpp_paths->hashwlock[hash_idx]); err = -EEXIST; hlist_for_each_entry(node, n, bucket, list) { @@ -425,8 +427,8 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata) mpp_paths->mean_chain_len * (mpp_paths->hash_mask + 1)) grow = 1; - spin_unlock(&mpp_paths->hashwlock[hash_idx]); - read_unlock(&pathtbl_resize_lock); + spin_unlock_bh(&mpp_paths->hashwlock[hash_idx]); + read_unlock_bh(&pathtbl_resize_lock); if (grow) { set_bit(MESH_WORK_GROW_MPP_TABLE, &ifmsh->wrkq_flags); ieee80211_queue_work(&local->hw, &sdata->work); @@ -434,8 +436,8 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct ieee80211_sub_if_data *sdata) return 0; err_exists: - spin_unlock(&mpp_paths->hashwlock[hash_idx]); - read_unlock(&pathtbl_resize_lock); + spin_unlock_bh(&mpp_paths->hashwlock[hash_idx]); + read_unlock_bh(&pathtbl_resize_lock); kfree(new_node); err_node_alloc: kfree(new_mpath); @@ -548,11 +550,11 @@ int mesh_path_del(u8 *addr, struct ieee80211_sub_if_data *sdata) int hash_idx; int err = 0; - read_lock(&pathtbl_resize_lock); + read_lock_bh(&pathtbl_resize_lock); hash_idx = mesh_table_hash(addr, sdata, mesh_paths); bucket = &mesh_paths->hash_buckets[hash_idx]; - spin_lock(&mesh_paths->hashwlock[hash_idx]); + spin_lock_bh(&mesh_paths->hashwlock[hash_idx]); hlist_for_each_entry(node, n, bucket, list) { mpath = node->mpath; if (mpath->sdata == sdata && @@ -570,8 +572,8 @@ int mesh_path_del(u8 *addr, struct ieee80211_sub_if_data *sdata) err = -ENXIO; enddel: mesh_paths_generation++; - spin_unlock(&mesh_paths->hashwlock[hash_idx]); - read_unlock(&pathtbl_resize_lock); + spin_unlock_bh(&mesh_paths->hashwlock[hash_idx]); + read_unlock_bh(&pathtbl_resize_lock); return err; } @@ -723,7 +725,7 @@ void mesh_path_expire(struct ieee80211_sub_if_data *sdata) struct hlist_node *p; int i; - read_lock(&pathtbl_resize_lock); + read_lock_bh(&pathtbl_resize_lock); for_each_mesh_entry(mesh_paths, p, node, i) { if (node->mpath->sdata != sdata) continue; @@ -738,7 +740,7 @@ void mesh_path_expire(struct ieee80211_sub_if_data *sdata) } else spin_unlock_bh(&mpath->state_lock); } - read_unlock(&pathtbl_resize_lock); + read_unlock_bh(&pathtbl_resize_lock); } void mesh_pathtbl_unregister(void) -- cgit v1.1 From 6b86bd62a505a4a9739474f00f8088395b7a80ba Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 12 May 2011 13:38:50 +0200 Subject: mac80211: mesh: move some code to make it static There's no need to have table functions in one file and all users in another, move the functions to the right file and make them static. Also move a static variable to the beginning of the file to make it easier to find. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/mesh_pathtbl.c | 60 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 7 deletions(-) (limited to 'net/mac80211/mesh_pathtbl.c') diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c index c7cb3cc..0aa96cd 100644 --- a/net/mac80211/mesh_pathtbl.c +++ b/net/mac80211/mesh_pathtbl.c @@ -40,6 +40,50 @@ static struct mesh_table *mesh_paths; static struct mesh_table *mpp_paths; /* Store paths for MPP&MAP */ int mesh_paths_generation; + +/* This lock will have the grow table function as writer and add / delete nodes + * as readers. When reading the table (i.e. doing lookups) we are well protected + * by RCU + */ +static DEFINE_RWLOCK(pathtbl_resize_lock); + + +static struct mesh_table *mesh_table_alloc(int size_order) +{ + int i; + struct mesh_table *newtbl; + + newtbl = kmalloc(sizeof(struct mesh_table), GFP_KERNEL); + if (!newtbl) + return NULL; + + newtbl->hash_buckets = kzalloc(sizeof(struct hlist_head) * + (1 << size_order), GFP_KERNEL); + + if (!newtbl->hash_buckets) { + kfree(newtbl); + return NULL; + } + + newtbl->hashwlock = kmalloc(sizeof(spinlock_t) * + (1 << size_order), GFP_KERNEL); + if (!newtbl->hashwlock) { + kfree(newtbl->hash_buckets); + kfree(newtbl); + return NULL; + } + + newtbl->size_order = size_order; + newtbl->hash_mask = (1 << size_order) - 1; + atomic_set(&newtbl->entries, 0); + get_random_bytes(&newtbl->hash_rnd, + sizeof(newtbl->hash_rnd)); + for (i = 0; i <= newtbl->hash_mask; i++) + spin_lock_init(&newtbl->hashwlock[i]); + + return newtbl; +} + static void __mesh_table_free(struct mesh_table *tbl) { kfree(tbl->hash_buckets); @@ -47,7 +91,7 @@ static void __mesh_table_free(struct mesh_table *tbl) kfree(tbl); } -void mesh_table_free(struct mesh_table *tbl, bool free_leafs) +static void mesh_table_free(struct mesh_table *tbl, bool free_leafs) { struct hlist_head *mesh_hash; struct hlist_node *p, *q; @@ -66,7 +110,7 @@ void mesh_table_free(struct mesh_table *tbl, bool free_leafs) } static int mesh_table_grow(struct mesh_table *oldtbl, - struct mesh_table *newtbl) + struct mesh_table *newtbl) { struct hlist_head *oldhash; struct hlist_node *p, *q; @@ -97,12 +141,14 @@ errcopy: return -ENOMEM; } +static u32 mesh_table_hash(u8 *addr, struct ieee80211_sub_if_data *sdata, + struct mesh_table *tbl) +{ + /* Use last four bytes of hw addr and interface index as hash index */ + return jhash_2words(*(u32 *)(addr+2), sdata->dev->ifindex, tbl->hash_rnd) + & tbl->hash_mask; +} -/* This lock will have the grow table function as writer and add / delete nodes - * as readers. When reading the table (i.e. doing lookups) we are well protected - * by RCU - */ -static DEFINE_RWLOCK(pathtbl_resize_lock); /** * -- cgit v1.1