Hi David,
we have 3 more bugfixes for you which we would like to see getting merged into net-next/linux-3.2. They fix refcounting, a crash on module unload and a protocol handling bug.
Thanks, Marek
The following changes since commit 9d8523931f7f5eb8900077f0da0fbe6b8ad0010b:
batman-adv: correctly set the data field in the TT_REPONSE packet (2011-10-18 22:45:10 +0200)
are available in the git repository at: git://git.open-mesh.org/linux-merge.git batman-adv/maint
Antonio Quartulli (1): batman-adv: unify hash_entry field position in tt_local/global_entry
Simon Wunderlich (2): batman-adv: remove references for global tt entries batman-adv: add sanity check when removing global tts
net/batman-adv/translation-table.c | 17 ++++++++++++++++- net/batman-adv/types.h | 4 ++-- 2 files changed, 18 insertions(+), 3 deletions(-)
From: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de
struct tt_global_entry holds a reference to an orig_node which must be decremented before deallocating the structure.
Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Tested-by: Alexey Fisher bug-track@fisher-privat.net Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- net/batman-adv/translation-table.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 873fb3d..abf05cb 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -137,10 +137,22 @@ static void tt_local_entry_free_ref(struct tt_local_entry *tt_local_entry) kfree_rcu(tt_local_entry, rcu); }
+static void tt_global_entry_free_rcu(struct rcu_head *rcu) +{ + struct tt_global_entry *tt_global_entry; + + tt_global_entry = container_of(rcu, struct tt_global_entry, rcu); + + if (tt_global_entry->orig_node) + orig_node_free_ref(tt_global_entry->orig_node); + + kfree(tt_global_entry); +} + static void tt_global_entry_free_ref(struct tt_global_entry *tt_global_entry) { if (atomic_dec_and_test(&tt_global_entry->refcount)) - kfree_rcu(tt_global_entry, rcu); + call_rcu(&tt_global_entry->rcu, tt_global_entry_free_rcu); }
static void tt_local_event(struct bat_priv *bat_priv, const uint8_t *addr,
From: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de
After removing the batman-adv module, the hash may be already gone when tt_global_del_orig() tries to clean the hash. This patch adds a sanity check to avoid this.
Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Tested-by: Alexey Fisher bug-track@fisher-privat.net Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- net/batman-adv/translation-table.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index abf05cb..c7aafc7 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -722,6 +722,9 @@ void tt_global_del_orig(struct bat_priv *bat_priv, struct hlist_head *head; spinlock_t *list_lock; /* protects write access to the hash lists */
+ if (!hash) + return; + for (i = 0; i < hash->size; i++) { head = &hash->table[i]; list_lock = &hash->list_locks[i];
From: Antonio Quartulli ordex@autistici.org
Function tt_response_fill_table() actually uses a tt_local_entry pointer to iterate either over the local or the global table entries (it depends on the what hash table is passed as argument). To iterate over such entries the hlist_for_each_entry_rcu() macro has to access their "hash_entry" field which MUST be at the same position in both the tt_global/local_entry structures.
Reported-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- net/batman-adv/types.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 1ae3557..ab8d0fe 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -224,22 +224,22 @@ struct socket_packet {
struct tt_local_entry { uint8_t addr[ETH_ALEN]; + struct hlist_node hash_entry; unsigned long last_seen; uint16_t flags; atomic_t refcount; struct rcu_head rcu; - struct hlist_node hash_entry; };
struct tt_global_entry { uint8_t addr[ETH_ALEN]; + struct hlist_node hash_entry; /* entry in the global table */ struct orig_node *orig_node; uint8_t ttvn; uint16_t flags; /* only TT_GLOBAL_ROAM is used */ unsigned long roam_at; /* time at which TT_GLOBAL_ROAM was set */ atomic_t refcount; struct rcu_head rcu; - struct hlist_node hash_entry; /* entry in the global table */ };
struct tt_change_node {
From: Marek Lindner lindner_marek@yahoo.de Date: Sat, 29 Oct 2011 10:06:43 +0200
git://git.open-mesh.org/linux-merge.git batman-adv/maint
Pulled, but long term you should shore up your datastructures to handle that issue in patch #3.
Make a common header:
struct tt_entry_common { u8 addr[ETH_ALEN]; struct hlist_node hash_entry; };
Then use that at the beginning of both structures:
struct tt_local_entry { struct tt_entry_common common; unsigned long last_seen; ... };
struct tt_global_entry { struct tt_entry_comomn common; struct orig_node *orig_node; ... };
And &p->common is what gets passed into tt_response_fill_table().
On Sunday 30 October 2011 03:07:45 David Miller wrote: [...]
Make a common header:
struct tt_entry_common { u8 addr[ETH_ALEN]; struct hlist_node hash_entry; };
Then use that at the beginning of both structures:
struct tt_local_entry { struct tt_entry_common common; unsigned long last_seen; ... };
struct tt_global_entry { struct tt_entry_comomn common; struct orig_node *orig_node; ... };
And &p->common is what gets passed into tt_response_fill_table().
Thanks for the pull. This is exactly the long term solution we want to submit later to net-next. But we also wanted to keep the patch as small as possible for stable@kernel.org
Thanks, Sven
b.a.t.m.a.n@lists.open-mesh.org
-
David Miller -
Marek Lindner -
Sven Eckelmann