[B.A.T.M.A.N.] [PATCHv2 0/2] fixes for multicast counter imbalances

[B.A.T.M.A.N.] [PATCHv2 0/2] fixes for multicast counter imbalances

[Linus Lüssing]

The following two patches fix critical bugs with the counter tracking
the number of nodes with no multicast optimizations support.

Mesh networks from batman-adv version 2014.2.0 or later are potentially
affected and might kill kittens (or at least kills downloads of cat
pictures you try to do over IPv6).

Cheers, Linus


Changelog v2:

- PATCH 1/2:
  * changed author email
- PATCH 2/2:
  * new

[B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: fix counter for multicast supporting nodes

[Linus Lüssing]

A miscounting of nodes having multicast optimizations enabled can lead
to multicast packet loss in the following scenario:

If the first OGM a node receives from another one has no multicast
optimizations support (no multicast tvlv) then we are missing to
increase the counter. This potentially leads to the wrong assumption
that we could safely use multicast optimizations.

Fixings this by increasing the counter if the initial OGM has the
multicast TVLV unset, too.

Introduced by 77ec494490d60d89b42cd01d5dbf8dee04503623
("batman-adv: Announce new capability via multicast TVLV")

Reported-by: Tobias Hachmer <tobias@hachmer.de>
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 multicast.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/multicast.c b/multicast.c
index 96b66fd..02c2e0c 100644
--- a/multicast.c
+++ b/multicast.c
@@ -686,11 +686,13 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 		if (orig_initialized)
 			atomic_dec(&bat_priv->mcast.num_disabled);
 		orig->capabilities |= BATADV_ORIG_CAPA_HAS_MCAST;
-	/* If mcast support is being switched off increase the disabled
-	 * mcast node counter.
+	/* If mcast support is being switched off or if this is an initial
+	 * OGM without mcast support then increase the disabled mcast
+	 * node counter.
 	 */
 	} else if (!orig_mcast_enabled &&
-		   orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST) {
+		   (orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST ||
+		    !orig_initialized)) {
 		atomic_inc(&bat_priv->mcast.num_disabled);
 		orig->capabilities &= ~BATADV_ORIG_CAPA_HAS_MCAST;
 	}
-- 
1.7.10.4

Re: [B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: fix counter for multicast supporting nodes

[Marek Lindner]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

On Thursday 30 October 2014 05:40:46 Linus Lüssing wrote:
> A miscounting of nodes having multicast optimizations enabled can lead
> to multicast packet loss in the following scenario:
> 
> If the first OGM a node receives from another one has no multicast
> optimizations support (no multicast tvlv) then we are missing to
> increase the counter. This potentially leads to the wrong assumption
> that we could safely use multicast optimizations.
> 
> Fixings this by increasing the counter if the initial OGM has the
> multicast TVLV unset, too.
> 
> Introduced by 77ec494490d60d89b42cd01d5dbf8dee04503623
> ("batman-adv: Announce new capability via multicast TVLV")
> 
> Reported-by: Tobias Hachmer <tobias@hachmer.de>
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  multicast.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Applied in revision 33fdd2c.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: fix multicast counter when purging originators

[Linus Lüssing]

When purging an orig_node we should only decrease counter tracking the
number of nodes without multicast optimizations support if it was
increased through this orig_node before.

A not yet quite initialized orig_node (meaning it did not have its turn
in the mcast-tvlv handler so far) which gets purged would not adhere to
this and will lead to a counter imbalance.

Fixing this by adding a check whether the orig_node is mcast-initalized
before decreasing the counter in the mcast-orig_node-purging routine.

Introduced by 77ec494490d60d89b42cd01d5dbf8dee04503623
("batman-adv: Announce new capability via multicast TVLV")

Reported-by: Tobias Hachmer <tobias@hachmer.de>
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 multicast.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/multicast.c b/multicast.c
index 02c2e0c..fe57bde 100644
--- a/multicast.c
+++ b/multicast.c
@@ -741,7 +741,8 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
 {
 	struct batadv_priv *bat_priv = orig->bat_priv;
 
-	if (!(orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST))
+	if (!(orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST) &&
+	    orig->capa_initialized & BATADV_ORIG_CAPA_HAS_MCAST)
 		atomic_dec(&bat_priv->mcast.num_disabled);
 
 	batadv_mcast_want_unsnoop_update(bat_priv, orig, BATADV_NO_FLAGS);
-- 
1.7.10.4

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: fix multicast counter when purging originators

[Marek Lindner]

[-- Attachment #1: Type: text/plain, Size: 971 bytes --]

On Thursday 30 October 2014 05:40:47 Linus Lüssing wrote:
> When purging an orig_node we should only decrease counter tracking the
> number of nodes without multicast optimizations support if it was
> increased through this orig_node before.
> 
> A not yet quite initialized orig_node (meaning it did not have its turn
> in the mcast-tvlv handler so far) which gets purged would not adhere to
> this and will lead to a counter imbalance.
> 
> Fixing this by adding a check whether the orig_node is mcast-initalized
> before decreasing the counter in the mcast-orig_node-purging routine.
> 
> Introduced by 77ec494490d60d89b42cd01d5dbf8dee04503623
> ("batman-adv: Announce new capability via multicast TVLV")
> 
> Reported-by: Tobias Hachmer <tobias@hachmer.de>
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  multicast.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied in revision 9e9d48e.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]