diff mbox series

[maint,2/2] batman-adv: Fix multicast TT issues with bogus ROAM flags

Message ID 20180606224624.10141-2-linus.luessing@c0d3.blue (mailing list archive)
State Accepted, archived
Commit c7054ffae0c3b08bb4bef3cffee1e0a543e14096
Delegated to: Simon Wunderlich
Headers
Series [maint,1/2] batman-adv: Avoid storing non-TT-sync flags on singular entries too |

Commit Message

Linus Lüssing June 6, 2018, 10:46 p.m. UTC 
  When a (broken) node wrongly sends multicast TT entries with a ROAM
flag then this causes any receiving node to drop all entries for the
same multicast MAC address announced by other nodes, leading to
packet loss.

Fix this DoS vector by only storing TT sync flags. For multicast TT
non-sync'ing flag bits like ROAM are unused so far anyway.

Fixes: 405cc1e5a81e ("batman-adv: Modified forwarding behaviour for multicast packets")
Reported-by: Leonardo Mörlein <me@irrelefant.net>
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---

Fixes: https://www.open-mesh.org/issues/355

The issue reported by Leonardo was reproduceable in a small, virtual test
setup with a ROAM flag injected on multicast TT entries by one node.
---
 net/batman-adv/translation-table.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Sven Eckelmann June 12, 2018, 7:47 p.m. UTC | #1 
On Donnerstag, 7. Juni 2018 00:46:24 CEST Linus Lüssing wrote:
> When a (broken) node wrongly sends multicast TT entries with a ROAM
> flag then this causes any receiving node to drop all entries for the
> same multicast MAC address announced by other nodes, leading to
> packet loss.
> 
> Fix this DoS vector by only storing TT sync flags. For multicast TT
> non-sync'ing flag bits like ROAM are unused so far anyway.
> 
> Fixes: 405cc1e5a81e ("batman-adv: Modified forwarding behaviour for multicast packets")
> Reported-by: Leonardo Mörlein <me@irrelefant.net>
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

Added as c7054ffae0c3 [1]

Thanks,
	Sven

[1] https://git.open-mesh.org/batman-adv.git/commit/c7054ffae0c3b08bb4bef3cffee1e0a543e14096
diff mbox series

Patch

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 61ce3000..12a2b7d2 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1705,7 +1705,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 & (~BATADV_TT_SYNC_MASK);
+		if (!is_multicast_ether_addr(common->addr))
+			common->flags = flags & (~BATADV_TT_SYNC_MASK);
 
 		tt_global_entry->roam_at = 0;
 		/* node must store current time in case of roaming. This is
@@ -1769,7 +1770,8 @@  static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		 * TT_CLIENT_TEMP, therefore they have to be copied in the
 		 * client entry
 		 */
-		common->flags |= flags & (~BATADV_TT_SYNC_MASK);
+		if (!is_multicast_ether_addr(common->addr))
+			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