Hi,
we were asked by Greg KH to check the failed backport of some recent fix to linux 4.4.y and provide backports for it when required. I've found a lot more missing patches than I've expected in this process and queued them up here.
Kind regards, Sven
Andrew Lunn (1): batman-adv: Avoid endless loop in bat-on-bat netdevice check
Florian Westphal (1): batman-adv: fix skb deref after free
Linus Lüssing (5): batman-adv: Avoid duplicate neigh_node additions batman-adv: Fix transmission of final, 16th fragment batman-adv: fix TT sync flag inconsistencies batman-adv: Fix TT sync flags for intermediate TT responses batman-adv: Avoid storing non-TT-sync flags on singular entries too
Marek Lindner (2): batman-adv: init neigh node last seen field batman-adv: prevent TT request storms by not sending inconsistent TT TLVLs
Matthias Schiffer (1): batman-adv: update data pointers after skb_cow()
Simon Wunderlich (1): batman-adv: lock crc access in bridge loop avoidance
Sven Eckelmann (37): batman-adv: Fix invalid read while copying bat_iv.bcast_own batman-adv: Only put gw_node list reference when removed batman-adv: Only put orig_node_vlan list reference when removed batman-adv: Fix unexpected free of bcast_own on add_if error batman-adv: Fix integer overflow in batadv_iv_ogm_calc_tq batman-adv: Deactivate TO_BE_ACTIVATED hardif on shutdown batman-adv: Drop reference to netdevice on last reference batman-adv: Fix reference counting of vlan object for tt_local_entry batman-adv: Fix use-after-free/double-free of tt_req_node batman-adv: Fix ICMP RR ethernet access after skb_linearize batman-adv: Clean up untagged vlan when destroying via rtnl-link batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag batman-adv: Avoid nullptr dereference in dat after vlan_insert_tag batman-adv: Fix orig_node_vlan leak on orig_node_release batman-adv: Fix non-atomic bla_claim::backbone_gw access batman-adv: Fix reference leak in batadv_find_router batman-adv: Free last_bonding_candidate on release of orig_node batman-adv: Fix speedy join in gateway client mode batman-adv: Add missing refcnt for last_candidate batman-adv: Fix double free during fragment merge error batman-adv: Fix rx packet/bytes stats on local ARP reply batman-adv: Fix lock for ogm cnt access in batadv_iv_ogm_calc_tq batman-adv: Fix internal interface indices types batman-adv: Fix skbuff rcsum on packet reroute batman-adv: Avoid race in TT TVLV allocator helper batman-adv: Fix debugfs path for renamed hardif batman-adv: Fix debugfs path for renamed softif batman-adv: Prevent duplicated gateway_node entry batman-adv: Prevent duplicated nc_node entry batman-adv: Prevent duplicated global TT entry batman-adv: Prevent duplicated tvlv handler batman-adv: Reduce claim hash refcnt only for removed entry batman-adv: Reduce tt_local hash refcnt only for removed entry batman-adv: Reduce tt_global hash refcnt only for removed entry batman-adv: Only read OGM tvlv_len after buffer len check batman-adv: Avoid free/alloc race when handling OGM buffer batman-adv: Don't schedule OGM for disabled interface
net/batman-adv/bat_iv_ogm.c | 115 +++++++++--- net/batman-adv/bridge_loop_avoidance.c | 152 ++++++++++++--- net/batman-adv/debugfs.c | 40 ++++ net/batman-adv/debugfs.h | 11 ++ net/batman-adv/distributed-arp-table.c | 15 +- net/batman-adv/fragmentation.c | 14 +- net/batman-adv/gateway_client.c | 18 +- net/batman-adv/hard-interface.c | 89 +++++++-- net/batman-adv/hard-interface.h | 6 +- net/batman-adv/main.c | 8 +- net/batman-adv/network-coding.c | 33 ++-- net/batman-adv/originator.c | 26 ++- net/batman-adv/originator.h | 4 +- net/batman-adv/routing.c | 111 ++++++++--- net/batman-adv/send.c | 4 +- net/batman-adv/soft-interface.c | 9 + net/batman-adv/translation-table.c | 249 +++++++++++++++++-------- net/batman-adv/types.h | 23 ++- 18 files changed, 707 insertions(+), 220 deletions(-)
commit 13bbdd370f67aef3351ad7bbc2fb624e3c23f905 upstream.
batadv_iv_ogm_orig_del_if removes a part of the bcast_own which previously belonged to the now removed interface. This is done by copying all data which comes before the removed interface and then appending all the data which comes after the removed interface.
The address calculation for the position of the data which comes after the removed interface assumed that the bat_iv.bcast_own is a pointer to a single byte datatype. But it is a pointer to unsigned long and thus the calculated position was wrong off factor sizeof(unsigned long).
Fixes: 83a8342678a0 ("more basic routing code added (forwarding packets / bitarray added)") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/bat_iv_ogm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 912d9c36fb1c..aa94b4ec766a 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -185,7 +185,8 @@ unlock: static int batadv_iv_ogm_orig_del_if(struct batadv_orig_node *orig_node, int max_if_num, int del_if_num) { - int chunk_size, ret = -ENOMEM, if_offset; + int ret = -ENOMEM; + size_t chunk_size, if_offset; void *data_ptr = NULL;
spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock); @@ -203,8 +204,9 @@ static int batadv_iv_ogm_orig_del_if(struct batadv_orig_node *orig_node, memcpy(data_ptr, orig_node->bat_iv.bcast_own, del_if_num * chunk_size);
/* copy second part */ + if_offset = (del_if_num + 1) * chunk_size; memcpy((char *)data_ptr + del_if_num * chunk_size, - orig_node->bat_iv.bcast_own + ((del_if_num + 1) * chunk_size), + (uint8_t *)orig_node->bat_iv.bcast_own + if_offset, (max_if_num - del_if_num) * chunk_size);
free_bcast_own:
commit c18bdd018e8912ca73ad6c12120b7283b5038875 upstream.
The batadv_gw_node reference counter in batadv_gw_node_update can only be reduced when the list entry was actually removed. Otherwise the reference counter may reach zero when batadv_gw_node_update is called from two different contexts for the same gw_node but only one context is actually removing the entry from the list.
The release function for this gw_node is not called inside the list_lock spinlock protected region because the function batadv_gw_node_update still holds a gw_node reference for the object pointer on the stack. Thus the actual release function (when required) will be called only at the end of the function.
Fixes: bd3524c14bd0 ("batman-adv: remove obsolete deleted attribute for gateway node") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/gateway_client.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c index 6abfba1e227f..055baa396260 100644 --- a/net/batman-adv/gateway_client.c +++ b/net/batman-adv/gateway_client.c @@ -527,11 +527,12 @@ void batadv_gw_node_update(struct batadv_priv *bat_priv, * gets dereferenced. */ spin_lock_bh(&bat_priv->gw.list_lock); - hlist_del_init_rcu(&gw_node->list); + if (!hlist_unhashed(&gw_node->list)) { + hlist_del_init_rcu(&gw_node->list); + batadv_gw_node_free_ref(gw_node); + } spin_unlock_bh(&bat_priv->gw.list_lock);
- batadv_gw_node_free_ref(gw_node); - curr_gw = batadv_gw_get_selected_gw_node(bat_priv); if (gw_node == curr_gw) batadv_gw_reselect(bat_priv);
commit 3db152093efb750bc47fd4d69355b90b18113105 upstream.
The batadv_orig_node_vlan reference counter in batadv_tt_global_size_mod can only be reduced when the list entry was actually removed. Otherwise the reference counter may reach zero when batadv_tt_global_size_mod is called from two different contexts for the same orig_node_vlan but only one context is actually removing the entry from the list.
The release function for this orig_node_vlan is not called inside the vlan_list_lock spinlock protected region because the function batadv_tt_global_size_mod still holds a orig_node_vlan reference for the object pointer on the stack. Thus the actual release function (when required) will be called only at the end of the function.
Fixes: 7ea7b4a14275 ("batman-adv: make the TT CRC logic VLAN specific") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- 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 ffd49b40e76a..117febee5fa6 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -303,9 +303,11 @@ static void batadv_tt_global_size_mod(struct batadv_orig_node *orig_node,
if (atomic_add_return(v, &vlan->tt.num_entries) == 0) { spin_lock_bh(&orig_node->vlan_list_lock); - hlist_del_init_rcu(&vlan->list); + if (!hlist_unhashed(&vlan->list)) { + hlist_del_init_rcu(&vlan->list); + batadv_orig_node_vlan_free_ref(vlan); + } spin_unlock_bh(&orig_node->vlan_list_lock); - batadv_orig_node_vlan_free_ref(vlan); }
batadv_orig_node_vlan_free_ref(vlan);
From: Andrew Lunn andrew@lunn.ch
commit 1bc4e2b000e7fa9773d6623bc8850561ce10a4fb upstream.
batman-adv checks in different situation if a new device is already on top of a different batman-adv device. This is done by getting the iflink of a device and all its parent. It assumes that this iflink is always a parent device in an acyclic graph. But this assumption is broken by devices like veth which are actually a pair of two devices linked to each other. The recursive check would therefore get veth0 when calling dev_get_iflink on veth1. And it gets veth0 when calling dev_get_iflink with veth1.
Creating a veth pair and loading batman-adv freezes parts of the system
ip link add veth0 type veth peer name veth1 modprobe batman-adv
An RCU stall will be detected on the system which cannot be fixed.
INFO: rcu_sched self-detected stall on CPU 1: (5264 ticks this GP) idle=3e9/140000000000001/0 softirq=144683/144686 fqs=5249 (t=5250 jiffies g=46 c=45 q=43) Task dump for CPU 1: insmod R running task 0 247 245 0x00000008 ffffffff8151f140 ffffffff8107888e ffff88000fd141c0 ffffffff8151f140 0000000000000000 ffffffff81552df0 ffffffff8107b420 0000000000000001 ffff88000e3fa700 ffffffff81540b00 ffffffff8107d667 0000000000000001 Call Trace: <IRQ> [<ffffffff8107888e>] ? rcu_dump_cpu_stacks+0x7e/0xd0 [<ffffffff8107b420>] ? rcu_check_callbacks+0x3f0/0x6b0 [<ffffffff8107d667>] ? hrtimer_run_queues+0x47/0x180 [<ffffffff8107cf9d>] ? update_process_times+0x2d/0x50 [<ffffffff810873fb>] ? tick_handle_periodic+0x1b/0x60 [<ffffffff810290ae>] ? smp_trace_apic_timer_interrupt+0x5e/0x90 [<ffffffff813bbae2>] ? apic_timer_interrupt+0x82/0x90 <EOI> [<ffffffff812c3fd7>] ? __dev_get_by_index+0x37/0x40 [<ffffffffa0031f3e>] ? batadv_hard_if_event+0xee/0x3a0 [batman_adv] [<ffffffff812c5801>] ? register_netdevice_notifier+0x81/0x1a0 [...]
This can be avoided by checking if two devices are each others parent and stopping the check in this situation.
Fixes: b7eddd0b3950 ("batman-adv: prevent using any virtual device created on batman-adv as hard-interface") Signed-off-by: Andrew Lunn andrew@lunn.ch [sven@narfation.org: rewritten description, extracted fix] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/hard-interface.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 3c8d8142e8c6..933f36012ae6 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -73,6 +73,28 @@ out: return hard_iface; }
+/** + * batadv_mutual_parents - check if two devices are each others parent + * @dev1: 1st net_device + * @dev2: 2nd net_device + * + * veth devices come in pairs and each is the parent of the other! + * + * Return: true if the devices are each others parent, otherwise false + */ +static bool batadv_mutual_parents(const struct net_device *dev1, + const struct net_device *dev2) +{ + int dev1_parent_iflink = dev_get_iflink(dev1); + int dev2_parent_iflink = dev_get_iflink(dev2); + + if (!dev1_parent_iflink || !dev2_parent_iflink) + return false; + + return (dev1_parent_iflink == dev2->ifindex) && + (dev2_parent_iflink == dev1->ifindex); +} + /** * batadv_is_on_batman_iface - check if a device is a batman iface descendant * @net_dev: the device to check @@ -108,6 +130,9 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev) return false; }
+ if (batadv_mutual_parents(net_dev, parent_dev)) + return false; + ret = batadv_is_on_batman_iface(parent_dev);
return ret;
commit f7dcdf5fdbe8fec7670d8f65a5db595c98e0ecab upstream.
The function batadv_iv_ogm_orig_add_if allocates new buffers for bcast_own and bcast_own_sum. It is expected that these buffers are unchanged in case either bcast_own or bcast_own_sum couldn't be resized.
But the error handling of this function frees the already resized buffer for bcast_own when the allocation of the new bcast_own_sum buffer failed. This will lead to an invalid memory access when some code will try to access bcast_own.
Instead the resized new bcast_own buffer has to be kept. This will not lead to problems because the size of the buffer was only increased and therefore no user of the buffer will try to access bytes outside of the new buffer.
Fixes: d0015fdd3d2c ("batman-adv: provide orig_node routing API") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/bat_iv_ogm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index aa94b4ec766a..cea4e321f588 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -155,10 +155,8 @@ static int batadv_iv_ogm_orig_add_if(struct batadv_orig_node *orig_node, orig_node->bat_iv.bcast_own = data_ptr;
data_ptr = kmalloc_array(max_if_num, sizeof(u8), GFP_ATOMIC); - if (!data_ptr) { - kfree(orig_node->bat_iv.bcast_own); + if (!data_ptr) goto unlock; - }
memcpy(data_ptr, orig_node->bat_iv.bcast_own_sum, (max_if_num - 1) * sizeof(u8));
From: Sven Eckelmann sven.eckelmann@open-mesh.com
commit d285f52cc0f23564fd61976d43fd5b991b4828f6 upstream.
The undefined behavior sanatizer detected an signed integer overflow in a setup with near perfect link quality
UBSAN: Undefined behaviour in net/batman-adv/bat_iv_ogm.c:1246:25 signed integer overflow: 8713350 * 255 cannot be represented in type 'int'
The problems happens because the calculation of mixed unsigned and signed integers resulted in an integer multiplication.
batadv_ogm_packet::tq (u8 255) * tq_own (u8 255) * tq_asym_penalty (int 134; max 255) * tq_iface_penalty (int 255; max 255)
The tq_iface_penalty, tq_asym_penalty and inv_asym_penalty can just be changed to unsigned int because they are not expected to become negative.
Fixes: c039876892e3 ("batman-adv: add WiFi penalty") Signed-off-by: Sven Eckelmann sven.eckelmann@open-mesh.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/bat_iv_ogm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index cea4e321f588..749b9157fe11 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -1140,9 +1140,10 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, u8 total_count; u8 orig_eq_count, neigh_rq_count, neigh_rq_inv, tq_own; unsigned int neigh_rq_inv_cube, neigh_rq_max_cube; - int tq_asym_penalty, inv_asym_penalty, if_num, ret = 0; + int if_num, ret = 0; + unsigned int tq_asym_penalty, inv_asym_penalty; unsigned int combined_tq; - int tq_iface_penalty; + unsigned int tq_iface_penalty;
/* find corresponding one hop neighbor */ rcu_read_lock();
From: Marek Lindner mareklindner@neomailbox.ch
commit e48474ed8a217b7f80f2a42bc05352406a06cb67 upstream.
Signed-off-by: Marek Lindner mareklindner@neomailbox.ch [sven@narfation.org: fix conflicts with current version] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/originator.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index 6282f021ddfb..19b4b91b19ff 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -483,6 +483,7 @@ batadv_neigh_node_new(struct batadv_orig_node *orig_node, ether_addr_copy(neigh_node->addr, neigh_addr); neigh_node->if_incoming = hard_iface; neigh_node->orig_node = orig_node; + neigh_node->last_seen = jiffies;
/* extra reference for return */ atomic_set(&neigh_node->refcount, 2);
commit f2d23861b818d08bcd15cc1612ae94aa33b3931c upstream.
The shutdown of an batman-adv interface can happen with one of its slave interfaces still being in the BATADV_IF_TO_BE_ACTIVATED state. A possible reason for it is that the routing algorithm BATMAN_V was selected and batadv_schedule_bat_ogm was not yet called for this interface. This slave interface still has to be set to BATADV_IF_INACTIVE or the batman-adv interface will never reduce its usage counter and thus never gets shutdown.
This problem can be simulated via:
$ modprobe dummy $ modprobe batman-adv routing_algo=BATMAN_V $ ip link add bat0 type batadv $ ip link set dummy0 master bat0 $ ip link set dummy0 up $ ip link del bat0 unregister_netdevice: waiting for bat0 to become free. Usage count = 3
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/hard-interface.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 933f36012ae6..ecaa68ae62f5 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -562,8 +562,7 @@ void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface, struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface); struct batadv_hard_iface *primary_if = NULL;
- if (hard_iface->if_status == BATADV_IF_ACTIVE) - batadv_hardif_deactivate_interface(hard_iface); + batadv_hardif_deactivate_interface(hard_iface);
if (hard_iface->if_status != BATADV_IF_INACTIVE) goto out;
commit 140ed8e87ca8f4875c2b146cdb2cdbf0c9ac6080 upstream.
The references to the network device should be dropped inside the release function for batadv_hard_iface similar to what is done with the batman-adv internal datastructures.
Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/hard-interface.c | 13 ++++++++----- net/batman-adv/hard-interface.h | 6 +++--- 2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index ecaa68ae62f5..cc3be5489f5c 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -45,13 +45,16 @@ #include "sysfs.h" #include "translation-table.h"
-void batadv_hardif_free_rcu(struct rcu_head *rcu) +/** + * batadv_hardif_release - release hard interface from lists and queue for + * free after rcu grace period + * @hard_iface: the hard interface to free + */ +void batadv_hardif_release(struct batadv_hard_iface *hard_iface) { - struct batadv_hard_iface *hard_iface; - - hard_iface = container_of(rcu, struct batadv_hard_iface, rcu); dev_put(hard_iface->net_dev); - kfree(hard_iface); + + kfree_rcu(hard_iface, rcu); }
struct batadv_hard_iface * diff --git a/net/batman-adv/hard-interface.h b/net/batman-adv/hard-interface.h index 7b12ea8ea29d..4d74c0415911 100644 --- a/net/batman-adv/hard-interface.h +++ b/net/batman-adv/hard-interface.h @@ -61,18 +61,18 @@ void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface, void batadv_hardif_remove_interfaces(void); int batadv_hardif_min_mtu(struct net_device *soft_iface); void batadv_update_min_mtu(struct net_device *soft_iface); -void batadv_hardif_free_rcu(struct rcu_head *rcu); +void batadv_hardif_release(struct batadv_hard_iface *hard_iface);
/** * batadv_hardif_free_ref - decrement the hard interface refcounter and - * possibly free it + * possibly release it * @hard_iface: the hard interface to free */ static inline void batadv_hardif_free_ref(struct batadv_hard_iface *hard_iface) { if (atomic_dec_and_test(&hard_iface->refcount)) - call_rcu(&hard_iface->rcu, batadv_hardif_free_rcu); + batadv_hardif_release(hard_iface); }
static inline struct batadv_hard_iface *
commit a33d970d0b54b09746d5540af8271fad4eb10229 upstream.
The batadv_tt_local_entry was specific to a batadv_softif_vlan and held an implicit reference to it. But this reference was never stored in form of a pointer in the tt_local_entry itself. Instead batadv_tt_local_remove, batadv_tt_local_table_free and batadv_tt_local_purge_pending_clients depend on a consistent state of bat_priv->softif_vlan_list and that batadv_softif_vlan_get always returns the batadv_softif_vlan object which it has a reference for. But batadv_softif_vlan_get cannot guarantee that because it is working only with rcu_read_lock on this list. It can therefore happen that an vid is in this list twice or that batadv_softif_vlan_get cannot find the batadv_softif_vlan for an vid due to some other list operations taking place at the same time.
Instead add a batadv_softif_vlan pointer directly in batadv_tt_local_entry which will be used for the reference counter decremented on release of batadv_tt_local_entry.
Fixes: 35df3b298fc8 ("batman-adv: fix TT VLAN inconsistency on VLAN re-add") Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Antonio Quartulli a@unstable.cc Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/translation-table.c | 44 ++++-------------------------- net/batman-adv/types.h | 2 ++ 2 files changed, 7 insertions(+), 39 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 117febee5fa6..4014e5130879 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -197,8 +197,11 @@ batadv_tt_global_hash_find(struct batadv_priv *bat_priv, const u8 *addr, static void batadv_tt_local_entry_free_ref(struct batadv_tt_local_entry *tt_local_entry) { - if (atomic_dec_and_test(&tt_local_entry->common.refcount)) + if (atomic_dec_and_test(&tt_local_entry->common.refcount)) { + batadv_softif_vlan_free_ref(tt_local_entry->vlan); + kfree_rcu(tt_local_entry, common.rcu); + } }
/** @@ -638,7 +641,6 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const u8 *addr, if (unlikely(hash_added != 0)) { /* remove the reference for the hash */ batadv_tt_local_entry_free_ref(tt_local); - batadv_softif_vlan_free_ref(vlan); goto out; }
@@ -942,7 +944,6 @@ int batadv_tt_local_seq_print_text(struct seq_file *seq, void *offset) struct batadv_tt_common_entry *tt_common_entry; struct batadv_tt_local_entry *tt_local; struct batadv_hard_iface *primary_if; - struct batadv_softif_vlan *vlan; struct hlist_head *head; unsigned short vid; u32 i; @@ -979,13 +980,6 @@ int batadv_tt_local_seq_print_text(struct seq_file *seq, void *offset)
no_purge = tt_common_entry->flags & np_flag;
- vlan = batadv_softif_vlan_get(bat_priv, vid); - if (!vlan) { - seq_printf(seq, "Cannot retrieve VLAN %d\n", - BATADV_PRINT_VID(vid)); - continue; - } - seq_printf(seq, " * %pM %4i [%c%c%c%c%c%c] %3u.%03u (%#.8x)\n", tt_common_entry->addr, @@ -1003,9 +997,7 @@ int batadv_tt_local_seq_print_text(struct seq_file *seq, void *offset) BATADV_TT_CLIENT_ISOLA) ? 'I' : '.'), no_purge ? 0 : last_seen_secs, no_purge ? 0 : last_seen_msecs, - vlan->tt.crc); - - batadv_softif_vlan_free_ref(vlan); + tt_local->vlan->tt.crc); } rcu_read_unlock(); } @@ -1050,7 +1042,6 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr, { struct batadv_tt_local_entry *tt_local_entry; u16 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); @@ -1090,14 +1081,6 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr, /* extra call to free the local tt entry */ batadv_tt_local_entry_free_ref(tt_local_entry);
- /* 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); - out: if (tt_local_entry) batadv_tt_local_entry_free_ref(tt_local_entry); @@ -1170,7 +1153,6 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv) spinlock_t *list_lock; /* protects write access to the hash lists */ struct batadv_tt_common_entry *tt_common_entry; struct batadv_tt_local_entry *tt_local; - struct batadv_softif_vlan *vlan; struct hlist_node *node_tmp; struct hlist_head *head; u32 i; @@ -1192,14 +1174,6 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv) struct batadv_tt_local_entry, common);
- /* decrease the reference held for this vlan */ - vlan = batadv_softif_vlan_get(bat_priv, - tt_common_entry->vid); - if (vlan) { - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); - } - batadv_tt_local_entry_free_ref(tt_local); } spin_unlock_bh(list_lock); @@ -3229,7 +3203,6 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv) struct batadv_hashtable *hash = bat_priv->tt.local_hash; struct batadv_tt_common_entry *tt_common; struct batadv_tt_local_entry *tt_local; - struct batadv_softif_vlan *vlan; struct hlist_node *node_tmp; struct hlist_head *head; spinlock_t *list_lock; /* protects write access to the hash lists */ @@ -3259,13 +3232,6 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv) struct batadv_tt_local_entry, common);
- /* decrease the reference held for this vlan */ - vlan = batadv_softif_vlan_get(bat_priv, tt_common->vid); - if (vlan) { - batadv_softif_vlan_free_ref(vlan); - batadv_softif_vlan_free_ref(vlan); - } - batadv_tt_local_entry_free_ref(tt_local); } spin_unlock_bh(list_lock); diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index cbd347c2e4a5..dcf285fae2e6 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -947,10 +947,12 @@ struct batadv_tt_common_entry { * struct batadv_tt_local_entry - translation table local entry data * @common: general translation table data * @last_seen: timestamp used for purging stale tt local entries + * @vlan: soft-interface vlan of the entry */ struct batadv_tt_local_entry { struct batadv_tt_common_entry common; unsigned long last_seen; + struct batadv_softif_vlan *vlan; };
/**
From: Linus Lüssing linus.luessing@c0d3.blue
commit e123705e58bf171be8c6eb0902ebfb5d6ed255ad upstream.
Two parallel calls to batadv_neigh_node_new() might race for creating and adding the same neig_node. Fix this by including the check for any already existing, identical neigh_node within the spin-lock.
This fixes splats like the following:
[ 739.535069] ------------[ cut here ]------------ [ 739.535079] WARNING: CPU: 0 PID: 0 at /usr/src/batman-adv/git/batman-adv/net/batman-adv/bat_iv_ogm.c:1004 batadv_iv_ogm_process_per_outif+0xe3f/0xe60 [batman_adv]() [ 739.535092] too many matching neigh_nodes [ 739.535094] Modules linked in: dm_mod tun ip6table_filter ip6table_mangle ip6table_nat nf_nat_ipv6 ip6_tables xt_nat iptable_nat nf_nat_ipv4 nf_nat xt_TCPMSS xt_mark iptable_mangle xt_tcpudp xt_conntrack iptable_filter ip_tables x_tables ip_gre ip_tunnel gre bridge stp llc thermal_sys kvm_intel kvm crct10dif_pclmul crc32_pclmul sha256_ssse3 sha256_generic hmac drbg ansi_cprng aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd evdev pcspkr ip6_gre ip6_tunnel tunnel6 batman_adv(O) libcrc32c nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack autofs4 ext4 crc16 mbcache jbd2 xen_netfront xen_blkfront crc32c_intel [ 739.535177] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W O 4.2.0-0.bpo.1-amd64 #1 Debian 4.2.6-3~bpo8+2 [ 739.535186] 0000000000000000 ffffffffa013b050 ffffffff81554521 ffff88007d003c18 [ 739.535201] ffffffff8106fa01 0000000000000000 ffff8800047a087a ffff880079c3a000 [ 739.735602] ffff88007b82bf40 ffff88007bc2d1c0 ffffffff8106fa7a ffffffffa013aa8e [ 739.735624] Call Trace: [ 739.735639] <IRQ> [<ffffffff81554521>] ? dump_stack+0x40/0x50 [ 739.735677] [<ffffffff8106fa01>] ? warn_slowpath_common+0x81/0xb0 [ 739.735692] [<ffffffff8106fa7a>] ? warn_slowpath_fmt+0x4a/0x50 [ 739.735715] [<ffffffffa012448f>] ? batadv_iv_ogm_process_per_outif+0xe3f/0xe60 [batman_adv] [ 739.735740] [<ffffffffa0124813>] ? batadv_iv_ogm_receive+0x363/0x380 [batman_adv] [ 739.735762] [<ffffffffa0124813>] ? batadv_iv_ogm_receive+0x363/0x380 [batman_adv] [ 739.735783] [<ffffffff810b0841>] ? __raw_callee_save___pv_queued_spin_unlock+0x11/0x20 [ 739.735804] [<ffffffffa012cb39>] ? batadv_batman_skb_recv+0xc9/0x110 [batman_adv] [ 739.735825] [<ffffffff81464891>] ? __netif_receive_skb_core+0x841/0x9a0 [ 739.735838] [<ffffffff810b0841>] ? __raw_callee_save___pv_queued_spin_unlock+0x11/0x20 [ 739.735853] [<ffffffff81465681>] ? process_backlog+0xa1/0x140 [ 739.735864] [<ffffffff81464f1a>] ? net_rx_action+0x20a/0x320 [ 739.735878] [<ffffffff81073aa7>] ? __do_softirq+0x107/0x270 [ 739.735891] [<ffffffff81073d82>] ? irq_exit+0x92/0xa0 [ 739.735905] [<ffffffff8137e0d1>] ? xen_evtchn_do_upcall+0x31/0x40 [ 739.735924] [<ffffffff8155b8fe>] ? xen_do_hypervisor_callback+0x1e/0x40 [ 739.735939] <EOI> [<ffffffff810013aa>] ? xen_hypercall_sched_op+0xa/0x20 [ 739.735965] [<ffffffff810013aa>] ? xen_hypercall_sched_op+0xa/0x20 [ 739.735979] [<ffffffff8100a39c>] ? xen_safe_halt+0xc/0x20 [ 739.735991] [<ffffffff8101da6c>] ? default_idle+0x1c/0xa0 [ 739.736004] [<ffffffff810abf6b>] ? cpu_startup_entry+0x2eb/0x350 [ 739.736019] [<ffffffff81b2af5e>] ? start_kernel+0x480/0x48b [ 739.736032] [<ffffffff81b2d116>] ? xen_start_kernel+0x507/0x511 [ 739.736048] ---[ end trace c106bb901244bc8c ]---
Fixes: f987ed6ebd99 ("batman-adv: protect neighbor list with rcu locks") Reported-by: Martin Weinelt martin@darmstadt.freifunk.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/originator.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index 19b4b91b19ff..71d0f6deb8aa 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -462,6 +462,8 @@ batadv_neigh_node_new(struct batadv_orig_node *orig_node, { struct batadv_neigh_node *neigh_node;
+ spin_lock_bh(&orig_node->neigh_list_lock); + neigh_node = batadv_neigh_node_get(orig_node, hard_iface, neigh_addr); if (neigh_node) goto out; @@ -488,15 +490,15 @@ batadv_neigh_node_new(struct batadv_orig_node *orig_node, /* extra reference for return */ atomic_set(&neigh_node->refcount, 2);
- spin_lock_bh(&orig_node->neigh_list_lock); hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list); - spin_unlock_bh(&orig_node->neigh_list_lock);
batadv_dbg(BATADV_DBG_BATMAN, orig_node->bat_priv, "Creating new neighbor %pM for orig_node %pM on interface %s\n", neigh_addr, orig_node->orig, hard_iface->net_dev->name);
out: + spin_unlock_bh(&orig_node->neigh_list_lock); + return neigh_node; }
From: Florian Westphal fw@strlen.de
commit 63d443efe8be2c1d02b30d7e4edeb9aa085352b3 upstream.
batadv_send_skb_to_orig() calls dev_queue_xmit() so we can't use skb->len.
Fixes: 953324776d6d ("batman-adv: network coding - buffer unicast packets before forward") Signed-off-by: Florian Westphal fw@strlen.de Reviewed-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/routing.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index d8a2f33e60e5..de24ab1cf67f 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -585,6 +585,7 @@ static int batadv_route_unicast_packet(struct sk_buff *skb, struct batadv_unicast_packet *unicast_packet; struct ethhdr *ethhdr = eth_hdr(skb); int res, hdr_len, ret = NET_RX_DROP; + unsigned int len;
unicast_packet = (struct batadv_unicast_packet *)skb->data;
@@ -625,6 +626,7 @@ static int batadv_route_unicast_packet(struct sk_buff *skb, if (hdr_len > 0) batadv_skb_set_priority(skb, hdr_len);
+ len = skb->len; res = batadv_send_skb_to_orig(skb, orig_node, recv_if);
/* translate transmit result into receive result */ @@ -632,7 +634,7 @@ static int batadv_route_unicast_packet(struct sk_buff *skb, /* skb was transmitted and consumed */ batadv_inc_counter(bat_priv, BATADV_CNT_FORWARD); batadv_add_counter(bat_priv, BATADV_CNT_FORWARD_BYTES, - skb->len + ETH_HLEN); + len + ETH_HLEN);
ret = NET_RX_SUCCESS; } else if (res == NET_XMIT_POLICED) {
commit 9c4604a298e0a9807eaf2cd912d1ebf24d98fbeb upstream.
The tt_req_node is added and removed from a list inside a spinlock. But the locking is sometimes removed even when the object is still referenced and will be used later via this reference. For example batadv_send_tt_request can create a new tt_req_node (including add to a list) and later re-acquires the lock to remove it from the list and to free it. But at this time another context could have already removed this tt_req_node from the list and freed it.
CPU#0
batadv_batman_skb_recv from net_device 0 -> batadv_iv_ogm_receive -> batadv_iv_ogm_process -> batadv_iv_ogm_process_per_outif -> batadv_tvlv_ogm_receive -> batadv_tvlv_ogm_receive -> batadv_tvlv_containers_process -> batadv_tvlv_call_handler -> batadv_tt_tvlv_ogm_handler_v1 -> batadv_tt_update_orig -> batadv_send_tt_request -> batadv_tt_req_node_new spin_lock(...) allocates new tt_req_node and adds it to list spin_unlock(...) return tt_req_node
CPU#1
batadv_batman_skb_recv from net_device 1 -> batadv_recv_unicast_tvlv -> batadv_tvlv_containers_process -> batadv_tvlv_call_handler -> batadv_tt_tvlv_unicast_handler_v1 -> batadv_handle_tt_response spin_lock(...) tt_req_node gets removed from list and is freed spin_unlock(...)
CPU#0
<- returned to batadv_send_tt_request spin_lock(...) tt_req_node gets removed from list and is freed MEMORY CORRUPTION/SEGFAULT/... spin_unlock(...)
This can only be solved via reference counting to allow multiple contexts to handle the list manipulation while making sure that only the last context holding a reference will free the object.
Fixes: a73105b8d4c7 ("batman-adv: improved client announcement mechanism") Signed-off-by: Sven Eckelmann sven@narfation.org Tested-by: Martin Weinelt martin@darmstadt.freifunk.net Tested-by: Amadeus Alfa amadeus@chemnitz.freifunk.net Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: David S. Miller davem@davemloft.net --- net/batman-adv/translation-table.c | 43 +++++++++++++++++++++++++----- net/batman-adv/types.h | 2 ++ 2 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 4014e5130879..a30a77d1a220 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -2206,6 +2206,29 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv, return crc; }
+/** + * batadv_tt_req_node_release - free tt_req node entry + * @ref: kref pointer of the tt req_node entry + */ +static void batadv_tt_req_node_release(struct kref *ref) +{ + struct batadv_tt_req_node *tt_req_node; + + tt_req_node = container_of(ref, struct batadv_tt_req_node, refcount); + + kfree(tt_req_node); +} + +/** + * batadv_tt_req_node_put - decrement the tt_req_node refcounter and + * possibly release it + * @tt_req_node: tt_req_node to be free'd + */ +static void batadv_tt_req_node_put(struct batadv_tt_req_node *tt_req_node) +{ + kref_put(&tt_req_node->refcount, batadv_tt_req_node_release); +} + static void batadv_tt_req_list_free(struct batadv_priv *bat_priv) { struct batadv_tt_req_node *node; @@ -2215,7 +2238,7 @@ static void batadv_tt_req_list_free(struct batadv_priv *bat_priv)
hlist_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) { hlist_del_init(&node->list); - kfree(node); + batadv_tt_req_node_put(node); }
spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2252,7 +2275,7 @@ static void batadv_tt_req_purge(struct batadv_priv *bat_priv) if (batadv_has_timed_out(node->issued_at, BATADV_TT_REQUEST_TIMEOUT)) { hlist_del_init(&node->list); - kfree(node); + batadv_tt_req_node_put(node); } } spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2284,9 +2307,11 @@ batadv_tt_req_node_new(struct batadv_priv *bat_priv, if (!tt_req_node) goto unlock;
+ kref_init(&tt_req_node->refcount); ether_addr_copy(tt_req_node->addr, orig_node->orig); tt_req_node->issued_at = jiffies;
+ kref_get(&tt_req_node->refcount); hlist_add_head(&tt_req_node->list, &bat_priv->tt.req_list); unlock: spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2536,13 +2561,19 @@ static int batadv_send_tt_request(struct batadv_priv *bat_priv, out: if (primary_if) batadv_hardif_free_ref(primary_if); + if (ret && tt_req_node) { spin_lock_bh(&bat_priv->tt.req_list_lock); - /* hlist_del_init() verifies tt_req_node still is in the list */ - hlist_del_init(&tt_req_node->list); + if (!hlist_unhashed(&tt_req_node->list)) { + hlist_del_init(&tt_req_node->list); + batadv_tt_req_node_put(tt_req_node); + } spin_unlock_bh(&bat_priv->tt.req_list_lock); - kfree(tt_req_node); } + + if (tt_req_node) + batadv_tt_req_node_put(tt_req_node); + kfree(tvlv_tt_data); return ret; } @@ -2978,7 +3009,7 @@ static void batadv_handle_tt_response(struct batadv_priv *bat_priv, if (!batadv_compare_eth(node->addr, resp_src)) continue; hlist_del_init(&node->list); - kfree(node); + batadv_tt_req_node_put(node); }
spin_unlock_bh(&bat_priv->tt.req_list_lock); diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index dcf285fae2e6..3188907c2b4b 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1001,11 +1001,13 @@ struct batadv_tt_change_node { * struct batadv_tt_req_node - data to keep track of the tt requests in flight * @addr: mac address address of the originator this request was sent to * @issued_at: timestamp used for purging stale tt requests + * @refcount: number of contexts the object is used by * @list: list node for batadv_priv_tt::req_list */ struct batadv_tt_req_node { u8 addr[ETH_ALEN]; unsigned long issued_at; + struct kref refcount; struct hlist_node list; };
commit 3b55e4422087f9f7b241031d758a0c65584e4297 upstream.
The skb_linearize may reallocate the skb. This makes the calculated pointer for ethhdr invalid. But it the pointer is used later to fill in the RR field of the batadv_icmp_packet_rr packet.
Instead re-evaluate eth_hdr after the skb_linearize+skb_cow to fix the pointer and avoid the invalid read.
Fixes: da6b8c20a5b8 ("batman-adv: generalize batman-adv icmp packet handling") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: David S. Miller davem@davemloft.net --- net/batman-adv/routing.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index de24ab1cf67f..477899ba7616 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -359,6 +359,7 @@ int batadv_recv_icmp_packet(struct sk_buff *skb, if (skb_cow(skb, ETH_HLEN) < 0) goto out;
+ ethhdr = eth_hdr(skb); icmph = (struct batadv_icmp_header *)skb->data; icmp_packet_rr = (struct batadv_icmp_packet_rr *)icmph; if (icmp_packet_rr->rr_cur >= BATADV_RR_LEN)
commit 420cb1b764f9169c5d2601b4af90e4a1702345ee upstream.
The untagged vlan object is only destroyed when the interface is removed via the legacy sysfs interface. But it also has to be destroyed when the standard rtnl-link interface is used.
Fixes: 5d2c05b21337 ("batman-adv: add per VLAN interface attribute framework") Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Antonio Quartulli a@unstable.cc Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: David S. Miller davem@davemloft.net --- net/batman-adv/soft-interface.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index 4812123e0a2c..ff693887ea82 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -1000,7 +1000,9 @@ void batadv_softif_destroy_sysfs(struct net_device *soft_iface) static void batadv_softif_destroy_netlink(struct net_device *soft_iface, struct list_head *head) { + struct batadv_priv *bat_priv = netdev_priv(soft_iface); struct batadv_hard_iface *hard_iface; + struct batadv_softif_vlan *vlan;
list_for_each_entry(hard_iface, &batadv_hardif_list, list) { if (hard_iface->soft_iface == soft_iface) @@ -1008,6 +1010,13 @@ static void batadv_softif_destroy_netlink(struct net_device *soft_iface, BATADV_IF_CLEANUP_KEEP); }
+ /* destroy the "untagged" VLAN */ + vlan = batadv_softif_vlan_get(bat_priv, BATADV_NO_FLAGS); + if (vlan) { + batadv_softif_destroy_vlan(bat_priv, vlan); + batadv_softif_vlan_free_ref(vlan); + } + batadv_sysfs_del_meshif(soft_iface); unregister_netdevice_queue(soft_iface, head); }
commit 10c78f5854d361ded4736c1831948e0a5f67b932 upstream.
vlan_insert_tag can return NULL on errors. The bridge loop avoidance code therefore has to check the return value of vlan_insert_tag for NULL before it can safely operate on this pointer.
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bridge_loop_avoidance.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index c5208136e3fc..6749fe7e0321 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -352,9 +352,12 @@ static void batadv_bla_send_claim(struct batadv_priv *bat_priv, u8 *mac, break; }
- if (vid & BATADV_VLAN_HAS_TAG) + if (vid & BATADV_VLAN_HAS_TAG) { skb = vlan_insert_tag(skb, htons(ETH_P_8021Q), vid & VLAN_VID_MASK); + if (!skb) + goto out; + }
skb_reset_mac_header(skb); skb->protocol = eth_type_trans(skb, soft_iface);
commit 60154a1e0495ffb8343a95cefe1e874634572fa8 upstream.
vlan_insert_tag can return NULL on errors. The distributed arp table code therefore has to check the return value of vlan_insert_tag for NULL before it can safely operate on this pointer.
Fixes: be1db4f6615b ("batman-adv: make the Distributed ARP Table vlan aware") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/distributed-arp-table.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index 76808c5e8183..f890b13fad43 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -993,9 +993,12 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv, if (!skb_new) goto out;
- if (vid & BATADV_VLAN_HAS_TAG) + if (vid & BATADV_VLAN_HAS_TAG) { skb_new = vlan_insert_tag(skb_new, htons(ETH_P_8021Q), vid & VLAN_VID_MASK); + if (!skb_new) + goto out; + }
skb_reset_mac_header(skb_new); skb_new->protocol = eth_type_trans(skb_new, @@ -1073,9 +1076,12 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv, */ skb_reset_mac_header(skb_new);
- if (vid & BATADV_VLAN_HAS_TAG) + if (vid & BATADV_VLAN_HAS_TAG) { skb_new = vlan_insert_tag(skb_new, htons(ETH_P_8021Q), vid & VLAN_VID_MASK); + if (!skb_new) + goto out; + }
/* To preserve backwards compatibility, the node has choose the outgoing * format based on the incoming request packet type. The assumption is
commit 33fbb1f3db87ce53da925b3e034b4dd446d483f8 upstream.
batadv_orig_node_new uses batadv_orig_node_vlan_new to allocate a new batadv_orig_node_vlan and add it to batadv_orig_node::vlan_list. References to this list have also to be cleaned when the batadv_orig_node is removed.
Fixes: 7ea7b4a14275 ("batman-adv: make the TT CRC logic VLAN specific") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/originator.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index 71d0f6deb8aa..ce9743aa8a14 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -564,6 +564,7 @@ static void batadv_orig_node_release(struct batadv_orig_node *orig_node) struct hlist_node *node_tmp; struct batadv_neigh_node *neigh_node; struct batadv_orig_ifinfo *orig_ifinfo; + struct batadv_orig_node_vlan *vlan;
spin_lock_bh(&orig_node->neigh_list_lock);
@@ -581,6 +582,13 @@ static void batadv_orig_node_release(struct batadv_orig_node *orig_node) } spin_unlock_bh(&orig_node->neigh_list_lock);
+ spin_lock_bh(&orig_node->vlan_list_lock); + hlist_for_each_entry_safe(vlan, node_tmp, &orig_node->vlan_list, list) { + hlist_del_rcu(&vlan->list); + batadv_orig_node_vlan_free_ref(vlan); + } + spin_unlock_bh(&orig_node->vlan_list_lock); + /* Free nc_nodes */ batadv_nc_purge_orig(orig_node->bat_priv, orig_node, NULL);
From: Simon Wunderlich sw@simonwunderlich.de
commit 5a1dd8a4773d4c24e925cc6154826d555a85c370 upstream.
We have found some networks in which nodes were constantly requesting other nodes BLA claim tables to synchronize, just to ask for that again once completed. The reason was that the crc checksum of the asked nodes were out of sync due to missing locking and multiple writes to the same crc checksum when adding/removing entries. Therefore the asked nodes constantly reported the wrong crc, which caused repeating requests.
To avoid multiple functions changing a backbone gateways crc entry at the same time, lock it using a spinlock.
Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Tested-by: Alfons Name AlfonsName@web.de Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/bridge_loop_avoidance.c | 35 ++++++++++++++++++++++---- net/batman-adv/types.h | 2 ++ 2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index 6749fe7e0321..f7b30d8ec63b 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -256,7 +256,9 @@ batadv_bla_del_backbone_claims(struct batadv_bla_backbone_gw *backbone_gw) }
/* all claims gone, initialize CRC */ + spin_lock_bh(&backbone_gw->crc_lock); backbone_gw->crc = BATADV_BLA_CRC_INIT; + spin_unlock_bh(&backbone_gw->crc_lock); }
/** @@ -407,6 +409,7 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, u8 *orig, entry->lasttime = jiffies; entry->crc = BATADV_BLA_CRC_INIT; entry->bat_priv = bat_priv; + spin_lock_init(&entry->crc_lock); atomic_set(&entry->request_sent, 0); atomic_set(&entry->wait_periods, 0); ether_addr_copy(entry->orig, orig); @@ -556,7 +559,9 @@ static void batadv_bla_send_announce(struct batadv_priv *bat_priv, __be16 crc;
memcpy(mac, batadv_announce_mac, 4); + spin_lock_bh(&backbone_gw->crc_lock); crc = htons(backbone_gw->crc); + spin_unlock_bh(&backbone_gw->crc_lock); memcpy(&mac[4], &crc, 2);
batadv_bla_send_claim(bat_priv, mac, backbone_gw->vid, @@ -617,14 +622,18 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv, "bla_add_claim(): changing ownership for %pM, vid %d\n", mac, BATADV_PRINT_VID(vid));
+ spin_lock_bh(&claim->backbone_gw->crc_lock); claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); + spin_unlock_bh(&claim->backbone_gw->crc_lock); batadv_backbone_gw_free_ref(claim->backbone_gw); } /* set (new) backbone gw */ atomic_inc(&backbone_gw->refcount); claim->backbone_gw = backbone_gw;
+ spin_lock_bh(&backbone_gw->crc_lock); backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); + spin_unlock_bh(&backbone_gw->crc_lock); backbone_gw->lasttime = jiffies;
claim_free_ref: @@ -652,7 +661,9 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv, batadv_choose_claim, claim); batadv_claim_free_ref(claim); /* reference from the hash is gone */
+ spin_lock_bh(&claim->backbone_gw->crc_lock); claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); + spin_unlock_bh(&claim->backbone_gw->crc_lock);
/* don't need the reference from hash_find() anymore */ batadv_claim_free_ref(claim); @@ -663,7 +674,7 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv, u8 *an_addr, u8 *backbone_addr, unsigned short vid) { struct batadv_bla_backbone_gw *backbone_gw; - u16 crc; + u16 backbone_crc, crc;
if (memcmp(an_addr, batadv_announce_mac, 4) != 0) return 0; @@ -682,12 +693,16 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv, u8 *an_addr, "handle_announce(): ANNOUNCE vid %d (sent by %pM)... CRC = %#.4x\n", BATADV_PRINT_VID(vid), backbone_gw->orig, crc);
- if (backbone_gw->crc != crc) { + spin_lock_bh(&backbone_gw->crc_lock); + backbone_crc = backbone_gw->crc; + spin_unlock_bh(&backbone_gw->crc_lock); + + if (backbone_crc != crc) { batadv_dbg(BATADV_DBG_BLA, backbone_gw->bat_priv, "handle_announce(): CRC FAILED for %pM/%d (my = %#.4x, sent = %#.4x)\n", backbone_gw->orig, BATADV_PRINT_VID(backbone_gw->vid), - backbone_gw->crc, crc); + backbone_crc, crc);
batadv_bla_send_request(backbone_gw); } else { @@ -1658,6 +1673,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset) struct batadv_bla_claim *claim; struct batadv_hard_iface *primary_if; struct hlist_head *head; + u16 backbone_crc; u32 i; bool is_own; u8 *primary_addr; @@ -1680,11 +1696,15 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset) hlist_for_each_entry_rcu(claim, head, hash_entry) { is_own = batadv_compare_eth(claim->backbone_gw->orig, primary_addr); + + spin_lock_bh(&claim->backbone_gw->crc_lock); + backbone_crc = claim->backbone_gw->crc; + spin_unlock_bh(&claim->backbone_gw->crc_lock); seq_printf(seq, " * %pM on %5d by %pM [%c] (%#.4x)\n", claim->addr, BATADV_PRINT_VID(claim->vid), claim->backbone_gw->orig, (is_own ? 'x' : ' '), - claim->backbone_gw->crc); + backbone_crc); } rcu_read_unlock(); } @@ -1703,6 +1723,7 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset) struct batadv_hard_iface *primary_if; struct hlist_head *head; int secs, msecs; + u16 backbone_crc; u32 i; bool is_own; u8 *primary_addr; @@ -1733,10 +1754,14 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset) if (is_own) continue;
+ spin_lock_bh(&backbone_gw->crc_lock); + backbone_crc = backbone_gw->crc; + spin_unlock_bh(&backbone_gw->crc_lock); + seq_printf(seq, " * %pM on %5d %4i.%03is (%#.4x)\n", backbone_gw->orig, BATADV_PRINT_VID(backbone_gw->vid), secs, - msecs, backbone_gw->crc); + msecs, backbone_crc); } rcu_read_unlock(); } diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 3188907c2b4b..4764495b4071 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -884,6 +884,7 @@ struct batadv_socket_packet { * backbone gateway - no bcast traffic is formwared until the situation was * resolved * @crc: crc16 checksum over all claims + * @crc_lock: lock protecting crc * @refcount: number of contexts the object is used * @rcu: struct used for freeing in an RCU-safe manner */ @@ -897,6 +898,7 @@ struct batadv_bla_backbone_gw { atomic_t wait_periods; atomic_t request_sent; u16 crc; + spinlock_t crc_lock; /* protects crc */ atomic_t refcount; struct rcu_head rcu; };
commit 3db0decf1185357d6ab2256d0dede1ca9efda03d upstream.
The pointer batadv_bla_claim::backbone_gw can be changed at any time. Therefore, access to it must be protected to ensure that two function accessing the same backbone_gw are actually accessing the same. This is especially important when the crc_lock is used or when the backbone_gw of a claim is exchanged.
Not doing so leads to invalid memory access and/or reference leaks.
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code") Fixes: 5a1dd8a4773d ("batman-adv: lock crc access in bridge loop avoidance") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bridge_loop_avoidance.c | 112 ++++++++++++++++++++----- net/batman-adv/types.h | 1 + 2 files changed, 90 insertions(+), 23 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index f7b30d8ec63b..6e2a5d02ce1f 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -129,7 +129,19 @@ batadv_backbone_gw_free_ref(struct batadv_bla_backbone_gw *backbone_gw) /* finally deinitialize the claim */ static void batadv_claim_release(struct batadv_bla_claim *claim) { - batadv_backbone_gw_free_ref(claim->backbone_gw); + struct batadv_bla_backbone_gw *old_backbone_gw; + + spin_lock_bh(&claim->backbone_lock); + old_backbone_gw = claim->backbone_gw; + claim->backbone_gw = NULL; + spin_unlock_bh(&claim->backbone_lock); + + spin_lock_bh(&old_backbone_gw->crc_lock); + old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); + spin_unlock_bh(&old_backbone_gw->crc_lock); + + batadv_backbone_gw_free_ref(old_backbone_gw); + kfree_rcu(claim, rcu); }
@@ -579,8 +591,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv, const u8 *mac, const unsigned short vid, struct batadv_bla_backbone_gw *backbone_gw) { + struct batadv_bla_backbone_gw *old_backbone_gw; struct batadv_bla_claim *claim; struct batadv_bla_claim search_claim; + bool remove_crc = false; int hash_added;
ether_addr_copy(search_claim.addr, mac); @@ -594,8 +608,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv, return;
ether_addr_copy(claim->addr, mac); + spin_lock_init(&claim->backbone_lock); claim->vid = vid; claim->lasttime = jiffies; + atomic_inc(&backbone_gw->refcount); claim->backbone_gw = backbone_gw;
atomic_set(&claim->refcount, 2); @@ -622,15 +638,26 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv, "bla_add_claim(): changing ownership for %pM, vid %d\n", mac, BATADV_PRINT_VID(vid));
- spin_lock_bh(&claim->backbone_gw->crc_lock); - claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); - spin_unlock_bh(&claim->backbone_gw->crc_lock); - batadv_backbone_gw_free_ref(claim->backbone_gw); + remove_crc = true; } - /* set (new) backbone gw */ + + /* replace backbone_gw atomically and adjust reference counters */ + spin_lock_bh(&claim->backbone_lock); + old_backbone_gw = claim->backbone_gw; atomic_inc(&backbone_gw->refcount); claim->backbone_gw = backbone_gw; + spin_unlock_bh(&claim->backbone_lock);
+ if (remove_crc) { + /* remove claim address from old backbone_gw */ + spin_lock_bh(&old_backbone_gw->crc_lock); + old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); + spin_unlock_bh(&old_backbone_gw->crc_lock); + } + + batadv_backbone_gw_free_ref(old_backbone_gw); + + /* add claim address to new backbone_gw */ spin_lock_bh(&backbone_gw->crc_lock); backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); spin_unlock_bh(&backbone_gw->crc_lock); @@ -640,6 +667,26 @@ claim_free_ref: batadv_claim_free_ref(claim); }
+/** + * batadv_bla_claim_get_backbone_gw - Get valid reference for backbone_gw of + * claim + * @claim: claim whose backbone_gw should be returned + * + * Return: valid reference to claim::backbone_gw + */ +static struct batadv_bla_backbone_gw * +batadv_bla_claim_get_backbone_gw(struct batadv_bla_claim *claim) +{ + struct batadv_bla_backbone_gw *backbone_gw; + + spin_lock_bh(&claim->backbone_lock); + backbone_gw = claim->backbone_gw; + atomic_inc(&backbone_gw->refcount); + spin_unlock_bh(&claim->backbone_lock); + + return backbone_gw; +} + /* Delete a claim from the claim hash which has the * given mac address and vid. */ @@ -661,10 +708,6 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv, batadv_choose_claim, claim); batadv_claim_free_ref(claim); /* reference from the hash is gone */
- spin_lock_bh(&claim->backbone_gw->crc_lock); - claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); - spin_unlock_bh(&claim->backbone_gw->crc_lock); - /* don't need the reference from hash_find() anymore */ batadv_claim_free_ref(claim); } @@ -1074,6 +1117,7 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv, struct batadv_hard_iface *primary_if, int now) { + struct batadv_bla_backbone_gw *backbone_gw; struct batadv_bla_claim *claim; struct hlist_head *head; struct batadv_hashtable *hash; @@ -1088,14 +1132,17 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
rcu_read_lock(); hlist_for_each_entry_rcu(claim, head, hash_entry) { + backbone_gw = batadv_bla_claim_get_backbone_gw(claim); if (now) goto purge_now; - if (!batadv_compare_eth(claim->backbone_gw->orig, + + if (!batadv_compare_eth(backbone_gw->orig, primary_if->net_dev->dev_addr)) - continue; + goto skip; + if (!batadv_has_timed_out(claim->lasttime, BATADV_BLA_CLAIM_TIMEOUT)) - continue; + goto skip;
batadv_dbg(BATADV_DBG_BLA, bat_priv, "bla_purge_claims(): %pM, vid %d, time out\n", @@ -1103,8 +1150,10 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
purge_now: batadv_handle_unclaim(bat_priv, primary_if, - claim->backbone_gw->orig, + backbone_gw->orig, claim->addr, claim->vid); +skip: + batadv_backbone_gw_free_ref(backbone_gw); } rcu_read_unlock(); } @@ -1488,9 +1537,11 @@ void batadv_bla_free(struct batadv_priv *bat_priv) int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, unsigned short vid, bool is_bcast) { + struct batadv_bla_backbone_gw *backbone_gw; struct ethhdr *ethhdr; struct batadv_bla_claim search_claim, *claim = NULL; struct batadv_hard_iface *primary_if; + bool own_claim; int ret;
ethhdr = eth_hdr(skb); @@ -1522,8 +1573,12 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, }
/* if it is our own claim ... */ - if (batadv_compare_eth(claim->backbone_gw->orig, - primary_if->net_dev->dev_addr)) { + backbone_gw = batadv_bla_claim_get_backbone_gw(claim); + own_claim = batadv_compare_eth(backbone_gw->orig, + primary_if->net_dev->dev_addr); + batadv_backbone_gw_free_ref(backbone_gw); + + if (own_claim) { /* ... allow it in any case */ claim->lasttime = jiffies; goto allow; @@ -1586,7 +1641,9 @@ int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, { struct ethhdr *ethhdr; struct batadv_bla_claim search_claim, *claim = NULL; + struct batadv_bla_backbone_gw *backbone_gw; struct batadv_hard_iface *primary_if; + bool client_roamed; int ret = 0;
primary_if = batadv_primary_if_get_selected(bat_priv); @@ -1616,8 +1673,12 @@ int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, goto allow;
/* check if we are responsible. */ - if (batadv_compare_eth(claim->backbone_gw->orig, - primary_if->net_dev->dev_addr)) { + backbone_gw = batadv_bla_claim_get_backbone_gw(claim); + client_roamed = batadv_compare_eth(backbone_gw->orig, + primary_if->net_dev->dev_addr); + batadv_backbone_gw_free_ref(backbone_gw); + + if (client_roamed) { /* if yes, the client has roamed and we have * to unclaim it. */ @@ -1670,6 +1731,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset) struct net_device *net_dev = (struct net_device *)seq->private; struct batadv_priv *bat_priv = netdev_priv(net_dev); struct batadv_hashtable *hash = bat_priv->bla.claim_hash; + struct batadv_bla_backbone_gw *backbone_gw; struct batadv_bla_claim *claim; struct batadv_hard_iface *primary_if; struct hlist_head *head; @@ -1694,17 +1756,21 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
rcu_read_lock(); hlist_for_each_entry_rcu(claim, head, hash_entry) { - is_own = batadv_compare_eth(claim->backbone_gw->orig, + backbone_gw = batadv_bla_claim_get_backbone_gw(claim); + + is_own = batadv_compare_eth(backbone_gw->orig, primary_addr);
- spin_lock_bh(&claim->backbone_gw->crc_lock); - backbone_crc = claim->backbone_gw->crc; - spin_unlock_bh(&claim->backbone_gw->crc_lock); + spin_lock_bh(&backbone_gw->crc_lock); + backbone_crc = backbone_gw->crc; + spin_unlock_bh(&backbone_gw->crc_lock); seq_printf(seq, " * %pM on %5d by %pM [%c] (%#.4x)\n", claim->addr, BATADV_PRINT_VID(claim->vid), - claim->backbone_gw->orig, + backbone_gw->orig, (is_own ? 'x' : ' '), backbone_crc); + + batadv_backbone_gw_free_ref(backbone_gw); } rcu_read_unlock(); } diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 4764495b4071..d3ebc3fd7d06 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -917,6 +917,7 @@ struct batadv_bla_claim { u8 addr[ETH_ALEN]; unsigned short vid; struct batadv_bla_backbone_gw *backbone_gw; + spinlock_t backbone_lock; /* protects backbone_gw */ unsigned long lasttime; struct hlist_node hash_entry; struct rcu_head rcu;
commit 15c2ed753cd9e3e746472deab8151337a5b6da56 upstream.
The replacement of last_bonding_candidate in batadv_orig_node has to be an atomic operation. Otherwise it is possible that the reference counter of a batadv_orig_ifinfo is reduced which was no longer the last_bonding_candidate when the new candidate is added. This can either lead to an invalid memory access or to reference leaks which make it impossible to an interface which was added to batman-adv.
Fixes: f3b3d9018975 ("batman-adv: add bonding again") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/routing.c | 52 ++++++++++++++++++++++++++++++---------- net/batman-adv/types.h | 4 +++- 2 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index 477899ba7616..ff9aefddb6ef 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -439,6 +439,29 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv, return 0; }
+/** + * batadv_last_bonding_replace - Replace last_bonding_candidate of orig_node + * @orig_node: originator node whose bonding candidates should be replaced + * @new_candidate: new bonding candidate or NULL + */ +static void +batadv_last_bonding_replace(struct batadv_orig_node *orig_node, + struct batadv_orig_ifinfo *new_candidate) +{ + struct batadv_orig_ifinfo *old_candidate; + + spin_lock_bh(&orig_node->neigh_list_lock); + old_candidate = orig_node->last_bonding_candidate; + + if (new_candidate) + atomic_inc(&new_candidate->refcount); + orig_node->last_bonding_candidate = new_candidate; + spin_unlock_bh(&orig_node->neigh_list_lock); + + if (old_candidate) + batadv_orig_ifinfo_free_ref(old_candidate); +} + /** * batadv_find_router - find a suitable router for this originator * @bat_priv: the bat priv with all the soft interface information @@ -546,10 +569,6 @@ next: } rcu_read_unlock();
- /* last_bonding_candidate is reset below, remove the old reference. */ - if (orig_node->last_bonding_candidate) - batadv_orig_ifinfo_free_ref(orig_node->last_bonding_candidate); - /* After finding candidates, handle the three cases: * 1) there is a next candidate, use that * 2) there is no next candidate, use the first of the list @@ -558,21 +577,28 @@ next: if (next_candidate) { batadv_neigh_node_free_ref(router);
- /* remove references to first candidate, we don't need it. */ - if (first_candidate) { - batadv_neigh_node_free_ref(first_candidate_router); - batadv_orig_ifinfo_free_ref(first_candidate); - } + atomic_inc(&next_candidate_router->refcount); router = next_candidate_router; - orig_node->last_bonding_candidate = next_candidate; + batadv_last_bonding_replace(orig_node, next_candidate); } else if (first_candidate) { batadv_neigh_node_free_ref(router);
- /* refcounting has already been done in the loop above. */ + atomic_inc(&first_candidate_router->refcount); router = first_candidate_router; - orig_node->last_bonding_candidate = first_candidate; + batadv_last_bonding_replace(orig_node, first_candidate); } else { - orig_node->last_bonding_candidate = NULL; + batadv_last_bonding_replace(orig_node, NULL); + } + + /* cleanup of candidates */ + if (first_candidate) { + batadv_neigh_node_free_ref(first_candidate_router); + batadv_orig_ifinfo_free_ref(first_candidate); + } + + if (next_candidate) { + batadv_neigh_node_free_ref(next_candidate_router); + batadv_orig_ifinfo_free_ref(next_candidate); }
return router; diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index d3ebc3fd7d06..375c27a68cbc 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -287,7 +287,9 @@ struct batadv_orig_node { DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE); u32 last_bcast_seqno; struct hlist_head neigh_list; - /* neigh_list_lock protects: neigh_list and router */ + /* neigh_list_lock protects: neigh_list, ifinfo_list, + * last_bonding_candidate and router + */ spinlock_t neigh_list_lock; struct hlist_node hash_entry; struct batadv_priv *bat_priv;
commit cbef1e102003edb236c6b2319ab269ccef963731 upstream.
The orig_ifinfo reference counter for last_bonding_candidate in batadv_orig_node has to be reduced when an originator node is released. Otherwise the orig_ifinfo is leaked and the reference counter the netdevice is not reduced correctly.
Fixes: f3b3d9018975 ("batman-adv: add bonding again") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/originator.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index ce9743aa8a14..ce5b991707d0 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -565,6 +565,7 @@ static void batadv_orig_node_release(struct batadv_orig_node *orig_node) struct batadv_neigh_node *neigh_node; struct batadv_orig_ifinfo *orig_ifinfo; struct batadv_orig_node_vlan *vlan; + struct batadv_orig_ifinfo *last_candidate;
spin_lock_bh(&orig_node->neigh_list_lock);
@@ -580,8 +581,14 @@ static void batadv_orig_node_release(struct batadv_orig_node *orig_node) hlist_del_rcu(&orig_ifinfo->list); batadv_orig_ifinfo_free_ref(orig_ifinfo); } + + last_candidate = orig_node->last_bonding_candidate; + orig_node->last_bonding_candidate = NULL; spin_unlock_bh(&orig_node->neigh_list_lock);
+ if (last_candidate) + batadv_orig_ifinfo_free_ref(last_candidate); + spin_lock_bh(&orig_node->vlan_list_lock); hlist_for_each_entry_safe(vlan, node_tmp, &orig_node->vlan_list, list) { hlist_del_rcu(&vlan->list);
commit d1fe176ca51fa3cb35f70c1d876d9a090e9befce upstream.
Speedy join only works when the received packet is either broadcast or an 4addr unicast packet. Thus packets converted from broadcast to unicast via the gateway handling code have to be converted to 4addr packets to allow the receiving gateway server to add the sender address as temporary entry to the translation table.
Not doing it will make the batman-adv gateway server drop the DHCP response in many situations because it doesn't yet have the TT entry for the destination of the DHCP response.
Fixes: 371351731e9c ("batman-adv: change interface_rx to get orig node") Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Antonio Quartulli a@unstable.cc Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/send.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c index 0e0c3b8ed927..11fbfb222c49 100644 --- a/net/batman-adv/send.c +++ b/net/batman-adv/send.c @@ -381,8 +381,8 @@ int batadv_send_skb_via_gw(struct batadv_priv *bat_priv, struct sk_buff *skb, struct batadv_orig_node *orig_node;
orig_node = batadv_gw_get_selected_orig(bat_priv); - return batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST, 0, - orig_node, vid); + return batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST_4ADDR, + BATADV_P_DATA, orig_node, vid); }
void batadv_schedule_bat_ogm(struct batadv_hard_iface *hard_iface)
commit 936523441bb64cdc9a5b263e8fd2782e70313a57 upstream.
batadv_find_router dereferences last_bonding_candidate from orig_node without making sure that it has a valid reference. This reference has to be retrieved by increasing the reference counter while holding neigh_list_lock. The lock is required to avoid that batadv_last_bonding_replace removes the current last_bonding_candidate, reduces the reference counter and maybe destroys the object in this process.
Fixes: f3b3d9018975 ("batman-adv: add bonding again") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/routing.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index ff9aefddb6ef..39b4c6ab3e71 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -439,6 +439,29 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv, return 0; }
+/** + * batadv_last_bonding_get - Get last_bonding_candidate of orig_node + * @orig_node: originator node whose last bonding candidate should be retrieved + * + * Return: last bonding candidate of router or NULL if not found + * + * The object is returned with refcounter increased by 1. + */ +static struct batadv_orig_ifinfo * +batadv_last_bonding_get(struct batadv_orig_node *orig_node) +{ + struct batadv_orig_ifinfo *last_bonding_candidate; + + spin_lock_bh(&orig_node->neigh_list_lock); + last_bonding_candidate = orig_node->last_bonding_candidate; + + if (last_bonding_candidate) + atomic_inc(&last_bonding_candidate->refcount); + spin_unlock_bh(&orig_node->neigh_list_lock); + + return last_bonding_candidate; +} + /** * batadv_last_bonding_replace - Replace last_bonding_candidate of orig_node * @orig_node: originator node whose bonding candidates should be replaced @@ -509,7 +532,7 @@ batadv_find_router(struct batadv_priv *bat_priv, * router - obviously there are no other candidates. */ rcu_read_lock(); - last_candidate = orig_node->last_bonding_candidate; + last_candidate = batadv_last_bonding_get(orig_node); if (last_candidate) last_cand_router = rcu_dereference(last_candidate->router);
@@ -601,6 +624,9 @@ next: batadv_orig_ifinfo_free_ref(next_candidate); }
+ if (last_candidate) + batadv_orig_ifinfo_free_ref(last_candidate); + return router; }
commit 248e23b50e2da0753f3b5faa068939cbe9f8a75a upstream.
The function batadv_frag_skb_buffer was supposed not to consume the skbuff on errors. This was followed in the helper function batadv_frag_insert_packet when the skb would potentially be inserted in the fragment queue. But it could happen that the next helper function batadv_frag_merge_packets would try to merge the fragments and fail. This results in a kfree_skb of all the enqueued fragments (including the just inserted one). batadv_recv_frag_packet would detect the error in batadv_frag_skb_buffer and try to free the skb again.
The behavior of batadv_frag_skb_buffer (and its helper batadv_frag_insert_packet) must therefore be changed to always consume the skbuff to have a common behavior and avoid the double kfree_skb.
Fixes: 610bfc6bc99b ("batman-adv: Receive fragmented packets and merge") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/fragmentation.c | 6 ++++-- net/batman-adv/routing.c | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index d50c3b003dc9..8605e3c7958d 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -233,8 +233,10 @@ err_unlock: spin_unlock_bh(&chain->lock);
err: - if (!ret) + if (!ret) { kfree(frag_entry_new); + kfree_skb(skb); + }
return ret; } @@ -329,9 +331,9 @@ bool batadv_frag_skb_buffer(struct sk_buff **skb, goto out_err;
out: - *skb = skb_out; ret = true; out_err: + *skb = skb_out; return ret; }
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index 39b4c6ab3e71..6c3dfd65023f 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -1053,6 +1053,12 @@ int batadv_recv_frag_packet(struct sk_buff *skb, batadv_inc_counter(bat_priv, BATADV_CNT_FRAG_RX); batadv_add_counter(bat_priv, BATADV_CNT_FRAG_RX_BYTES, skb->len);
+ /* batadv_frag_skb_buffer will always consume the skb and + * the caller should therefore never try to free the + * skb after this point + */ + ret = NET_RX_SUCCESS; + /* Add fragment to buffer and merge if possible. */ if (!batadv_frag_skb_buffer(&skb, orig_node_src)) goto out;
From: Linus Lüssing linus.luessing@c0d3.blue
commit 51c6b429c0c95e67edd1cb0b548c5cf6a6604763 upstream.
Trying to split and transmit a unicast packet in 16 parts will fail for the final fragment: After having sent the 15th one with a frag_packet.no index of 14, we will increase the the index to 15 - and return with an error code immediately, even though one more fragment is due for transmission and allowed.
Fixing this issue by moving the check before incrementing the index.
While at it, adding an unlikely(), because the check is actually more of an assertion.
Fixes: ee75ed88879a ("batman-adv: Fragment and send skbs larger than mtu") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/fragmentation.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 8605e3c7958d..9751b207b01f 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -480,6 +480,10 @@ bool batadv_frag_send_packet(struct sk_buff *skb,
/* Eat and send fragments from the tail of skb */ while (skb->len > max_fragment_size) { + /* The initial check in this function should cover this case */ + if (frag_header.no == BATADV_FRAG_MAX_FRAGMENTS - 1) + goto out_err; + skb_fragment = batadv_frag_create(skb, &frag_header, mtu); if (!skb_fragment) goto out_err; @@ -490,10 +494,6 @@ bool batadv_frag_send_packet(struct sk_buff *skb, batadv_send_skb_packet(skb_fragment, neigh_node->if_incoming, neigh_node->addr); frag_header.no++; - - /* The initial check in this function should cover this case */ - if (frag_header.no == BATADV_FRAG_MAX_FRAGMENTS - 1) - goto out_err; }
/* Make room for the fragment header. */
commit 36d4d68cd658d914ef73ac845705c4a89e7d9e2f upstream.
The stats are generated by batadv_interface_stats and must not be stored directly in the net_device stats member variable. The batadv_priv bat_counters information is assembled when ndo_get_stats is called. The stats previously stored in net_device::stats is then overwritten.
The batman-adv counters must therefore be increased when an ARP packet is answered locally via the distributed arp table.
Fixes: c384ea3ec930 ("batman-adv: Distributed ARP Table - add snooping functions for ARP messages") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/distributed-arp-table.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index f890b13fad43..769683da8d9d 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -1003,8 +1003,9 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv, skb_reset_mac_header(skb_new); skb_new->protocol = eth_type_trans(skb_new, bat_priv->soft_iface); - bat_priv->stats.rx_packets++; - bat_priv->stats.rx_bytes += skb->len + ETH_HLEN + hdr_size; + batadv_inc_counter(bat_priv, BATADV_CNT_RX); + batadv_add_counter(bat_priv, BATADV_CNT_RX_BYTES, + skb->len + ETH_HLEN + hdr_size); bat_priv->soft_iface->last_rx = jiffies;
netif_rx(skb_new);
From: Linus Lüssing linus.luessing@c0d3.blue
commit 54e22f265e872ae140755b3318521d400a094605 upstream.
This patch fixes an issue in the translation table code potentially leading to a TT Request + Response storm. The issue may occur for nodes involving BLA and an inconsistent configuration of the batman-adv AP isolation feature. However, since the new multicast optimizations, a single, malformed packet may lead to a mesh-wide, persistent Denial-of-Service, too.
The issue occurs because nodes are currently OR-ing the TT sync flags of all originators announcing a specific MAC address via the translation table. When an intermediate node now receives a TT Request and wants to answer this on behalf of the destination node, then this intermediate node now responds with an altered flag field and broken CRC. The next OGM of the real destination will lead to a CRC mismatch and triggering a TT Request and Response again.
Furthermore, the OR-ing is currently never undone as long as at least one originator announcing the according MAC address remains, leading to the potential persistency of this issue.
This patch fixes this issue by storing the flags used in the CRC calculation on a a per TT orig entry basis to be able to respond with the correct, original flags in an intermediate TT Response for one thing. And to be able to correctly unset sync flags once all nodes announcing a sync flag vanish for another.
Fixes: e9c00136a475 ("batman-adv: fix tt_global_entries flags update") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Acked-by: Antonio Quartulli a@unstable.cc [sw: typo in commit message] Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 55 ++++++++++++++++++++++++++---- net/batman-adv/types.h | 2 ++ 2 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index a30a77d1a220..43f6d546a67e 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1263,9 +1263,41 @@ batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry, return found; }
+/** + * batadv_tt_global_sync_flags - update TT sync flags + * @tt_global: the TT global entry to update sync flags in + * + * Updates the sync flag bits in the tt_global flag attribute with a logical + * OR of all sync flags from any of its TT orig entries. + */ +static void +batadv_tt_global_sync_flags(struct batadv_tt_global_entry *tt_global) +{ + struct batadv_tt_orig_list_entry *orig_entry; + const struct hlist_head *head; + u16 flags = BATADV_NO_FLAGS; + + rcu_read_lock(); + head = &tt_global->orig_list; + hlist_for_each_entry_rcu(orig_entry, head, list) + flags |= orig_entry->flags; + rcu_read_unlock(); + + flags |= tt_global->common.flags & (~BATADV_TT_SYNC_MASK); + tt_global->common.flags = flags; +} + +/** + * batadv_tt_global_orig_entry_add - add or update a TT orig entry + * @tt_global: the TT global entry to add an orig entry in + * @orig_node: the originator to add an orig entry for + * @ttvn: translation table version number of this changeset + * @flags: TT sync flags + */ static void batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, - struct batadv_orig_node *orig_node, int ttvn) + struct batadv_orig_node *orig_node, int ttvn, + u8 flags) { struct batadv_tt_orig_list_entry *orig_entry;
@@ -1275,7 +1307,8 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, * was added during a "temporary client detection" */ orig_entry->ttvn = ttvn; - goto out; + orig_entry->flags = flags; + goto sync_flags; }
orig_entry = kzalloc(sizeof(*orig_entry), GFP_ATOMIC); @@ -1287,6 +1320,7 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, batadv_tt_global_size_inc(orig_node, tt_global->common.vid); orig_entry->orig_node = orig_node; orig_entry->ttvn = ttvn; + orig_entry->flags = flags; atomic_set(&orig_entry->refcount, 2);
spin_lock_bh(&tt_global->list_lock); @@ -1295,6 +1329,8 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, spin_unlock_bh(&tt_global->list_lock); atomic_inc(&tt_global->orig_list_count);
+sync_flags: + batadv_tt_global_sync_flags(tt_global); out: if (orig_entry) batadv_tt_orig_list_entry_free_ref(orig_entry); @@ -1417,7 +1453,7 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv, * TT_CLIENT_WIFI, therefore they have to be copied in the * client entry */ - tt_global_entry->common.flags |= flags; + tt_global_entry->common.flags |= flags & (~BATADV_TT_SYNC_MASK);
/* If there is the BATADV_TT_CLIENT_ROAM flag set, there is only * one originator left in the list and we previously received a @@ -1434,7 +1470,8 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv, } add_orig_entry: /* add the new orig_entry (if needed) or update it */ - batadv_tt_global_orig_entry_add(tt_global_entry, orig_node, ttvn); + batadv_tt_global_orig_entry_add(tt_global_entry, orig_node, ttvn, + flags & BATADV_TT_SYNC_MASK);
batadv_dbg(BATADV_DBG_TT, bat_priv, "Creating new global tt entry: %pM (vid: %d, via %pM)\n", @@ -2087,6 +2124,7 @@ static u32 batadv_tt_global_crc(struct batadv_priv *bat_priv, unsigned short vid) { struct batadv_hashtable *hash = bat_priv->tt.global_hash; + struct batadv_tt_orig_list_entry *tt_orig; struct batadv_tt_common_entry *tt_common; struct batadv_tt_global_entry *tt_global; struct hlist_head *head; @@ -2125,8 +2163,9 @@ static u32 batadv_tt_global_crc(struct batadv_priv *bat_priv, /* find out if this global entry is announced by this * originator */ - if (!batadv_tt_global_entry_has_orig(tt_global, - orig_node)) + tt_orig = batadv_tt_global_orig_entry_find(tt_global, + orig_node); + if (!tt_orig) continue;
/* use network order to read the VID: this ensures that @@ -2138,10 +2177,12 @@ static u32 batadv_tt_global_crc(struct batadv_priv *bat_priv, /* compute the CRC on flags that have to be kept in sync * among nodes */ - flags = tt_common->flags & BATADV_TT_SYNC_MASK; + flags = tt_orig->flags; crc_tmp = crc32c(crc_tmp, &flags, sizeof(flags));
crc ^= crc32c(crc_tmp, tt_common->addr, ETH_ALEN); + + batadv_tt_orig_list_entry_free_ref(tt_orig); } rcu_read_unlock(); } diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 375c27a68cbc..630777a93cdd 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -980,6 +980,7 @@ struct batadv_tt_global_entry { * struct batadv_tt_orig_list_entry - orig node announcing a non-mesh client * @orig_node: pointer to orig node announcing this non-mesh client * @ttvn: translation table version number which added the non-mesh client + * @flags: per orig entry TT sync flags * @list: list node for batadv_tt_global_entry::orig_list * @refcount: number of contexts the object is used * @rcu: struct used for freeing in an RCU-safe manner @@ -987,6 +988,7 @@ struct batadv_tt_global_entry { struct batadv_tt_orig_list_entry { struct batadv_orig_node *orig_node; u8 ttvn; + u8 flags; struct hlist_node list; atomic_t refcount; struct rcu_head rcu;
commit 5ba7dcfe77037b67016263ea597a8b431692ecab upstream.
The originator node object orig_neigh_node is used to when accessing the bcast_own(_sum) and real_packet_count information. The access to them has to be protected with the spinlock in orig_neigh_node.
But the function uses the lock in orig_node instead. This is incorrect because they could be two different originator node objects.
Fixes: 0ede9f41b217 ("batman-adv: protect bit operations to count OGMs with spinlock") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bat_iv_ogm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 749b9157fe11..910889dd5356 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -1180,7 +1180,7 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, orig_node->last_seen = jiffies;
/* find packet count of corresponding one hop neighbor */ - spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock); + spin_lock_bh(&orig_neigh_node->bat_iv.ogm_cnt_lock); if_num = if_incoming->if_num; orig_eq_count = orig_neigh_node->bat_iv.bcast_own_sum[if_num]; neigh_ifinfo = batadv_neigh_ifinfo_new(neigh_node, if_outgoing); @@ -1190,7 +1190,7 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, } else { neigh_rq_count = 0; } - spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock); + spin_unlock_bh(&orig_neigh_node->bat_iv.ogm_cnt_lock);
/* pay attention to not get a value bigger than 100 % */ if (orig_eq_count > neigh_rq_count)
commit f22e08932c2960f29b5e828e745c9f3fb7c1bb86 upstream.
batman-adv uses internal indices for each enabled and active interface. It is currently used by the B.A.T.M.A.N. IV algorithm to identifify the correct position in the ogm_cnt bitmaps.
The type for the number of enabled interfaces (which defines the next interface index) was set to char. This type can be (depending on the architecture) either signed (limiting batman-adv to 127 active slave interfaces) or unsigned (limiting batman-adv to 255 active slave interfaces).
This limit was not correctly checked when an interface was enabled and thus an overflow happened. This was only catched on systems with the signed char type when the B.A.T.M.A.N. IV code tried to resize its counter arrays with a negative size.
The if_num interface index was only a s16 and therefore significantly smaller than the ifindex (int) used by the code net code.
Both &batadv_hard_iface->if_num and &batadv_priv->num_ifaces must be (unsigned) int to support the same number of slave interfaces as the net core code. And the interface activation code must check the number of active slave interfaces to avoid integer overflows.
Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bat_iv_ogm.c | 16 +++++++++------- net/batman-adv/hard-interface.c | 9 +++++++-- net/batman-adv/originator.c | 4 ++-- net/batman-adv/originator.h | 4 ++-- net/batman-adv/types.h | 8 ++++---- 5 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 910889dd5356..5010b81e746f 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -135,7 +135,7 @@ static void batadv_iv_ogm_orig_free(struct batadv_orig_node *orig_node) * Returns 0 on success, a negative error code otherwise. */ static int batadv_iv_ogm_orig_add_if(struct batadv_orig_node *orig_node, - int max_if_num) + unsigned int max_if_num) { void *data_ptr; size_t old_size; @@ -181,7 +181,8 @@ unlock: * Returns 0 on success, a negative error code otherwise. */ static int batadv_iv_ogm_orig_del_if(struct batadv_orig_node *orig_node, - int max_if_num, int del_if_num) + unsigned int max_if_num, + unsigned int del_if_num) { int ret = -ENOMEM; size_t chunk_size, if_offset; @@ -252,7 +253,8 @@ static struct batadv_orig_node * batadv_iv_ogm_orig_get(struct batadv_priv *bat_priv, const u8 *addr) { struct batadv_orig_node *orig_node; - int size, hash_added; + int hash_added; + size_t size;
orig_node = batadv_orig_hash_find(bat_priv, addr); if (orig_node) @@ -871,7 +873,7 @@ batadv_iv_ogm_slide_own_bcast_window(struct batadv_hard_iface *hard_iface) u32 i; size_t word_index; u8 *w; - int if_num; + unsigned int if_num;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -982,7 +984,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, struct batadv_neigh_node *tmp_neigh_node = NULL; struct batadv_neigh_node *router = NULL; struct batadv_orig_node *orig_node_tmp; - int if_num; + unsigned int if_num; u8 sum_orig, sum_neigh; u8 *neigh_addr; u8 tq_avg; @@ -1647,9 +1649,9 @@ static void batadv_iv_ogm_process(const struct sk_buff *skb, int ogm_offset,
if (is_my_orig) { unsigned long *word; - int offset; + size_t offset; s32 bit_pos; - s16 if_num; + unsigned int if_num; u8 *weight;
orig_neigh_node = batadv_iv_ogm_orig_get(bat_priv, diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index cc3be5489f5c..7c297cef20a7 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -493,6 +493,11 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface, hard_iface->soft_iface = soft_iface; bat_priv = netdev_priv(hard_iface->soft_iface);
+ if (bat_priv->num_ifaces >= UINT_MAX) { + ret = -ENOSPC; + goto err_dev; + } + ret = netdev_master_upper_dev_link(hard_iface->net_dev, soft_iface); if (ret) goto err_dev; @@ -600,7 +605,7 @@ void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface, batadv_hardif_recalc_extra_skbroom(hard_iface->soft_iface);
/* nobody uses this interface anymore */ - if (!bat_priv->num_ifaces) { + if (bat_priv->num_ifaces == 0) { batadv_gw_check_client_stop(bat_priv);
if (autodel == BATADV_IF_CLEANUP_AUTO) @@ -656,7 +661,7 @@ batadv_hardif_add_interface(struct net_device *net_dev) if (ret) goto free_if;
- hard_iface->if_num = -1; + hard_iface->if_num = 0; hard_iface->net_dev = net_dev; hard_iface->soft_iface = NULL; hard_iface->if_status = BATADV_IF_NOT_IN_USE; diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index ce5b991707d0..b3013fbc417e 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -1103,7 +1103,7 @@ out: }
int batadv_orig_hash_add_if(struct batadv_hard_iface *hard_iface, - int max_if_num) + unsigned int max_if_num) { struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface); struct batadv_algo_ops *bao = bat_priv->bat_algo_ops; @@ -1139,7 +1139,7 @@ err: }
int batadv_orig_hash_del_if(struct batadv_hard_iface *hard_iface, - int max_if_num) + unsigned int max_if_num) { struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface); struct batadv_hashtable *hash = bat_priv->orig_hash; diff --git a/net/batman-adv/originator.h b/net/batman-adv/originator.h index a5c37882b409..65824d892a6a 100644 --- a/net/batman-adv/originator.h +++ b/net/batman-adv/originator.h @@ -67,9 +67,9 @@ void batadv_orig_ifinfo_free_ref(struct batadv_orig_ifinfo *orig_ifinfo); int batadv_orig_seq_print_text(struct seq_file *seq, void *offset); int batadv_orig_hardif_seq_print_text(struct seq_file *seq, void *offset); int batadv_orig_hash_add_if(struct batadv_hard_iface *hard_iface, - int max_if_num); + unsigned int max_if_num); int batadv_orig_hash_del_if(struct batadv_hard_iface *hard_iface, - int max_if_num); + unsigned int max_if_num); struct batadv_orig_node_vlan * batadv_orig_node_vlan_new(struct batadv_orig_node *orig_node, unsigned short vid); diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 630777a93cdd..6dcee1991c54 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -103,7 +103,7 @@ struct batadv_hard_iface_bat_iv { */ struct batadv_hard_iface { struct list_head list; - s16 if_num; + unsigned int if_num; char if_status; struct net_device *net_dev; u8 num_bcasts; @@ -808,7 +808,7 @@ struct batadv_priv { atomic_t bcast_seqno; atomic_t bcast_queue_left; atomic_t batman_queue_left; - char num_ifaces; + unsigned int num_ifaces; struct kobject *mesh_obj; struct dentry *debug_dir; struct hlist_head forw_bat_list; @@ -1179,9 +1179,9 @@ struct batadv_algo_ops { struct batadv_hard_iface *hard_iface); void (*bat_orig_free)(struct batadv_orig_node *orig_node); int (*bat_orig_add_if)(struct batadv_orig_node *orig_node, - int max_if_num); + unsigned int max_if_num); int (*bat_orig_del_if)(struct batadv_orig_node *orig_node, - int max_if_num, int del_if_num); + unsigned int max_if_num, unsigned int del_if_num); };
/**
From: Matthias Schiffer mschiffer@universe-factory.net
commit bc44b78157f621ff2a2618fe287a827bcb094ac4 upstream.
batadv_check_unicast_ttvn() calls skb_cow(), so pointers into the SKB data must be (re)set after calling it. The ethhdr variable is dropped altogether.
Fixes: 78fc6bbe0aca ("batman-adv: add UNICAST_4ADDR packet type") Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/routing.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index 6c3dfd65023f..b8ff2a17b7ef 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -904,7 +904,6 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, bool is4addr;
unicast_packet = (struct batadv_unicast_packet *)skb->data; - unicast_4addr_packet = (struct batadv_unicast_4addr_packet *)skb->data;
is4addr = unicast_packet->packet_type == BATADV_UNICAST_4ADDR; /* the caller function should have already pulled 2 bytes */ @@ -925,9 +924,13 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, if (!batadv_check_unicast_ttvn(bat_priv, skb, hdr_size)) return NET_RX_DROP;
+ unicast_packet = (struct batadv_unicast_packet *)skb->data; + /* packet for me */ if (batadv_is_my_mac(bat_priv, unicast_packet->dest)) { if (is4addr) { + unicast_4addr_packet = + (struct batadv_unicast_4addr_packet *)skb->data; subtype = unicast_4addr_packet->subtype; batadv_dat_inc_counter(bat_priv, subtype);
commit fc04fdb2c8a894283259f5621d31d75610701091 upstream.
batadv_check_unicast_ttvn may redirect a packet to itself or another originator. This involves rewriting the ttvn and the destination address in the batadv unicast header. These field were not yet pulled (with skb rcsum update) and thus any change to them also requires a change in the receive checksum.
Reported-by: Matthias Schiffer mschiffer@universe-factory.net Fixes: a73105b8d4c7 ("batman-adv: improved client announcement mechanism") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/routing.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index b8ff2a17b7ef..b3e8b0e3073c 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -704,6 +704,7 @@ out: /** * batadv_reroute_unicast_packet - update the unicast header for re-routing * @bat_priv: the bat priv with all the soft interface information + * @skb: unicast packet to process * @unicast_packet: the unicast header to be updated * @dst_addr: the payload destination * @vid: VLAN identifier @@ -715,7 +716,7 @@ out: * Returns true if the packet header has been updated, false otherwise */ static bool -batadv_reroute_unicast_packet(struct batadv_priv *bat_priv, +batadv_reroute_unicast_packet(struct batadv_priv *bat_priv, struct sk_buff *skb, struct batadv_unicast_packet *unicast_packet, u8 *dst_addr, unsigned short vid) { @@ -744,8 +745,10 @@ batadv_reroute_unicast_packet(struct batadv_priv *bat_priv, }
/* update the packet header */ + skb_postpull_rcsum(skb, unicast_packet, sizeof(*unicast_packet)); ether_addr_copy(unicast_packet->dest, orig_addr); unicast_packet->ttvn = orig_ttvn; + skb_postpush_rcsum(skb, unicast_packet, sizeof(*unicast_packet));
ret = true; out: @@ -785,7 +788,7 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv, * the packet to */ if (batadv_tt_local_client_is_roaming(bat_priv, ethhdr->h_dest, vid)) { - if (batadv_reroute_unicast_packet(bat_priv, unicast_packet, + if (batadv_reroute_unicast_packet(bat_priv, skb, unicast_packet, ethhdr->h_dest, vid)) batadv_dbg_ratelimited(BATADV_DBG_TT, bat_priv, @@ -831,7 +834,7 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv, * destination can possibly be updated and forwarded towards the new * target host */ - if (batadv_reroute_unicast_packet(bat_priv, unicast_packet, + if (batadv_reroute_unicast_packet(bat_priv, skb, unicast_packet, ethhdr->h_dest, vid)) { batadv_dbg_ratelimited(BATADV_DBG_TT, bat_priv, "Rerouting unicast packet to %pM (dst=%pM): TTVN mismatch old_ttvn=%u new_ttvn=%u\n", @@ -854,12 +857,14 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv, if (!primary_if) return 0;
+ /* update the packet header */ + skb_postpull_rcsum(skb, unicast_packet, sizeof(*unicast_packet)); ether_addr_copy(unicast_packet->dest, primary_if->net_dev->dev_addr); + unicast_packet->ttvn = curr_ttvn; + skb_postpush_rcsum(skb, unicast_packet, sizeof(*unicast_packet));
batadv_hardif_free_ref(primary_if);
- unicast_packet->ttvn = curr_ttvn; - return 1; }
commit 8ba0f9bd3bdea1058c2b2676bec7905724418e40 upstream.
The functions batadv_tt_prepare_tvlv_local_data and batadv_tt_prepare_tvlv_global_data are responsible for preparing a buffer which can be used to store the TVLV container for TT and add the VLAN information to it.
This will be done in three phases:
1. count the number of VLANs and their entries 2. allocate the buffer using the counters from the previous step and limits from the caller (parameter tt_len) 3. insert the VLAN information to the buffer
The step 1 and 3 operate on a list which contains the VLANs. The access to these lists must be protected with an appropriate lock or otherwise they might operate on on different entries. This could for example happen when another context is adding VLAN entries to this list.
This could lead to a buffer overflow in these functions when enough entries were added between step 1 and 3 to the VLAN lists that the buffer room for the entries (*tt_change) is smaller then the now required extra buffer for new VLAN entries.
Fixes: 7ea7b4a14275 ("batman-adv: make the TT CRC logic VLAN specific") Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Antonio Quartulli a@unstable.cc Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 43f6d546a67e..57edbf6b26dd 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -744,7 +744,7 @@ batadv_tt_prepare_tvlv_global_data(struct batadv_orig_node *orig_node, struct batadv_orig_node_vlan *vlan; u8 *tt_change_ptr;
- rcu_read_lock(); + spin_lock_bh(&orig_node->vlan_list_lock); hlist_for_each_entry_rcu(vlan, &orig_node->vlan_list, list) { num_vlan++; num_entries += atomic_read(&vlan->tt.num_entries); @@ -782,7 +782,7 @@ batadv_tt_prepare_tvlv_global_data(struct batadv_orig_node *orig_node, *tt_change = (struct batadv_tvlv_tt_change *)tt_change_ptr;
out: - rcu_read_unlock(); + spin_unlock_bh(&orig_node->vlan_list_lock); return tvlv_len; }
@@ -818,7 +818,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv, u8 *tt_change_ptr; int change_offset;
- rcu_read_lock(); + spin_lock_bh(&bat_priv->softif_vlan_list_lock); hlist_for_each_entry_rcu(vlan, &bat_priv->softif_vlan_list, list) { num_vlan++; num_entries += atomic_read(&vlan->tt.num_entries); @@ -856,7 +856,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv, *tt_change = (struct batadv_tvlv_tt_change *)tt_change_ptr;
out: - rcu_read_unlock(); + spin_unlock_bh(&bat_priv->softif_vlan_list_lock); return tvlv_len; }
From: Linus Lüssing linus.luessing@c0d3.blue
commit 7072337e52b3e9d5460500d8dc9cbc1ba2db084c upstream.
The previous TT sync fix so far only fixed TT responses issued by the target node directly. So far, TT responses issued by intermediate nodes still lead to the wrong flags being added, leading to CRC mismatches.
This behaviour was observed at Freifunk Hannover in a 800 nodes setup where a considerable amount of nodes were still infected with 'WI' TT flags even with (most) nodes having the previous TT sync fix applied.
I was able to reproduce the issue with intermediate TT responses in a four node test setup and this patch fixes this issue by ensuring to use the per originator instead of the summarized, OR'd ones.
Fixes: e9c00136a475 ("batman-adv: fix tt_global_entries flags update") Reported-by: Leonardo Mörlein me@irrelefant.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 37 +++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 57edbf6b26dd..9cabcb9c9fd0 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1249,7 +1249,8 @@ batadv_tt_global_orig_entry_find(const struct batadv_tt_global_entry *entry, */ static bool batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry, - const struct batadv_orig_node *orig_node) + const struct batadv_orig_node *orig_node, + u8 *flags) { struct batadv_tt_orig_list_entry *orig_entry; bool found = false; @@ -1257,6 +1258,10 @@ batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry, orig_entry = batadv_tt_global_orig_entry_find(entry, orig_node); if (orig_entry) { found = true; + + if (flags) + *flags = orig_entry->flags; + batadv_tt_orig_list_entry_free_ref(orig_entry); }
@@ -1432,7 +1437,7 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv, if (!(common->flags & BATADV_TT_CLIENT_TEMP)) goto out; if (batadv_tt_global_entry_has_orig(tt_global_entry, - orig_node)) + orig_node, NULL)) goto out_remove; batadv_tt_global_del_orig_list(tt_global_entry); goto add_orig_entry; @@ -2366,17 +2371,24 @@ unlock: * * Returns 1 if the entry is a valid, 0 otherwise. */ -static int batadv_tt_local_valid(const void *entry_ptr, const void *data_ptr) +static int batadv_tt_local_valid(const void *entry_ptr, + const void *data_ptr, + u8 *flags) { const struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
if (tt_common_entry->flags & BATADV_TT_CLIENT_NEW) return 0; + + if (flags) + *flags = tt_common_entry->flags; + return 1; }
static int batadv_tt_global_valid(const void *entry_ptr, - const void *data_ptr) + const void *data_ptr, + u8 *flags) { const struct batadv_tt_common_entry *tt_common_entry = entry_ptr; const struct batadv_tt_global_entry *tt_global_entry; @@ -2390,7 +2402,8 @@ static int batadv_tt_global_valid(const void *entry_ptr, struct batadv_tt_global_entry, common);
- return batadv_tt_global_entry_has_orig(tt_global_entry, orig_node); + return batadv_tt_global_entry_has_orig(tt_global_entry, orig_node, + flags); }
/** @@ -2406,18 +2419,25 @@ static int batadv_tt_global_valid(const void *entry_ptr, static void batadv_tt_tvlv_generate(struct batadv_priv *bat_priv, struct batadv_hashtable *hash, void *tvlv_buff, u16 tt_len, - int (*valid_cb)(const void *, const void *), + int (*valid_cb)(const void *, + const void *, + u8 *flags), void *cb_data) { struct batadv_tt_common_entry *tt_common_entry; struct batadv_tvlv_tt_change *tt_change; struct hlist_head *head; u16 tt_tot, tt_num_entries = 0; + u8 flags; + bool ret; u32 i;
tt_tot = batadv_tt_entries(tt_len); tt_change = (struct batadv_tvlv_tt_change *)tvlv_buff;
+ if (!valid_cb) + return; + rcu_read_lock(); for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -2427,11 +2447,12 @@ static void batadv_tt_tvlv_generate(struct batadv_priv *bat_priv, if (tt_tot == tt_num_entries) break;
- if ((valid_cb) && (!valid_cb(tt_common_entry, cb_data))) + ret = valid_cb(tt_common_entry, cb_data, &flags); + if (!ret) continue;
ether_addr_copy(tt_change->addr, tt_common_entry->addr); - tt_change->flags = tt_common_entry->flags; + tt_change->flags = flags; tt_change->vid = htons(tt_common_entry->vid); memset(tt_change->reserved, 0, sizeof(tt_change->reserved));
From: Marek Lindner mareklindner@neomailbox.ch
commit 16116dac23396e73c01eeee97b102e4833a4b205 upstream.
A translation table TVLV changset sent with an OGM consists of a number of headers (one per VLAN) plus the changeset itself (addition and/or deletion of entries).
The per-VLAN headers are used by OGM recipients for consistency checks. Said consistency check might determine that a full translation table request is needed to restore consistency. If the TT sender adds per-VLAN headers of empty VLANs into the OGM, recipients are led to believe to have reached an inconsistent state and thus request a full table update. The full table does not contain empty VLANs (due to missing entries) the cycle restarts when the next OGM is issued.
Consequently, when the translation table TVLV headers are composed, empty VLANs are to be excluded.
Fixes: 21a57f6e7a3b ("batman-adv: make the TT CRC logic VLAN specific") Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 9cabcb9c9fd0..f16d49473a28 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -813,15 +813,20 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv, struct batadv_tvlv_tt_vlan_data *tt_vlan; struct batadv_softif_vlan *vlan; u16 num_vlan = 0; - u16 num_entries = 0; + u16 vlan_entries = 0; + u16 total_entries = 0; u16 tvlv_len; u8 *tt_change_ptr; int change_offset;
spin_lock_bh(&bat_priv->softif_vlan_list_lock); hlist_for_each_entry_rcu(vlan, &bat_priv->softif_vlan_list, list) { + vlan_entries = atomic_read(&vlan->tt.num_entries); + if (vlan_entries < 1) + continue; + num_vlan++; - num_entries += atomic_read(&vlan->tt.num_entries); + total_entries += vlan_entries; }
change_offset = sizeof(**tt_data); @@ -829,7 +834,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
/* if tt_len is negative, allocate the space needed by the full table */ if (*tt_len < 0) - *tt_len = batadv_tt_len(num_entries); + *tt_len = batadv_tt_len(total_entries);
tvlv_len = *tt_len; tvlv_len += change_offset; @@ -846,6 +851,10 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
tt_vlan = (struct batadv_tvlv_tt_vlan_data *)(*tt_data + 1); hlist_for_each_entry_rcu(vlan, &bat_priv->softif_vlan_list, list) { + vlan_entries = atomic_read(&vlan->tt.num_entries); + if (vlan_entries < 1) + continue; + tt_vlan->vid = htons(vlan->vid); tt_vlan->crc = htonl(vlan->tt.crc);
commit 36dc621ceca1be3ec885aeade5fdafbbcc452a6d upstream.
batman-adv is creating special debugfs directories in the init net_namespace for each valid hard-interface (net_device). But it is possible to rename a net_device to a completely different name then the original one.
It can therefore happen that a user registers a new net_device which gets the name "wlan0" assigned by default. batman-adv is also adding a new directory under $debugfs/batman-adv/ with the name "wlan0".
The user then decides to rename this device to "wl_pri" and registers a different device. The kernel may now decide to use the name "wlan0" again for this new device. batman-adv will detect it as a valid net_device and tries to create a directory with the name "wlan0" under $debugfs/batman-adv/. But there already exists one with this name under this path and thus this fails. batman-adv will detect a problem and rollback the registering of this device.
batman-adv must therefore take care of renaming the debugfs directories for hard-interfaces whenever it detects such a net_device rename.
Fixes: 5bc7c1eb44f2 ("batman-adv: add debugfs structure for information per interface") Reported-by: John Soros sorosj@gmail.com Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/debugfs.c | 20 ++++++++++++++++++++ net/batman-adv/debugfs.h | 6 ++++++ net/batman-adv/hard-interface.c | 3 +++ 3 files changed, 29 insertions(+)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c index c4c1e8030ba0..0d6fe6977be7 100644 --- a/net/batman-adv/debugfs.c +++ b/net/batman-adv/debugfs.c @@ -19,6 +19,7 @@ #include "main.h"
#include <linux/compiler.h> +#include <linux/dcache.h> #include <linux/debugfs.h> #include <linux/device.h> #include <linux/errno.h> @@ -506,6 +507,25 @@ out: return -ENOMEM; }
+/** + * batadv_debugfs_rename_hardif() - Fix debugfs path for renamed hardif + * @hard_iface: hard interface which was renamed + */ +void batadv_debugfs_rename_hardif(struct batadv_hard_iface *hard_iface) +{ + const char *name = hard_iface->net_dev->name; + struct dentry *dir; + struct dentry *d; + + dir = hard_iface->debug_dir; + if (!dir) + return; + + d = debugfs_rename(dir->d_parent, dir, dir->d_parent, name); + if (!d) + pr_err("Can't rename debugfs dir to %s\n", name); +} + /** * batadv_debugfs_del_hardif - delete the base directory for a hard interface * in debugfs. diff --git a/net/batman-adv/debugfs.h b/net/batman-adv/debugfs.h index 80ab8d6f0ab3..4b62b9f152a6 100644 --- a/net/batman-adv/debugfs.h +++ b/net/batman-adv/debugfs.h @@ -33,6 +33,7 @@ void batadv_debugfs_destroy(void); int batadv_debugfs_add_meshif(struct net_device *dev); void batadv_debugfs_del_meshif(struct net_device *dev); int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface); +void batadv_debugfs_rename_hardif(struct batadv_hard_iface *hard_iface); void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface);
#else @@ -60,6 +61,11 @@ int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface) return 0; }
+static inline +void batadv_debugfs_rename_hardif(struct batadv_hard_iface *hard_iface) +{ +} + static inline void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface) { diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 7c297cef20a7..d659441cc64e 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -780,6 +780,9 @@ static int batadv_hard_if_event(struct notifier_block *this, if (hard_iface == primary_if) batadv_primary_if_update_addr(bat_priv, NULL); break; + case NETDEV_CHANGENAME: + batadv_debugfs_rename_hardif(hard_iface); + break; default: break; }
commit 6da7be7d24b2921f8215473ba7552796dff05fe1 upstream.
batman-adv is creating special debugfs directories in the init net_namespace for each created soft-interface (batadv net_device). But it is possible to rename a net_device to a completely different name then the original one.
It can therefore happen that a user registers a new batadv net_device with the name "bat0". batman-adv is then also adding a new directory under $debugfs/batman-adv/ with the name "wlan0".
The user then decides to rename this device to "bat1" and registers a different batadv device with the name "bat0". batman-adv will then try to create a directory with the name "bat0" under $debugfs/batman-adv/ again. But there already exists one with this name under this path and thus this fails. batman-adv will detect a problem and rollback the registering of this device.
batman-adv must therefore take care of renaming the debugfs directories for soft-interfaces whenever it detects such a net_device rename.
Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/debugfs.c | 20 +++++++++++++++++++ net/batman-adv/debugfs.h | 5 +++++ net/batman-adv/hard-interface.c | 34 +++++++++++++++++++++++++++------ 3 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c index 0d6fe6977be7..b2ef03a3a2d4 100644 --- a/net/batman-adv/debugfs.c +++ b/net/batman-adv/debugfs.c @@ -581,6 +581,26 @@ out: return -ENOMEM; }
+/** + * batadv_debugfs_rename_meshif() - Fix debugfs path for renamed softif + * @dev: net_device which was renamed + */ +void batadv_debugfs_rename_meshif(struct net_device *dev) +{ + struct batadv_priv *bat_priv = netdev_priv(dev); + const char *name = dev->name; + struct dentry *dir; + struct dentry *d; + + dir = bat_priv->debug_dir; + if (!dir) + return; + + d = debugfs_rename(dir->d_parent, dir, dir->d_parent, name); + if (!d) + pr_err("Can't rename debugfs dir to %s\n", name); +} + void batadv_debugfs_del_meshif(struct net_device *dev) { struct batadv_priv *bat_priv = netdev_priv(dev); diff --git a/net/batman-adv/debugfs.h b/net/batman-adv/debugfs.h index 4b62b9f152a6..347f793a18b2 100644 --- a/net/batman-adv/debugfs.h +++ b/net/batman-adv/debugfs.h @@ -31,6 +31,7 @@ struct net_device; void batadv_debugfs_init(void); void batadv_debugfs_destroy(void); int batadv_debugfs_add_meshif(struct net_device *dev); +void batadv_debugfs_rename_meshif(struct net_device *dev); void batadv_debugfs_del_meshif(struct net_device *dev); int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface); void batadv_debugfs_rename_hardif(struct batadv_hard_iface *hard_iface); @@ -51,6 +52,10 @@ static inline int batadv_debugfs_add_meshif(struct net_device *dev) return 0; }
+static inline void batadv_debugfs_rename_meshif(struct net_device *dev) +{ +} + static inline void batadv_debugfs_del_meshif(struct net_device *dev) { } diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index d659441cc64e..70f7bb42cd2a 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -725,6 +725,32 @@ void batadv_hardif_remove_interfaces(void) rtnl_unlock(); }
+/** + * batadv_hard_if_event_softif() - Handle events for soft interfaces + * @event: NETDEV_* event to handle + * @net_dev: net_device which generated an event + * + * Return: NOTIFY_* result + */ +static int batadv_hard_if_event_softif(unsigned long event, + struct net_device *net_dev) +{ + struct batadv_priv *bat_priv; + + switch (event) { + case NETDEV_REGISTER: + batadv_sysfs_add_meshif(net_dev); + bat_priv = netdev_priv(net_dev); + batadv_softif_create_vlan(bat_priv, BATADV_NO_FLAGS); + break; + case NETDEV_CHANGENAME: + batadv_debugfs_rename_meshif(net_dev); + break; + } + + return NOTIFY_DONE; +} + static int batadv_hard_if_event(struct notifier_block *this, unsigned long event, void *ptr) { @@ -733,12 +759,8 @@ static int batadv_hard_if_event(struct notifier_block *this, struct batadv_hard_iface *primary_if = NULL; struct batadv_priv *bat_priv;
- if (batadv_softif_is_valid(net_dev) && event == NETDEV_REGISTER) { - batadv_sysfs_add_meshif(net_dev); - bat_priv = netdev_priv(net_dev); - batadv_softif_create_vlan(bat_priv, BATADV_NO_FLAGS); - return NOTIFY_DONE; - } + if (batadv_softif_is_valid(net_dev)) + return batadv_hard_if_event_softif(event, net_dev);
hard_iface = batadv_hardif_get_by_netdev(net_dev); if (!hard_iface && event == NETDEV_REGISTER)
From: Linus Lüssing linus.luessing@c0d3.blue
commit 4a519b83da16927fb98fd32b0f598e639d1f1859 upstream.
Since commit 54e22f265e87 ("batman-adv: fix TT sync flag inconsistencies") TT sync flags and TT non-sync'd flags are supposed to be stored separately.
The previous patch missed to apply this separation on a TT entry with only a single TT orig entry.
This is a minor fix because with only a single TT orig entry the DDoS issue the former patch solves does not apply.
Fixes: 54e22f265e87 ("batman-adv: fix TT sync flag inconsistencies") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index f16d49473a28..4a685cf2439d 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1405,7 +1405,8 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv, ether_addr_copy(common->addr, tt_addr); common->vid = vid;
- common->flags = flags; + common->flags = flags & (~BATADV_TT_SYNC_MASK); + tt_global_entry->roam_at = 0; /* node must store current time in case of roaming. This is * needed to purge this entry out on timeout (if nobody claims
commit dff9bc42ab0b2d38c5e90ddd79b238fed5b4c7ad upstream.
The function batadv_gw_node_add is responsible for adding new gw_node to the gateway_list. It is expecting that the caller already checked that there is not already an entry with the same key or not.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol") Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/gateway_client.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c index 055baa396260..a88b529b7ca0 100644 --- a/net/batman-adv/gateway_client.c +++ b/net/batman-adv/gateway_client.c @@ -29,6 +29,7 @@ #include <linux/ipv6.h> #include <linux/kernel.h> #include <linux/list.h> +#include <linux/lockdep.h> #include <linux/netdevice.h> #include <linux/rculist.h> #include <linux/rcupdate.h> @@ -413,6 +414,9 @@ out: * @bat_priv: the bat priv with all the soft interface information * @orig_node: originator announcing gateway capabilities * @gateway: announced bandwidth information + * + * Has to be called with the appropriate locks being acquired + * (gw.list_lock). */ static void batadv_gw_node_add(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, @@ -420,6 +424,8 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv, { struct batadv_gw_node *gw_node;
+ lockdep_assert_held(&bat_priv->gw.list_lock); + if (gateway->bandwidth_down == 0) return;
@@ -438,9 +444,7 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv, gw_node->bandwidth_up = ntohl(gateway->bandwidth_up); atomic_set(&gw_node->refcount, 1);
- spin_lock_bh(&bat_priv->gw.list_lock); hlist_add_head_rcu(&gw_node->list, &bat_priv->gw.list); - spin_unlock_bh(&bat_priv->gw.list_lock);
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Found new gateway %pM -> gw bandwidth: %u.%u/%u.%u MBit\n", @@ -493,11 +497,14 @@ void batadv_gw_node_update(struct batadv_priv *bat_priv, { struct batadv_gw_node *gw_node, *curr_gw = NULL;
+ spin_lock_bh(&bat_priv->gw.list_lock); gw_node = batadv_gw_node_get(bat_priv, orig_node); if (!gw_node) { batadv_gw_node_add(bat_priv, orig_node, gateway); + spin_unlock_bh(&bat_priv->gw.list_lock); goto out; } + spin_unlock_bh(&bat_priv->gw.list_lock);
if ((gw_node->bandwidth_down == ntohl(gateway->bandwidth_down)) && (gw_node->bandwidth_up == ntohl(gateway->bandwidth_up)))
commit fa122fec8640eb7186ce5a41b83a4c1744ceef8f upstream.
The function batadv_nc_get_nc_node is responsible for adding new nc_nodes to the in_coding_list and out_coding_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
Fixes: d56b1705e28c ("batman-adv: network coding - detect coding nodes and remove these after timeout") Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Marek Lindner mareklindner@neomailbox.ch Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/network-coding.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c index d0956f726547..86c69208da2b 100644 --- a/net/batman-adv/network-coding.c +++ b/net/batman-adv/network-coding.c @@ -828,19 +828,29 @@ static struct batadv_nc_node spinlock_t *lock; /* Used to lock list selected by "int in_coding" */ struct list_head *list;
+ /* Select ingoing or outgoing coding node */ + if (in_coding) { + lock = &orig_neigh_node->in_coding_list_lock; + list = &orig_neigh_node->in_coding_list; + } else { + lock = &orig_neigh_node->out_coding_list_lock; + list = &orig_neigh_node->out_coding_list; + } + + spin_lock_bh(lock); + /* Check if nc_node is already added */ nc_node = batadv_nc_find_nc_node(orig_node, orig_neigh_node, in_coding);
/* Node found */ if (nc_node) - return nc_node; + goto unlock;
nc_node = kzalloc(sizeof(*nc_node), GFP_ATOMIC); if (!nc_node) - return NULL; + goto unlock;
- if (!atomic_inc_not_zero(&orig_neigh_node->refcount)) - goto free; + atomic_inc(&orig_neigh_node->refcount);
/* Initialize nc_node */ INIT_LIST_HEAD(&nc_node->list); @@ -848,28 +858,15 @@ static struct batadv_nc_node nc_node->orig_node = orig_neigh_node; atomic_set(&nc_node->refcount, 2);
- /* Select ingoing or outgoing coding node */ - if (in_coding) { - lock = &orig_neigh_node->in_coding_list_lock; - list = &orig_neigh_node->in_coding_list; - } else { - lock = &orig_neigh_node->out_coding_list_lock; - list = &orig_neigh_node->out_coding_list; - } - batadv_dbg(BATADV_DBG_NC, bat_priv, "Adding nc_node %pM -> %pM\n", nc_node->addr, nc_node->orig_node->orig);
/* Add nc_node to orig_node */ - spin_lock_bh(lock); list_add_tail_rcu(&nc_node->list, list); +unlock: spin_unlock_bh(lock);
return nc_node; - -free: - kfree(nc_node); - return NULL; }
/**
commit e7136e48ffdfb9f37b0820f619380485eb407361 upstream.
The function batadv_tt_global_orig_entry_add is responsible for adding new tt_orig_list_entry to the orig_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
Fixes: d657e621a0f5 ("batman-adv: add reference counting for type batadv_tt_orig_list_entry") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- 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 4a685cf2439d..1d445293efea 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1315,6 +1315,8 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, { struct batadv_tt_orig_list_entry *orig_entry;
+ spin_lock_bh(&tt_global->list_lock); + orig_entry = batadv_tt_global_orig_entry_find(tt_global, orig_node); if (orig_entry) { /* refresh the ttvn: the current value could be a bogus one that @@ -1337,10 +1339,8 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, orig_entry->flags = flags; atomic_set(&orig_entry->refcount, 2);
- spin_lock_bh(&tt_global->list_lock); hlist_add_head_rcu(&orig_entry->list, &tt_global->orig_list); - spin_unlock_bh(&tt_global->list_lock); atomic_inc(&tt_global->orig_list_count);
sync_flags: @@ -1348,6 +1348,8 @@ sync_flags: out: if (orig_entry) batadv_tt_orig_list_entry_free_ref(orig_entry); + + spin_unlock_bh(&tt_global->list_lock); }
/**
commit ae3cdc97dc10c7a3b31f297dab429bfb774c9ccb upstream.
The function batadv_tvlv_handler_register is responsible for adding new tvlv_handler to the handler_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
Fixes: ef26157747d4 ("batman-adv: tvlv - basic infrastructure") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/main.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index d7f17c1aa4a4..2bdbaff3279b 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -1079,15 +1079,20 @@ void batadv_tvlv_handler_register(struct batadv_priv *bat_priv, { struct batadv_tvlv_handler *tvlv_handler;
+ spin_lock_bh(&bat_priv->tvlv.handler_list_lock); + tvlv_handler = batadv_tvlv_handler_get(bat_priv, type, version); if (tvlv_handler) { + spin_unlock_bh(&bat_priv->tvlv.handler_list_lock); batadv_tvlv_handler_free_ref(tvlv_handler); return; }
tvlv_handler = kzalloc(sizeof(*tvlv_handler), GFP_ATOMIC); - if (!tvlv_handler) + if (!tvlv_handler) { + spin_unlock_bh(&bat_priv->tvlv.handler_list_lock); return; + }
tvlv_handler->ogm_handler = optr; tvlv_handler->unicast_handler = uptr; @@ -1097,7 +1102,6 @@ void batadv_tvlv_handler_register(struct batadv_priv *bat_priv, atomic_set(&tvlv_handler->refcount, 1); INIT_HLIST_NODE(&tvlv_handler->list);
- spin_lock_bh(&bat_priv->tvlv.handler_list_lock); hlist_add_head_rcu(&tvlv_handler->list, &bat_priv->tvlv.handler_list); spin_unlock_bh(&bat_priv->tvlv.handler_list_lock); }
commit 4ba104f468bbfc27362c393815d03aa18fb7a20f upstream.
The batadv_hash_remove is a function which searches the hashtable for an entry using a needle, a hashtable bucket selection function and a compare function. It will lock the bucket list and delete an entry when the compare function matches it with the needle. It returns the pointer to the hlist_node which matches or NULL when no entry matches the needle.
The batadv_bla_del_claim is not itself protected in anyway to avoid that any other function is modifying the hashtable between the search for the entry and the call to batadv_hash_remove. It can therefore happen that the entry either doesn't exist anymore or an entry was deleted which is not the same object as the needle. In such an situation, the reference counter (for the reference stored in the hashtable) must not be reduced for the needle. Instead the reference counter of the actually removed entry has to be reduced.
Otherwise the reference counter will underflow and the object might be freed before all its references were dropped. The kref helpers reported this problem as:
refcount_t: underflow; use-after-free.
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bridge_loop_avoidance.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index 6e2a5d02ce1f..cea7fdeac5aa 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -694,6 +694,8 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv, const u8 *mac, const unsigned short vid) { struct batadv_bla_claim search_claim, *claim; + struct batadv_bla_claim *claim_removed_entry; + struct hlist_node *claim_removed_node;
ether_addr_copy(search_claim.addr, mac); search_claim.vid = vid; @@ -704,10 +706,18 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv, batadv_dbg(BATADV_DBG_BLA, bat_priv, "bla_del_claim(): %pM, vid %d\n", mac, BATADV_PRINT_VID(vid));
- batadv_hash_remove(bat_priv->bla.claim_hash, batadv_compare_claim, - batadv_choose_claim, claim); - batadv_claim_free_ref(claim); /* reference from the hash is gone */ + claim_removed_node = batadv_hash_remove(bat_priv->bla.claim_hash, + batadv_compare_claim, + batadv_choose_claim, claim); + if (!claim_removed_node) + goto free_claim;
+ /* reference from the hash is gone */ + claim_removed_entry = hlist_entry(claim_removed_node, + struct batadv_bla_claim, hash_entry); + batadv_claim_free_ref(claim_removed_entry); + +free_claim: /* don't need the reference from hash_find() anymore */ batadv_claim_free_ref(claim); }
commit 3d65b9accab4a7ed5038f6df403fbd5e298398c7 upstream.
The batadv_hash_remove is a function which searches the hashtable for an entry using a needle, a hashtable bucket selection function and a compare function. It will lock the bucket list and delete an entry when the compare function matches it with the needle. It returns the pointer to the hlist_node which matches or NULL when no entry matches the needle.
The batadv_tt_local_remove is not itself protected in anyway to avoid that any other function is modifying the hashtable between the search for the entry and the call to batadv_hash_remove. It can therefore happen that the entry either doesn't exist anymore or an entry was deleted which is not the same object as the needle. In such an situation, the reference counter (for the reference stored in the hashtable) must not be reduced for the needle. Instead the reference counter of the actually removed entry has to be reduced.
Otherwise the reference counter will underflow and the object might be freed before all its references were dropped. The kref helpers reported this problem as:
refcount_t: underflow; use-after-free.
Fixes: ef72706a0543 ("batman-adv: protect tt_local_entry from concurrent delete events") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/translation-table.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 1d445293efea..a6db0ebf911f 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1049,9 +1049,10 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr, unsigned short vid, const char *message, bool roaming) { + struct batadv_tt_local_entry *tt_removed_entry; struct batadv_tt_local_entry *tt_local_entry; u16 flags, curr_flags = BATADV_NO_FLAGS; - void *tt_entry_exists; + struct hlist_node *tt_removed_node;
tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid); if (!tt_local_entry) @@ -1080,15 +1081,18 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr, */ batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
- tt_entry_exists = batadv_hash_remove(bat_priv->tt.local_hash, + tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash, batadv_compare_tt, batadv_choose_tt, &tt_local_entry->common); - if (!tt_entry_exists) + if (!tt_removed_node) goto out;
- /* extra call to free the local tt entry */ - batadv_tt_local_entry_free_ref(tt_local_entry); + /* drop reference of remove hash entry */ + tt_removed_entry = hlist_entry(tt_removed_node, + struct batadv_tt_local_entry, + common.hash_entry); + batadv_tt_local_entry_free_ref(tt_removed_entry);
out: if (tt_local_entry)
commit f131a56880d10932931e74773fb8702894a94a75 upstream.
The batadv_hash_remove is a function which searches the hashtable for an entry using a needle, a hashtable bucket selection function and a compare function. It will lock the bucket list and delete an entry when the compare function matches it with the needle. It returns the pointer to the hlist_node which matches or NULL when no entry matches the needle.
The batadv_tt_global_free is not itself protected in anyway to avoid that any other function is modifying the hashtable between the search for the entry and the call to batadv_hash_remove. It can therefore happen that the entry either doesn't exist anymore or an entry was deleted which is not the same object as the needle. In such an situation, the reference counter (for the reference stored in the hashtable) must not be reduced for the needle. Instead the reference counter of the actually removed entry has to be reduced.
Otherwise the reference counter will underflow and the object might be freed before all its references were dropped. The kref helpers reported this problem as:
refcount_t: underflow; use-after-free.
Fixes: 7683fdc1e886 ("batman-adv: protect the local and the global trans-tables with rcu") Reported-by: Martin Weinelt martin@linuxlounge.net Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Antonio Quartulli a@unstable.cc Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/translation-table.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index a6db0ebf911f..67ee7c83a28d 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -508,14 +508,26 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv, struct batadv_tt_global_entry *tt_global, const char *message) { + struct batadv_tt_global_entry *tt_removed_entry; + struct hlist_node *tt_removed_node; + batadv_dbg(BATADV_DBG_TT, bat_priv, "Deleting global tt entry %pM (vid: %d): %s\n", tt_global->common.addr, BATADV_PRINT_VID(tt_global->common.vid), message);
- batadv_hash_remove(bat_priv->tt.global_hash, batadv_compare_tt, - batadv_choose_tt, &tt_global->common); - batadv_tt_global_entry_free_ref(tt_global); + tt_removed_node = batadv_hash_remove(bat_priv->tt.global_hash, + batadv_compare_tt, + batadv_choose_tt, + &tt_global->common); + if (!tt_removed_node) + return; + + /* drop reference of remove hash entry */ + tt_removed_entry = hlist_entry(tt_removed_node, + struct batadv_tt_global_entry, + common.hash_entry); + batadv_tt_global_entry_free_ref(tt_removed_entry); }
/**
commit a15d56a60760aa9dbe26343b9a0ac5228f35d445 upstream.
Multiple batadv_ogm_packet can be stored in an skbuff. The functions batadv_iv_ogm_send_to_if()/batadv_iv_ogm_receive() use batadv_iv_ogm_aggr_packet() to check if there is another additional batadv_ogm_packet in the skb or not before they continue processing the packet.
The length for such an OGM is BATADV_OGM_HLEN + batadv_ogm_packet->tvlv_len. The check must first check that at least BATADV_OGM_HLEN bytes are available before it accesses tvlv_len (which is part of the header. Otherwise it might try read outside of the currently available skbuff to get the content of tvlv_len.
Fixes: ef26157747d4 ("batman-adv: tvlv - basic infrastructure") Reported-by: syzbot+355cab184197dbbfa384@syzkaller.appspotmail.com Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Antonio Quartulli a@unstable.cc Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/bat_iv_ogm.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 5010b81e746f..f1c8b1311dee 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -397,14 +397,19 @@ static u8 batadv_hop_penalty(u8 tq, const struct batadv_priv *bat_priv) return new_tq; }
-/* is there another aggregated packet here? */ -static bool batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len, - __be16 tvlv_len) +static bool +batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len, + const struct batadv_ogm_packet *ogm_packet) { int next_buff_pos = 0;
- next_buff_pos += buff_pos + BATADV_OGM_HLEN; - next_buff_pos += ntohs(tvlv_len); + /* check if there is enough space for the header */ + next_buff_pos += buff_pos + sizeof(*ogm_packet); + if (next_buff_pos > packet_len) + return false; + + /* check if there is enough space for the optional TVLV */ + next_buff_pos += ntohs(ogm_packet->tvlv_len);
return (next_buff_pos <= packet_len) && (next_buff_pos <= BATADV_MAX_AGGREGATION_BYTES); @@ -432,7 +437,7 @@ static void batadv_iv_ogm_send_to_if(struct batadv_forw_packet *forw_packet,
/* adjust all flags and log packets */ while (batadv_iv_ogm_aggr_packet(buff_pos, forw_packet->packet_len, - batadv_ogm_packet->tvlv_len)) { + batadv_ogm_packet)) { /* we might have aggregated direct link packets with an * ordinary base packet */ @@ -1751,7 +1756,7 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
/* unpack the aggregated packets and process them one by one */ while (batadv_iv_ogm_aggr_packet(ogm_offset, skb_headlen(skb), - ogm_packet->tvlv_len)) { + ogm_packet)) { batadv_iv_ogm_process(skb, ogm_offset, if_incoming);
ogm_offset += BATADV_OGM_HLEN;
commit 40e220b4218bb3d278e5e8cc04ccdfd1c7ff8307 upstream.
Each slave interface of an B.A.T.M.A.N. IV virtual interface has an OGM packet buffer which is initialized using data from netdevice notifier and other rtnetlink related hooks. It is sent regularly via various slave interfaces of the batadv virtual interface and in this process also modified (realloced) to integrate additional state information via TVLV containers.
It must be avoided that the worker item is executed without a common lock with the netdevice notifier/rtnetlink helpers. Otherwise it can either happen that half modified/freed data is sent out or functions modifying the OGM buffer try to access already freed memory regions.
Reported-by: syzbot+0cc629f19ccb8534935b@syzkaller.appspotmail.com Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bat_iv_ogm.c | 57 +++++++++++++++++++++++++++++---- net/batman-adv/hard-interface.c | 2 ++ net/batman-adv/types.h | 2 ++ 3 files changed, 55 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index f1c8b1311dee..eb2f2ed9a956 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -316,14 +316,18 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface) unsigned char *ogm_buff; u32 random_seqno;
+ mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); + /* randomize initial seqno to avoid collision */ get_random_bytes(&random_seqno, sizeof(random_seqno)); atomic_set(&hard_iface->bat_iv.ogm_seqno, random_seqno);
hard_iface->bat_iv.ogm_buff_len = BATADV_OGM_HLEN; ogm_buff = kmalloc(hard_iface->bat_iv.ogm_buff_len, GFP_ATOMIC); - if (!ogm_buff) + if (!ogm_buff) { + mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); return -ENOMEM; + }
hard_iface->bat_iv.ogm_buff = ogm_buff;
@@ -335,36 +339,60 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface) batadv_ogm_packet->reserved = 0; batadv_ogm_packet->tq = BATADV_TQ_MAX_VALUE;
+ mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); + return 0; }
static void batadv_iv_ogm_iface_disable(struct batadv_hard_iface *hard_iface) { + mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); + kfree(hard_iface->bat_iv.ogm_buff); hard_iface->bat_iv.ogm_buff = NULL; + + mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); }
static void batadv_iv_ogm_iface_update_mac(struct batadv_hard_iface *hard_iface) { struct batadv_ogm_packet *batadv_ogm_packet; - unsigned char *ogm_buff = hard_iface->bat_iv.ogm_buff; + void *ogm_buff;
- batadv_ogm_packet = (struct batadv_ogm_packet *)ogm_buff; + mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); + + ogm_buff = hard_iface->bat_iv.ogm_buff; + if (!ogm_buff) + goto unlock; + + batadv_ogm_packet = ogm_buff; ether_addr_copy(batadv_ogm_packet->orig, hard_iface->net_dev->dev_addr); ether_addr_copy(batadv_ogm_packet->prev_sender, hard_iface->net_dev->dev_addr); + +unlock: + mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); }
static void batadv_iv_ogm_primary_iface_set(struct batadv_hard_iface *hard_iface) { struct batadv_ogm_packet *batadv_ogm_packet; - unsigned char *ogm_buff = hard_iface->bat_iv.ogm_buff; + void *ogm_buff;
- batadv_ogm_packet = (struct batadv_ogm_packet *)ogm_buff; + mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); + + ogm_buff = hard_iface->bat_iv.ogm_buff; + if (!ogm_buff) + goto unlock; + + batadv_ogm_packet = ogm_buff; batadv_ogm_packet->flags = BATADV_PRIMARIES_FIRST_HOP; batadv_ogm_packet->ttl = BATADV_TTL; + +unlock: + mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); }
/* when do we schedule our own ogm to be sent */ @@ -899,7 +927,11 @@ batadv_iv_ogm_slide_own_bcast_window(struct batadv_hard_iface *hard_iface) } }
-static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) +/** + * batadv_iv_ogm_schedule_buff() - schedule submission of hardif ogm buffer + * @hard_iface: interface whose ogm buffer should be transmitted + */ +static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface) { struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface); unsigned char **ogm_buff = &hard_iface->bat_iv.ogm_buff; @@ -910,6 +942,8 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) u16 tvlv_len = 0; unsigned long send_time;
+ lockdep_assert_held(&hard_iface->bat_iv.ogm_buff_mutex); + primary_if = batadv_primary_if_get_selected(bat_priv);
if (hard_iface == primary_if) { @@ -961,6 +995,17 @@ out: batadv_hardif_free_ref(primary_if); }
+static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) +{ + if (hard_iface->if_status == BATADV_IF_NOT_IN_USE || + hard_iface->if_status == BATADV_IF_TO_BE_REMOVED) + return; + + mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); + batadv_iv_ogm_schedule_buff(hard_iface); + mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); +} + /** * batadv_iv_ogm_orig_update - use OGM to update corresponding data in an * originator diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 70f7bb42cd2a..c59bbc327763 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -26,6 +26,7 @@ #include <linux/if.h> #include <linux/kernel.h> #include <linux/list.h> +#include <linux/mutex.h> #include <linux/netdevice.h> #include <linux/printk.h> #include <linux/rculist.h> @@ -671,6 +672,7 @@ batadv_hardif_add_interface(struct net_device *net_dev) goto free_sysfs;
INIT_LIST_HEAD(&hard_iface->list); + mutex_init(&hard_iface->bat_iv.ogm_buff_mutex); INIT_WORK(&hard_iface->cleanup_work, batadv_hardif_remove_interface_finish);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 6dcee1991c54..8fce1241ad6d 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -77,11 +77,13 @@ enum batadv_dhcp_recipient { * @ogm_buff: buffer holding the OGM packet * @ogm_buff_len: length of the OGM packet buffer * @ogm_seqno: OGM sequence number - used to identify each OGM + * @ogm_buff_mutex: lock protecting ogm_buff and ogm_buff_len */ struct batadv_hard_iface_bat_iv { unsigned char *ogm_buff; int ogm_buff_len; atomic_t ogm_seqno; + struct mutex ogm_buff_mutex; };
/**
A transmission scheduling for an interface which is currently dropped by batadv_iv_ogm_iface_disable could still be in progress. The B.A.T.M.A.N. V is simply cancelling the workqueue item in an synchronous way but this is not possible with B.A.T.M.A.N. IV because the OGM submissions are intertwined.
Instead it has to stop submitting the OGM when it detect that the buffer pointer is set to NULL.
Reported-by: syzbot+a98f2016f40b9cd3818a@syzkaller.appspotmail.com Reported-by: syzbot+ac36b6a33c28a491e929@syzkaller.appspotmail.com Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol") Signed-off-by: Sven Eckelmann sven@narfation.org Cc: Hillf Danton hdanton@sina.com Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bat_iv_ogm.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index eb2f2ed9a956..caea5bb38d4b 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -944,6 +944,10 @@ static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface)
lockdep_assert_held(&hard_iface->bat_iv.ogm_buff_mutex);
+ /* interface already disabled by batadv_iv_ogm_iface_disable */ + if (!*ogm_buff) + return; + primary_if = batadv_primary_if_get_selected(bat_priv);
if (hard_iface == primary_if) {
On Wed, Mar 18, 2020 at 12:26:46AM +0100, Sven Eckelmann wrote:
Hi,
we were asked by Greg KH to check the failed backport of some recent fix to linux 4.4.y and provide backports for it when required. I've found a lot more missing patches than I've expected in this process and queued them up here.
All now queued up, thanks!
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Sven Eckelmann