batman-adv: Merge bugfixes from 2024.4

* Do not send uninitialized TT changes * Remove uninitialized data in full table TT response * Do not let TT changes list grows indefinitely Signed-off-by: Sven Eckelmann <[email protected]>
openwrt · Dec 10, 2024 · eaa4aba · eaa4aba
1 parent 3f15699
commit eaa4aba
Show file tree

Hide file tree

Showing 4 changed files with 229 additions and 1 deletion.
diff --git a/batman-adv/Makefile b/batman-adv/Makefile
@@ -4,7 +4,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=batman-adv
 PKG_VERSION:=2024.3
-PKG_RELEASE:=2
+PKG_RELEASE:=3
 
 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
 PKG_SOURCE_URL:=https://downloads.open-mesh.org/batman/releases/batman-adv-$(PKG_VERSION)

diff --git a/batman-adv/patches/0003-batman-adv-Do-not-send-uninitialized-TT-changes.patch b/batman-adv/patches/0003-batman-adv-Do-not-send-uninitialized-TT-changes.patch
@@ -0,0 +1,64 @@
+From: Remi Pommarel <[email protected]>
+Date: Fri, 22 Nov 2024 16:52:48 +0100
+Subject: batman-adv: Do not send uninitialized TT changes
+
+The number of TT changes can be less than initially expected in
+batadv_tt_tvlv_container_update() (changes can be removed by
+batadv_tt_local_event() in ADD+DEL sequence between reading
+tt_diff_entries_num and actually iterating the change list under lock).
+
+Thus tt_diff_len could be bigger than the actual changes size that need
+to be sent. Because batadv_send_my_tt_response sends the whole
+packet, uninitialized data can be interpreted as TT changes on other
+nodes leading to weird TT global entries on those nodes such as:
+
+ * 00:00:00:00:00:00   -1 [....] (  0) 88:12:4e:ad:7e:ba (179) (0x45845380)
+ * 00:00:00:00:78:79 4092 [.W..] (  0) 88:12:4e:ad:7e:3c (145) (0x8ebadb8b)
+
+All of the above also applies to OGM tvlv container buffer's tvlv_len.
+
+Remove the extra allocated space to avoid sending uninitialized TT
+changes in batadv_send_my_tt_response() and batadv_v_ogm_send_softif().
+
+Fixes: 8405301b9794 ("batman-adv: tvlv - convert tt data sent within OGMs")
+Signed-off-by: Remi Pommarel <[email protected]>
+Signed-off-by: Sven Eckelmann <[email protected]>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/5fe3a7a48ea6374280dc855db7b802d70b1870c6
+
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -990,6 +990,7 @@ static void batadv_tt_tvlv_container_upd
+ 	int tt_diff_len, tt_change_len = 0;
+ 	int tt_diff_entries_num = 0;
+ 	int tt_diff_entries_count = 0;
++	size_t tt_extra_len = 0;
+ 	u16 tvlv_len;
+
+ 	tt_diff_entries_num = atomic_read(&bat_priv->tt.local_changes);
+@@ -1027,6 +1028,9 @@ static void batadv_tt_tvlv_container_upd
+ 	}
+ 	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
+
++	tt_extra_len = batadv_tt_len(tt_diff_entries_num -
++				     tt_diff_entries_count);
++
+ 	/* Keep the buffer for possible tt_request */
+ 	spin_lock_bh(&bat_priv->tt.last_changeset_lock);
+ 	kfree(bat_priv->tt.last_changeset);
+@@ -1035,6 +1039,7 @@ static void batadv_tt_tvlv_container_upd
+ 	tt_change_len = batadv_tt_len(tt_diff_entries_count);
+ 	/* check whether this new OGM has no changes due to size problems */
+ 	if (tt_diff_entries_count > 0) {
++		tt_diff_len -= tt_extra_len;
+ 		/* if kmalloc() fails we will reply with the full table
+ 		 * instead of providing the diff
+ 		 */
+@@ -1047,6 +1052,8 @@ static void batadv_tt_tvlv_container_upd
+ 	}
+ 	spin_unlock_bh(&bat_priv->tt.last_changeset_lock);
+
++	/* Remove extra packet space for OGM */
++	tvlv_len -= tt_extra_len;
+ container_register:
+ 	batadv_tvlv_container_register(bat_priv, BATADV_TVLV_TT, 1, tt_data,
+ 				       tvlv_len);
diff --git a/batman-adv/patches/0004-batman-adv-Remove-uninitialized-data-in-full-table-T.patch b/batman-adv/patches/0004-batman-adv-Remove-uninitialized-data-in-full-table-T.patch
@@ -0,0 +1,101 @@
+From: Remi Pommarel <[email protected]>
+Date: Fri, 22 Nov 2024 16:52:49 +0100
+Subject: batman-adv: Remove uninitialized data in full table TT response
+
+The number of entries filled by batadv_tt_tvlv_generate() can be less
+than initially expected in batadv_tt_prepare_tvlv_{global,local}_data()
+(changes can be removed by batadv_tt_local_event() in ADD+DEL sequence
+in the meantime as the lock held during the whole tvlv global/local data
+generation).
+
+Thus tvlv_len could be bigger than the actual TT entry size that need
+to be sent so full table TT_RESPONSE could hold invalid TT entries such
+as below.
+
+ * 00:00:00:00:00:00   -1 [....] (  0) 88:12:4e:ad:7e:ba (179) (0x45845380)
+ * 00:00:00:00:78:79 4092 [.W..] (  0) 88:12:4e:ad:7e:3c (145) (0x8ebadb8b)
+
+Remove the extra allocated space to avoid sending uninitialized entries
+for full table TT_RESPONSE in both batadv_send_other_tt_response() and
+batadv_send_my_tt_response().
+
+Fixes: 21a57f6e7a3b ("batman-adv: make the TT CRC logic VLAN specific")
+Signed-off-by: Remi Pommarel <[email protected]>
+Signed-off-by: Sven Eckelmann <[email protected]>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/095c3965bdc29e43546a9cdd21f179f952e01f48
+
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -2754,14 +2754,16 @@ static bool batadv_tt_global_valid(const
+  *
+  * Fills the tvlv buff with the tt entries from the specified hash. If valid_cb
+  * is not provided then this becomes a no-op.
++ *
++ * Return: Remaining unused length in tvlv_buff.
+  */
+-static void batadv_tt_tvlv_generate(struct batadv_priv *bat_priv,
+-				    struct batadv_hashtable *hash,
+-				    void *tvlv_buff, u16 tt_len,
+-				    bool (*valid_cb)(const void *,
+-						     const void *,
+-						     u8 *flags),
+-				    void *cb_data)
++static u16 batadv_tt_tvlv_generate(struct batadv_priv *bat_priv,
++				   struct batadv_hashtable *hash,
++				   void *tvlv_buff, u16 tt_len,
++				   bool (*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;
+@@ -2775,7 +2777,7 @@ static void batadv_tt_tvlv_generate(stru
+ 	tt_change = tvlv_buff;
+
+ 	if (!valid_cb)
+-		return;
++		return tt_len;
+
+ 	rcu_read_lock();
+ 	for (i = 0; i < hash->size; i++) {
+@@ -2801,6 +2803,8 @@ static void batadv_tt_tvlv_generate(stru
+ 		}
+ 	}
+ 	rcu_read_unlock();
++
++	return batadv_tt_len(tt_tot - tt_num_entries);
+ }
+
+ /**
+@@ -3076,10 +3080,11 @@ static bool batadv_send_other_tt_respons
+ 			goto out;
+
+ 		/* fill the rest of the tvlv with the real TT entries */
+-		batadv_tt_tvlv_generate(bat_priv, bat_priv->tt.global_hash,
+-					tt_change, tt_len,
+-					batadv_tt_global_valid,
+-					req_dst_orig_node);
++		tvlv_len -= batadv_tt_tvlv_generate(bat_priv,
++						    bat_priv->tt.global_hash,
++						    tt_change, tt_len,
++						    batadv_tt_global_valid,
++						    req_dst_orig_node);
+ 	}
+
+ 	/* Don't send the response, if larger than fragmented packet. */
+@@ -3203,9 +3208,11 @@ static bool batadv_send_my_tt_response(s
+ 			goto out;
+
+ 		/* fill the rest of the tvlv with the real TT entries */
+-		batadv_tt_tvlv_generate(bat_priv, bat_priv->tt.local_hash,
+-					tt_change, tt_len,
+-					batadv_tt_local_valid, NULL);
++		tvlv_len -= batadv_tt_tvlv_generate(bat_priv,
++						    bat_priv->tt.local_hash,
++						    tt_change, tt_len,
++						    batadv_tt_local_valid,
++						    NULL);
+ 	}
+
+ 	tvlv_tt_data->flags = BATADV_TT_RESPONSE;
diff --git a/batman-adv/patches/0005-batman-adv-Do-not-let-TT-changes-list-grows-indefini.patch b/batman-adv/patches/0005-batman-adv-Do-not-let-TT-changes-list-grows-indefini.patch
@@ -0,0 +1,63 @@
+From: Remi Pommarel <[email protected]>
+Date: Fri, 22 Nov 2024 16:52:50 +0100
+Subject: batman-adv: Do not let TT changes list grows indefinitely
+
+When TT changes list is too big to fit in packet due to MTU size, an
+empty OGM is sent expected other node to send TT request to get the
+changes. The issue is that tt.last_changeset was not built thus the
+originator was responding with previous changes to those TT requests
+(see batadv_send_my_tt_response). Also the changes list was never
+cleaned up effectively never ending growing from this point onwards,
+repeatedly sending the same TT response changes over and over, and
+creating a new empty OGM every OGM interval expecting for the local
+changes to be purged.
+
+When there is more TT changes that can fit in packet, drop all changes,
+send empty OGM and wait for TT request so we can respond with a full
+table instead.
+
+Fixes: 8405301b9794 ("batman-adv: tvlv - convert tt data sent within OGMs")
+Signed-off-by: Remi Pommarel <[email protected]>
+Acked-by: Antonio Quartulli <[email protected]>
+Signed-off-by: Sven Eckelmann <[email protected]>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/4d49d6e9d60c41a6da727108518cb8fb33295537
+
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -990,6 +990,7 @@ static void batadv_tt_tvlv_container_upd
+ 	int tt_diff_len, tt_change_len = 0;
+ 	int tt_diff_entries_num = 0;
+ 	int tt_diff_entries_count = 0;
++	bool drop_changes = false;
+ 	size_t tt_extra_len = 0;
+ 	u16 tvlv_len;
+
+@@ -997,10 +998,17 @@ static void batadv_tt_tvlv_container_upd
+ 	tt_diff_len = batadv_tt_len(tt_diff_entries_num);
+
+ 	/* if we have too many changes for one packet don't send any
+-	 * and wait for the tt table request which will be fragmented
++	 * and wait for the tt table request so we can reply with the full
++	 * (fragmented) table.
++	 *
++	 * The local change history should still be cleaned up so the next
++	 * TT round can start again with a clean state.
+ 	 */
+-	if (tt_diff_len > bat_priv->soft_iface->mtu)
++	if (tt_diff_len > bat_priv->soft_iface->mtu) {
+ 		tt_diff_len = 0;
++		tt_diff_entries_num = 0;
++		drop_changes = true;
++	}
+
+ 	tvlv_len = batadv_tt_prepare_tvlv_local_data(bat_priv, &tt_data,
+ 						     &tt_change, &tt_diff_len);
+@@ -1009,7 +1017,7 @@ static void batadv_tt_tvlv_container_upd
+
+ 	tt_data->flags = BATADV_TT_OGM_DIFF;
+
+-	if (tt_diff_len == 0)
++	if (!drop_changes && tt_diff_len == 0)
+ 		goto container_register;
+
+ 	spin_lock_bh(&bat_priv->tt.changes_list_lock);