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 7776ae5a8f15..c1a2bf2aa2de 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.2.3-59-g8ed1b 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 c1a2bf2aa2de..c7cb3cc083df 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.2.3-59-g8ed1b 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.c | 43 -------------------------------- net/mac80211/mesh.h | 4 --- net/mac80211/mesh_pathtbl.c | 60 +++++++++++++++++++++++++++++++++++++++------ 3 files changed, 53 insertions(+), 54 deletions(-) (limited to 'net/mac80211/mesh_pathtbl.c') diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c index 75378e852f00..29e9980c8e60 100644 --- a/net/mac80211/mesh.c +++ b/net/mac80211/mesh.c @@ -287,49 +287,6 @@ void mesh_mgmt_ies_add(struct sk_buff *skb, struct ieee80211_sub_if_data *sdata) } } -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; -} - -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 ieee80211_mesh_path_timer(unsigned long data) { diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h index 10acf1cc8082..5c0c20389a1a 100644 --- a/net/mac80211/mesh.h +++ b/net/mac80211/mesh.h @@ -240,12 +240,8 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, /* Private interfaces */ /* Mesh tables */ -struct mesh_table *mesh_table_alloc(int size_order); -void mesh_table_free(struct mesh_table *tbl, bool free_leafs); void mesh_mpath_table_grow(void); void mesh_mpp_table_grow(void); -u32 mesh_table_hash(u8 *addr, struct ieee80211_sub_if_data *sdata, - struct mesh_table *tbl); /* Mesh paths */ int mesh_path_error_tx(u8 ttl, u8 *target, __le32 target_sn, __le16 target_rcode, const u8 *ra, struct ieee80211_sub_if_data *sdata); diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c index c7cb3cc083df..0aa96cd6bd27 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.2.3-59-g8ed1b