batman-adv: Avoid infinite loop trying to resize local TT 1057/head
authorSven Eckelmann <sven@narfation.org>
Fri, 5 Apr 2024 19:55:06 +0000 (21:55 +0200)
committerSven Eckelmann <sven@narfation.org>
Fri, 5 Apr 2024 20:02:14 +0000 (22:02 +0200)
If the MTU of one of an attached interface becomes too small to transmit
the local translation table then it must be resized to fit inside all
fragments (when enabled) or a single packet.

But if the MTU becomes too low to transmit even the header + the VLAN
specific part then the resizing of the local TT will never succeed. This
can for example happen when the usable space is 110 bytes and 11 VLANs are
on top of batman-adv. In this case, at least 116 byte would be needed.
There will just be an endless spam of

   batman_adv: batadv0: Forced to purge local tt entries to fit new maximum fragment MTU (110)

in the log but the function will never finish. Problem here is that the
timeout will be halved all the time and will then stagnate at 0 and
therefore never be able to reduce the table even more.

There are other scenarios possible with a similar result. The number of
BATADV_TT_CLIENT_NOPURGE entries in the local TT can for example be too
high to fit inside a packet. Such a scenario can therefore happen also with
only a single VLAN + 7 non-purgable addresses - requiring at least 120
bytes.

While this should be handled proactively when:

* interface with too low MTU is added
* VLAN is added
* non-purgeable local mac is added
* MTU of an attached interface is reduced
* fragmentation setting gets disabled (which most likely requires dropping
  attached interfaces)

not all of these scenarios can be prevented because batman-adv is only
consuming events without the the possibility to prevent these actions
(non-purgable MAC address added, MTU of an attached interface is reduced).
It is therefore necessary to also make sure that the code is able to handle
also the situations when there were already incompatible system
configuration are present.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
batman-adv/Makefile
batman-adv/patches/0020-batman-adv-Fix-batadv_v_ogm_aggr_send-memory-leak.patch [deleted file]
batman-adv/patches/0020-batman-adv-Hold-rtnl-lock-during-MTU-update-via-netl.patch [new file with mode: 0644]
batman-adv/patches/0021-batman-adv-Avoid-infinite-loop-trying-to-resize-loca.patch [new file with mode: 0644]

index 1b9265c18defdac13427833c910a994fce516b62..c9149f3da8be7e5baeb7cc828529927d26fdbb95 100644 (file)
@@ -4,7 +4,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=batman-adv
 PKG_VERSION:=2022.0
-PKG_RELEASE:=9
+PKG_RELEASE:=10
 
 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/0020-batman-adv-Fix-batadv_v_ogm_aggr_send-memory-leak.patch b/batman-adv/patches/0020-batman-adv-Fix-batadv_v_ogm_aggr_send-memory-leak.patch
deleted file mode 100644 (file)
index 295a203..0000000
+++ /dev/null
@@ -1,35 +0,0 @@
-From: Sven Eckelmann <sven@narfation.org>
-Date: Mon, 21 Aug 2023 21:48:48 +0200
-Subject: batman-adv: Hold rtnl lock during MTU update via netlink
-
-The automatic recalculation of the maximum allowed MTU is usually triggered
-by code sections which are already rtnl lock protected by callers outside
-of batman-adv. But when the fragmentation setting is changed via
-batman-adv's own batadv genl family, then the rtnl lock is not yet taken.
-
-But dev_set_mtu requires that the caller holds the rtnl lock because it
-uses netdevice notifiers. And this code will then fail the check for this
-lock:
-
-  RTNL: assertion failed at net/core/dev.c (1953)
-
-Cc: stable@vger.kernel.org
-Reported-by: syzbot+f8812454d9b3ac00d282@syzkaller.appspotmail.com
-Fixes: 27c4d7c1c7fa ("batman-adv: Trigger events for auto adjusted MTU")
-Reviewed-by: Simon Horman <horms@kernel.org>
-Signed-off-by: Sven Eckelmann <sven@narfation.org>
-Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/aeb35331aa9a17f9affd84c1a5b020aeb4a976f4
-
---- a/net/batman-adv/netlink.c
-+++ b/net/batman-adv/netlink.c
-@@ -494,7 +494,10 @@ static int batadv_netlink_set_mesh(struc
-               attr = info->attrs[BATADV_ATTR_FRAGMENTATION_ENABLED];
-               atomic_set(&bat_priv->fragmentation, !!nla_get_u8(attr));
-+
-+              rtnl_lock();
-               batadv_update_min_mtu(bat_priv->soft_iface);
-+              rtnl_unlock();
-       }
-       if (info->attrs[BATADV_ATTR_GW_BANDWIDTH_DOWN]) {
diff --git a/batman-adv/patches/0020-batman-adv-Hold-rtnl-lock-during-MTU-update-via-netl.patch b/batman-adv/patches/0020-batman-adv-Hold-rtnl-lock-during-MTU-update-via-netl.patch
new file mode 100644 (file)
index 0000000..295a203
--- /dev/null
@@ -0,0 +1,35 @@
+From: Sven Eckelmann <sven@narfation.org>
+Date: Mon, 21 Aug 2023 21:48:48 +0200
+Subject: batman-adv: Hold rtnl lock during MTU update via netlink
+
+The automatic recalculation of the maximum allowed MTU is usually triggered
+by code sections which are already rtnl lock protected by callers outside
+of batman-adv. But when the fragmentation setting is changed via
+batman-adv's own batadv genl family, then the rtnl lock is not yet taken.
+
+But dev_set_mtu requires that the caller holds the rtnl lock because it
+uses netdevice notifiers. And this code will then fail the check for this
+lock:
+
+  RTNL: assertion failed at net/core/dev.c (1953)
+
+Cc: stable@vger.kernel.org
+Reported-by: syzbot+f8812454d9b3ac00d282@syzkaller.appspotmail.com
+Fixes: 27c4d7c1c7fa ("batman-adv: Trigger events for auto adjusted MTU")
+Reviewed-by: Simon Horman <horms@kernel.org>
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/aeb35331aa9a17f9affd84c1a5b020aeb4a976f4
+
+--- a/net/batman-adv/netlink.c
++++ b/net/batman-adv/netlink.c
+@@ -494,7 +494,10 @@ static int batadv_netlink_set_mesh(struc
+               attr = info->attrs[BATADV_ATTR_FRAGMENTATION_ENABLED];
+               atomic_set(&bat_priv->fragmentation, !!nla_get_u8(attr));
++
++              rtnl_lock();
+               batadv_update_min_mtu(bat_priv->soft_iface);
++              rtnl_unlock();
+       }
+       if (info->attrs[BATADV_ATTR_GW_BANDWIDTH_DOWN]) {
diff --git a/batman-adv/patches/0021-batman-adv-Avoid-infinite-loop-trying-to-resize-loca.patch b/batman-adv/patches/0021-batman-adv-Avoid-infinite-loop-trying-to-resize-loca.patch
new file mode 100644 (file)
index 0000000..7d4f071
--- /dev/null
@@ -0,0 +1,59 @@
+From: Sven Eckelmann <sven@narfation.org>
+Date: Mon, 12 Feb 2024 14:32:13 +0100
+Subject: batman-adv: Avoid infinite loop trying to resize local TT
+
+If the MTU of one of an attached interface becomes too small to transmit
+the local translation table then it must be resized to fit inside all
+fragments (when enabled) or a single packet.
+
+But if the MTU becomes too low to transmit even the header + the VLAN
+specific part then the resizing of the local TT will never succeed. This
+can for example happen when the usable space is 110 bytes and 11 VLANs are
+on top of batman-adv. In this case, at least 116 byte would be needed.
+There will just be an endless spam of
+
+   batman_adv: batadv0: Forced to purge local tt entries to fit new maximum fragment MTU (110)
+
+in the log but the function will never finish. Problem here is that the
+timeout will be halved all the time and will then stagnate at 0 and
+therefore never be able to reduce the table even more.
+
+There are other scenarios possible with a similar result. The number of
+BATADV_TT_CLIENT_NOPURGE entries in the local TT can for example be too
+high to fit inside a packet. Such a scenario can therefore happen also with
+only a single VLAN + 7 non-purgable addresses - requiring at least 120
+bytes.
+
+While this should be handled proactively when:
+
+* interface with too low MTU is added
+* VLAN is added
+* non-purgeable local mac is added
+* MTU of an attached interface is reduced
+* fragmentation setting gets disabled (which most likely requires dropping
+  attached interfaces)
+
+not all of these scenarios can be prevented because batman-adv is only
+consuming events without the the possibility to prevent these actions
+(non-purgable MAC address added, MTU of an attached interface is reduced).
+It is therefore necessary to also make sure that the code is able to handle
+also the situations when there were already incompatible system
+configuration are present.
+
+Cc: stable@vger.kernel.org
+Fixes: f7f2fe494388 ("batman-adv: limit local translation table max size")
+Reported-by: syzbot+a6a4b5bb3da165594cff@syzkaller.appspotmail.com
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/05f6eadbbddc834669249ae204026c383445b571
+
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -3948,7 +3948,7 @@ void batadv_tt_local_resize_to_mtu(struc
+       spin_lock_bh(&bat_priv->tt.commit_lock);
+-      while (true) {
++      while (timeout) {
+               table_size = batadv_tt_local_table_transmit_size(bat_priv);
+               if (packet_size_max >= table_size)
+                       break;