B.A.T.M.A.N

b.a.t.m.a.n@lists.open-mesh.org

15 participants
4308 discussions

[B.A.T.M.A.N.] [PATCHv3 maint] batman-adv: protect tt_local_entry from concurrent delete events
by Marek Lindner 19 Jun '15

19 Jun '15

The tt_local_entry deletion performed in batadv_tt_local_remove() was neither protecting against simultaneous deletes nor checking whether the element was still part of the list before calling hlist_del_rcu(). Replacing the hlist_del_rcu() call with batadv_hash_remove() provides adequate protection via hash spinlocks as well as an is-element-still-in-hash check to avoid 'blind' hash removal. Fixes: 2443ba3 ("batman-adv: roaming handling mechanism redesign") Reported-by: alfonsname(a)web.de Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch> --- v3: added 'fixes' section v2: rebased on master translation-table.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/translation-table.c b/translation-table.c index f56b847..a65d10d 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1018,6 +1018,7 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, struct batadv_tt_local_entry *tt_local_entry; uint16_t flags, curr_flags = BATADV_NO_FLAGS; struct batadv_softif_vlan *vlan; + void *tt_entry_exists; tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid); if (!tt_local_entry) @@ -1045,7 +1046,15 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, * immediately purge it */ batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL); - hlist_del_rcu(&tt_local_entry->common.hash_entry); + + tt_entry_exists = batadv_hash_remove(bat_priv->tt.local_hash, + batadv_compare_tt, + batadv_choose_tt, + &tt_local_entry->common); + if (!tt_entry_exists) + goto out; + + /* extra call to free the local tt entry */ batadv_tt_local_entry_free_ref(tt_local_entry); /* decrease the reference held for this vlan */ -- 2.1.4

2 2

[B.A.T.M.A.N.] [PATCHv3 maint] batman-adv: fix kernel crash due to missing NULL checks
by Marek Lindner 19 Jun '15

19 Jun '15

batadv_softif_vlan_get() may return NULL which has to be verified by the caller. Fixes: 9729d20 ("batman-adv: fix TT VLAN inconsistency on VLAN re-add") Reported-by: Ryan Thompson <ryan(a)eero.com> Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch> --- v3: moved WARN() into if expression v2: added WARN() in batadv_tt_local_add() soft-interface.c | 3 +++ translation-table.c | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/soft-interface.c b/soft-interface.c index da89336..7841a4b 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -455,6 +455,9 @@ out: */ void batadv_softif_vlan_free_ref(struct batadv_softif_vlan *vlan) { + if (!vlan) + return; + if (atomic_dec_and_test(&vlan->refcount)) { spin_lock_bh(&vlan->bat_priv->softif_vlan_list_lock); hlist_del_rcu(&vlan->list); diff --git a/translation-table.c b/translation-table.c index e95a424..f56b847 100644 --- a/translation-table.c +++ b/translation-table.c @@ -575,6 +575,9 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, /* increase the refcounter of the related vlan */ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (WARN(!vlan, "adding TT local entry %pM to non-existent VLAN %d", + addr, BATADV_PRINT_VID(vid))) + goto out; batadv_dbg(BATADV_DBG_TT, bat_priv, "Creating new local tt entry: %pM (vid: %d, ttvn: %d)\n", @@ -1047,6 +1050,9 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (!vlan) + goto out; + batadv_softif_vlan_free_ref(vlan); batadv_softif_vlan_free_ref(vlan); @@ -1147,8 +1153,10 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv) /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, tt_common_entry->vid); - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); + if (vlan) { + batadv_softif_vlan_free_ref(vlan); + batadv_softif_vlan_free_ref(vlan); + } batadv_tt_local_entry_free_ref(tt_local); } @@ -3188,8 +3196,10 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv) /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, tt_common->vid); - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); + if (vlan) { + batadv_softif_vlan_free_ref(vlan); + batadv_softif_vlan_free_ref(vlan); + } batadv_tt_local_entry_free_ref(tt_local); } -- 2.1.4

2 2

[B.A.T.M.A.N.] [PATCHv2 maint] batman-adv: fix kernel crash due to missing NULL checks
by Marek Lindner 19 Jun '15

19 Jun '15

batadv_softif_vlan_get() may return NULL which has to be verified by the caller. Reported-by: Ryan Thompson <ryan(a)eero.com> Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch> --- v2: add WARN() in batadv_tt_local_add() soft-interface.c | 3 +++ translation-table.c | 20 ++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/soft-interface.c b/soft-interface.c index da89336..7841a4b 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -455,6 +455,9 @@ out: */ void batadv_softif_vlan_free_ref(struct batadv_softif_vlan *vlan) { + if (!vlan) + return; + if (atomic_dec_and_test(&vlan->refcount)) { spin_lock_bh(&vlan->bat_priv->softif_vlan_list_lock); hlist_del_rcu(&vlan->list); diff --git a/translation-table.c b/translation-table.c index e95a424..ae82f69 100644 --- a/translation-table.c +++ b/translation-table.c @@ -575,6 +575,11 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, /* increase the refcounter of the related vlan */ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (!vlan) { + WARN(1, "adding TT local entry %pM to non-existent VLAN %d", + addr, BATADV_PRINT_VID(vid)); + goto out; + } batadv_dbg(BATADV_DBG_TT, bat_priv, "Creating new local tt entry: %pM (vid: %d, ttvn: %d)\n", @@ -1047,6 +1052,9 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (!vlan) + goto out; + batadv_softif_vlan_free_ref(vlan); batadv_softif_vlan_free_ref(vlan); @@ -1147,8 +1155,10 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv) /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, tt_common_entry->vid); - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); + if (vlan) { + batadv_softif_vlan_free_ref(vlan); + batadv_softif_vlan_free_ref(vlan); + } batadv_tt_local_entry_free_ref(tt_local); } @@ -3188,8 +3198,10 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv) /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, tt_common->vid); - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); + if (vlan) { + batadv_softif_vlan_free_ref(vlan); + batadv_softif_vlan_free_ref(vlan); + } batadv_tt_local_entry_free_ref(tt_local); } -- 2.1.4

2 2

[B.A.T.M.A.N.] [PATCH] batman-adv: update kernel doc of batadv_tt_global_del_orig_entry()
by Marek Lindner 18 Jun '15

18 Jun '15

The updated kernel doc & additional comment shall prevent accidental copy & paste errors or calling the function without the required precautions. Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch> --- translation-table.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/translation-table.c b/translation-table.c index fa122a2..f6ac67c 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1630,20 +1630,26 @@ out: } /** - * batadv_tt_global_del_orig_entry - remove and free an orig_entry + * _batadv_tt_global_del_orig_entry - remove and free an orig_entry * @tt_global_entry: the global entry to remove the orig_entry from * @orig_entry: the orig entry to remove and free * * Remove an orig_entry from its list in the given tt_global_entry and * free this orig_entry afterwards. + * + * Caller must hold tt_global_entry->list_lock and ensure orig_entry->list is + * part of a list. */ static void -batadv_tt_global_del_orig_entry(struct batadv_tt_global_entry *tt_global_entry, - struct batadv_tt_orig_list_entry *orig_entry) +_batadv_tt_global_del_orig_entry(struct batadv_tt_global_entry *tt_global_entry, + struct batadv_tt_orig_list_entry *orig_entry) { batadv_tt_global_size_dec(orig_entry->orig_node, tt_global_entry->common.vid); atomic_dec(&tt_global_entry->orig_list_count); + /* requires holding tt_global_entry->list_lock and orig_entry->list + * being part of a list + */ hlist_del_rcu(&orig_entry->list); batadv_tt_orig_list_entry_free_ref(orig_entry); } @@ -1659,7 +1665,7 @@ batadv_tt_global_del_orig_list(struct batadv_tt_global_entry *tt_global_entry) spin_lock_bh(&tt_global_entry->list_lock); head = &tt_global_entry->orig_list; hlist_for_each_entry_safe(orig_entry, safe, head, list) - batadv_tt_global_del_orig_entry(tt_global_entry, orig_entry); + _batadv_tt_global_del_orig_entry(tt_global_entry, orig_entry); spin_unlock_bh(&tt_global_entry->list_lock); } @@ -1694,8 +1700,8 @@ batadv_tt_global_del_orig_node(struct batadv_priv *bat_priv, orig_node->orig, tt_global_entry->common.addr, BATADV_PRINT_VID(vid), message); - batadv_tt_global_del_orig_entry(tt_global_entry, - orig_entry); + _batadv_tt_global_del_orig_entry(tt_global_entry, + orig_entry); } } spin_unlock_bh(&tt_global_entry->list_lock); -- 2.1.4

1 0

[B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix compile error on deactivated MCAST feature
by Linus Lüssing 18 Jun '15

18 Jun '15

Some members of "struct batadv_orig_node" are not available if compiling without the multicast optimizations feature. Fix this by moving their initialization into the right #ifdef's. Fixes: 7f220ed1f063 ("batman-adv: Fix potential synchronization issues in mcast tvlv handler") Signed-off-by: Linus Lüssing <linus.luessing(a)c0d3.blue> --- originator.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/originator.c b/originator.c index a2ba182..a5276db 100644 --- a/originator.c +++ b/originator.c @@ -658,15 +658,11 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv, INIT_HLIST_HEAD(&orig_node->neigh_list); INIT_LIST_HEAD(&orig_node->vlan_list); INIT_HLIST_HEAD(&orig_node->ifinfo_list); - INIT_HLIST_NODE(&orig_node->mcast_want_all_unsnoopables_node); - INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv4_node); - INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv6_node); spin_lock_init(&orig_node->bcast_seqno_lock); spin_lock_init(&orig_node->neigh_list_lock); spin_lock_init(&orig_node->tt_buff_lock); spin_lock_init(&orig_node->tt_lock); spin_lock_init(&orig_node->vlan_list_lock); - spin_lock_init(&orig_node->mcast_handler_lock); batadv_nc_init_orig(orig_node); @@ -682,8 +678,13 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv, orig_node->last_seen = jiffies; reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS); orig_node->bcast_seqno_reset = reset_time; + #ifdef CONFIG_BATMAN_ADV_MCAST orig_node->mcast_flags = BATADV_NO_FLAGS; + INIT_HLIST_NODE(&orig_node->mcast_want_all_unsnoopables_node); + INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv4_node); + INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv6_node); + spin_lock_init(&orig_node->mcast_handler_lock); #endif /* create a vlan object for the "untagged" LAN */ -- 1.7.10.4

2 1

[B.A.T.M.A.N.] [BATMAN] batman-adv PING layer 3
by Carlos Meralto 17 Jun '15

17 Jun '15

Hello everyone. I have three mesh nodes. Node A don’t see node C. A can't communicate with C directly, but it can commuticate with C via node B, which is between A and C. The path is A - B - C. On node A, I can run the command 'batctl ping MAC' to node B and node C successfully. When i try the normal layer 3 ping [ping IP] it’s ok to node B for node C doesn't work. Maybe i have a wrong configuration at layer 3. My configurations: /etc/config/network config interface 'lan' option ifname 'eth0.1' option type 'bridge' option proto 'static' option ipaddr '192.168.0.1' option netmask ‘255.255.255.0' config interface 'mesh' option ifname 'adhoc0' option mtu '1528' option proto 'static' option mesh 'bat0' option ipaddr '192.168.11.1' option netmask ‘255.255.255.0' /etc/config/wireless config wifi-iface option device 'radio0' option ifname 'adhoc0' option network 'mesh' option mode 'adhoc' option ssid 'batman-mesh' option encryption 'none' option bssid '02:CA:FE:CA:CA:40’ /etc/config/batman-adv config mesh 'bat0' option interfaces 'adhoc0' option 'aggregated_ogms' option ‘ap_isolation' [..] root@OpenWrt:~# batctl if adhoc0: active the difference between nodes is the ip address for interface lan(192.168.0.1, 192.168.2.1, 192.168.3.1) and mesh (192.168.11.1, 192.168.11.2, 192.168.11.3) What i need to setup to can ping from node A to C? Best Regards.

2 4

[B.A.T.M.A.N.] [PATCHv2 maint] batman-adv: protect tt_local_entry from concurrent delete events
by Marek Lindner 17 Jun '15

17 Jun '15

The tt_local_entry deletion performed in batadv_tt_local_remove() was neither protecting against simultaneous deletes nor checking whether the element was still part of the list before calling hlist_del_rcu(). Replacing the hlist_del_rcu() call with batadv_hash_remove() provides adequate protection via hash spinlocks as well as an is-element-still-in-hash check to avoid 'blind' hash removal. Reported-by: alfonsname(a)web.de Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch> --- translation-table.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/translation-table.c b/translation-table.c index e95a424..fa90a6a 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1015,6 +1015,7 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, struct batadv_tt_local_entry *tt_local_entry; uint16_t flags, curr_flags = BATADV_NO_FLAGS; struct batadv_softif_vlan *vlan; + void *tt_entry_exists; tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid); if (!tt_local_entry) @@ -1042,7 +1043,15 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, * immediately purge it */ batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL); - hlist_del_rcu(&tt_local_entry->common.hash_entry); + + tt_entry_exists = batadv_hash_remove(bat_priv->tt.local_hash, + batadv_compare_tt, + batadv_choose_tt, + &tt_local_entry->common); + if (!tt_entry_exists) + goto out; + + /* extra call to free the local tt entry */ batadv_tt_local_entry_free_ref(tt_local_entry); /* decrease the reference held for this vlan */ -- 2.1.4

2 1

[B.A.T.M.A.N.] [PATCH] batman-adv: fix kernel crash due to missing NULL checks
by Marek Lindner 17 Jun '15

17 Jun '15

batadv_softif_vlan_get() may return NULL which has to be verified by the caller. Reported-by: Ryan Thompson <ryan(a)eero.com> Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch> --- soft-interface.c | 3 +++ translation-table.c | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/soft-interface.c b/soft-interface.c index da89336..7841a4b 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -455,6 +455,9 @@ out: */ void batadv_softif_vlan_free_ref(struct batadv_softif_vlan *vlan) { + if (!vlan) + return; + if (atomic_dec_and_test(&vlan->refcount)) { spin_lock_bh(&vlan->bat_priv->softif_vlan_list_lock); hlist_del_rcu(&vlan->list); diff --git a/translation-table.c b/translation-table.c index e95a424..efbcb59 100644 --- a/translation-table.c +++ b/translation-table.c @@ -575,6 +575,8 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, /* increase the refcounter of the related vlan */ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (!vlan) + goto out; batadv_dbg(BATADV_DBG_TT, bat_priv, "Creating new local tt entry: %pM (vid: %d, ttvn: %d)\n", @@ -1047,6 +1049,9 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (!vlan) + goto out; + batadv_softif_vlan_free_ref(vlan); batadv_softif_vlan_free_ref(vlan); @@ -1147,8 +1152,10 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv) /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, tt_common_entry->vid); - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); + if (vlan) { + batadv_softif_vlan_free_ref(vlan); + batadv_softif_vlan_free_ref(vlan); + } batadv_tt_local_entry_free_ref(tt_local); } @@ -3188,8 +3195,10 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv) /* decrease the reference held for this vlan */ vlan = batadv_softif_vlan_get(bat_priv, tt_common->vid); - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); + if (vlan) { + batadv_softif_vlan_free_ref(vlan); + batadv_softif_vlan_free_ref(vlan); + } batadv_tt_local_entry_free_ref(tt_local); } -- 2.1.4

2 1

[B.A.T.M.A.N.] [PATCH] batman-adv: protect tt_local_entry from concurrent delete events
by Marek Lindner 17 Jun '15

17 Jun '15

The tt_local_entry deletion performed batadv_tt_local_remove() was neither protecting against simultaneous deletes nor checking whether the element was still part of the list before calling hlist_del_rcu(). Replacing the hlist_del_rcu() with batadv_hash_remove() provides adequate protection via hash spinlocks as well as a is-element-still-in-hash check. Reported-by: alfonsname(a)web.de Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch> --- translation-table.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/translation-table.c b/translation-table.c index e95a424..4afeeb5 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1042,7 +1042,8 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv, * immediately purge it */ batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL); - hlist_del_rcu(&tt_local_entry->common.hash_entry); + batadv_hash_remove(bat_priv->tt.local_hash, batadv_compare_tt, + batadv_choose_tt, &tt_local_entry->common); batadv_tt_local_entry_free_ref(tt_local_entry); /* decrease the reference held for this vlan */ -- 2.1.4

2 2

[B.A.T.M.A.N.] [PATCHv4 maint 0/5] Fixes for parallel OGM processing
by Linus Lüssing 17 Jun '15

17 Jun '15

Hi, Here are five fixes which address potential problems with parallel OGM processing. For the multicast subsystem the issues can be quite severe as found and pointed out by Sven here [0]. Cheers, Linus [0]: https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-June/013193.html ----- Changelog v4: * [PATCHv4 maint 5/5]: * Shortened if...BUG() to BUG_ON() Changelog v3: * [PATCHv3 maint 5/5]: * added kerneldoc for new mcast_handler_lock in types.h Changelog v2: * Split [PATCH maint 1/2] into [PATCHv2 {1,2,3,4}/5] * [PATCHv2 maint 5/5]: * moved mcast_flags initialization outside of spinlock * added spinlock in batadv_mcast_purge_orig() (not strictly necessary bc. if we are here then the orig-node is out-of-scope as it's going to be kfree'd a few instructions later - but added for style reasons / to avoid future errors) * added hlist_unhashed()+BUG() calls (again, shouldn't be necessary as the checks and new spinlock should avoid this case. But since the safety of hlist_del_rcu() isn't directly obvious, better adding these annotations and to avoid future regressions with kernel panics) PS: Thanks to Marek for the suggestions.

2 10

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

B.A.T.M.A.N