Open-Mesh » batman-adv

There is the possibility that batadv_tt_prepare_tvlv_global_data/batadv_tt_prepare_tvlv_local_data and any function operating on the buffer is causing a buffer overflow

Here some details from IRC

<ecsv_> hm, isn't the batadv_tt_prepare_tvlv_global_data buggy?
<ecsv_> it traverses the vlan_list twice in hope that it returns the same number of entries
<ecsv_> this doesn't have to be true because it is not locked with a mutex/spinlock
<ecsv_> let us assume that something adds a vlan to vlan_list while batadv_tt_prepare_tvlv_global_data is running
<ecsv_> and this happens on a core so that the current core first sees the new entry in the list after the kmalloc
<ecsv_> the second loop will traverse the list again and then at the end write an entry more than it is expected to write to the buffer
<ecsv_> or is tt_buff_lock also locked around the vlan/tt entry del/add functions?
<ecsv_> ah, no. it isn't.  for example the non-full table doesn't use it  in batadv_send_other_tt_response

Potential fix is:

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 0225616d..7fa3a0a0 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -862,7 +862,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);
@@ -900,7 +900,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;
 }

@@ -936,7 +936,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);
@@ -974,7 +974,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;
 }