Hello David,
long time no see :)
I know it starts to be a bit late in the release cycle, but I think that these 4 small bug-fixes are still worth being merged.
Patch 1 fixes a compatibility issue between our Distributed ARP Table mechanism and the "early client detection" feature. Such issue creates an inconsistency in the global translation table leading to user clients not being reachable anymore. Fix provided by Simon Wunderlich.
Patch 2 is again provided by Simon Wunderlich and fixes another bug related to the "early client detection" feature. The fix consists in ensuring that temporary client entries not claimed by any originator are properly purged instead of being stored all time long till shutdown.
Patch 3 fixes the function used by the TT hash tables to detect duplicate entries. At the moment two clients with the same MAC address but lying in different VLANs are considered the same, while this should not be the case. Bugfix by Marek Lindner.
Patch 4 fixes an invalid stack access in the batadv_dat_select_candidates() function where a u32 pointer is accessed as it was a pointer to a batadv_dat_entry struct (larger than u32). Bugfix provided by Sven Eckelmann.
Please pull or let me know of any problem!
Thanks a lot, Antonio
The following changes since commit 326fcfa5acca446b3f71e99f6d19881145556e5c:
net: remove unnecessary semicolon in netdev_alloc_pcpu_stats() (2015-12-06 22:32:32 -0500)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batman-adv-fix-for-davem
for you to fetch changes up to b7fe3d4f4a65bc675e737d88071300ea9c4bcddd:
batman-adv: Fix invalid stack access in batadv_dat_select_candidates (2015-12-07 22:40:21 +0800)
---------------------------------------------------------------- Included changes: - prevent compatibility issue between DAT and speedy join from creating inconsistencies in the global translation table - make sure temporary TT entries are purged out if not claimed - fix comparison function used for TT hash table - fix invalid stack access in batadv_dat_select_candidates()
---------------------------------------------------------------- Marek Lindner (1): batman-adv: fix erroneous client entry duplicate detection
Simon Wunderlich (2): batman-adv: fix speedy join for DAT cache replies batman-adv: avoid keeping false temporary entry
Sven Eckelmann (1): batman-adv: Fix invalid stack access in batadv_dat_select_candidates
net/batman-adv/distributed-arp-table.c | 5 ++++- net/batman-adv/routing.c | 19 +++++++++++++++---- net/batman-adv/translation-table.c | 16 ++++++++++++---- 3 files changed, 31 insertions(+), 9 deletions(-)
From: Simon Wunderlich sw@simonwunderlich.de
DAT Cache replies are answered on behalf of other clients which are not connected to the answering originator. Therefore, we shouldn't add these clients to the answering originators TT table through speed join to avoid bogus entries.
Reported-by: Alessandro Bolletta alessandro@mediaspot.net Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Acked-by: Antonio Quartulli antonio@meshcoding.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/routing.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index 8d990b0..3207667 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -836,6 +836,7 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, u8 *orig_addr; struct batadv_orig_node *orig_node = NULL; int check, hdr_size = sizeof(*unicast_packet); + enum batadv_subtype subtype; bool is4addr;
unicast_packet = (struct batadv_unicast_packet *)skb->data; @@ -863,10 +864,20 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, /* packet for me */ if (batadv_is_my_mac(bat_priv, unicast_packet->dest)) { if (is4addr) { - batadv_dat_inc_counter(bat_priv, - unicast_4addr_packet->subtype); - orig_addr = unicast_4addr_packet->src; - orig_node = batadv_orig_hash_find(bat_priv, orig_addr); + subtype = unicast_4addr_packet->subtype; + batadv_dat_inc_counter(bat_priv, subtype); + + /* Only payload data should be considered for speedy + * join. For example, DAT also uses unicast 4addr + * types, but those packets should not be considered + * for speedy join, since the clients do not actually + * reside at the sending originator. + */ + if (subtype == BATADV_P_DATA) { + orig_addr = unicast_4addr_packet->src; + orig_node = batadv_orig_hash_find(bat_priv, + orig_addr); + } }
if (batadv_dat_snoop_incoming_arp_request(bat_priv, skb,
From: Simon Wunderlich sw@simonwunderlich.de
In the case when a temporary entry is added first and a proper tt entry is added after that, the temporary tt entry is kept in the orig list. However the temporary flag is removed at this point, and therefore the purge function can not find this temporary entry anymore.
Therefore, remove the previous temp entry before adding the new proper one.
This case can happen if a client behind a given originator moves before the TT announcement is sent out. Other than that, this case can also be created by bogus or malicious payload frames for VLANs which are not existent on the sending originator.
Reported-by: Alessandro Bolletta alessandro@mediaspot.net Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Acked-by: Antonio Quartulli antonio@meshcoding.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/translation-table.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 4228b10..a3fc9033 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1427,9 +1427,15 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv, }
/* if the client was temporary added before receiving the first - * OGM announcing it, we have to clear the TEMP flag + * OGM announcing it, we have to clear the TEMP flag. Also, + * remove the previous temporary orig node and re-add it + * if required. If the orig entry changed, the new one which + * is a non-temporary entry is preferred. */ - common->flags &= ~BATADV_TT_CLIENT_TEMP; + if (common->flags & BATADV_TT_CLIENT_TEMP) { + batadv_tt_global_del_orig_list(tt_global_entry); + common->flags &= ~BATADV_TT_CLIENT_TEMP; + }
/* the change can carry possible "attribute" flags like the * TT_CLIENT_WIFI, therefore they have to be copied in the
From: Marek Lindner mareklindner@neomailbox.ch
The translation table implementation, namely batadv_compare_tt(), is used to compare two client entries and deciding if they are the holding the same information. Each client entry is identified by its mac address and its VLAN id (VID). Consequently, batadv_compare_tt() has to not only compare the mac addresses but also the VIDs.
Without this fix adding a new client entry that possesses the same mac address as another client but operates on a different VID will fail because both client entries will considered identical.
Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/translation-table.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index a3fc9033..76f19ba 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -68,13 +68,15 @@ static void batadv_tt_global_del(struct batadv_priv *bat_priv, unsigned short vid, const char *message, bool roaming);
-/* returns 1 if they are the same mac addr */ +/* returns 1 if they are the same mac addr and vid */ static int batadv_compare_tt(const struct hlist_node *node, const void *data2) { const void *data1 = container_of(node, struct batadv_tt_common_entry, hash_entry); + const struct batadv_tt_common_entry *tt1 = data1; + const struct batadv_tt_common_entry *tt2 = data2;
- return batadv_compare_eth(data1, data2); + return (tt1->vid == tt2->vid) && batadv_compare_eth(data1, data2); }
/**
From: Sven Eckelmann sven@open-mesh.com
batadv_dat_select_candidates provides an u32 to batadv_hash_dat but it needs a batadv_dat_entry with at least ip and vid filled in.
Fixes: 3e26722bc9f2 ("batman-adv: make the Distributed ARP Table vlan aware") Signed-off-by: Sven Eckelmann sven@open-mesh.com Acked-by: Antonio Quartulli antonio@meshcoding.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/distributed-arp-table.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index 83bc1aa..a49c705 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -566,6 +566,7 @@ batadv_dat_select_candidates(struct batadv_priv *bat_priv, __be32 ip_dst) int select; batadv_dat_addr_t last_max = BATADV_DAT_ADDR_MAX, ip_key; struct batadv_dat_candidate *res; + struct batadv_dat_entry dat;
if (!bat_priv->orig_hash) return NULL; @@ -575,7 +576,9 @@ batadv_dat_select_candidates(struct batadv_priv *bat_priv, __be32 ip_dst) if (!res) return NULL;
- ip_key = (batadv_dat_addr_t)batadv_hash_dat(&ip_dst, + dat.ip = ip_dst; + dat.vid = 0; + ip_key = (batadv_dat_addr_t)batadv_hash_dat(&dat, BATADV_DAT_ADDR_MAX);
batadv_dbg(BATADV_DBG_DAT, bat_priv,
From: Antonio Quartulli antonio@meshcoding.com Date: Mon, 7 Dec 2015 23:12:11 +0800
I know it starts to be a bit late in the release cycle, but I think that these 4 small bug-fixes are still worth being merged.
Pulled, thanks.
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
David Miller