b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] pull request: batman-adv 20150811
@ 2015-08-11 16:35 Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 01/10] batman-adv: Replace gw_reselect divisor with simple shift Antonio Quartulli
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n

Hi David,

this is our first batch intended for net-next.

Here you have all those non-critical fixes/changes that we couldn't
merge into the net tree as it was already too late in the release
cycle.

This is a summary of what each patch does:

- patch 1 by Sven Eckelmann is changing the way the GW metric is
computed so that the resulting operation does not make use of
divisions and also does not lead to any data type promotion. This is
a requirement for patch 2;
- patch 2 by Ruben Wisniewski is changing the type of the variable used
in the same GW metric computation as patch 1 to uint64_t so that potential
integer overflows are prevented. Thanks to Sven's patch above no 64bit
division will be involved;
- patches 3, 4, 5 and 6 by Linus Lüssing are converting plain bitwise
operations on capability bits to set/clear/test_bit() in order to ensure
their atomicity and prevent potential race conditions;
- patch 7, also by Linus, is making the multicast TVLV parsing routine
thread-safe in order to prevent potential race conditions upon reception
of two OGMs from the same originator at the same time;
- patch 8 by Marek Lindner prevents potential double deletions
of TT Request objects from its lists which would lead to a kernel crash.
- patch 9 by Simon Wunderlich is ensuring that no enqueued packet is leaked
when an interface is deactivated;
- patch 10 by Linus Lüssing is setting the network header in the skb struct
right after a packet was delivered to the batman virtual interface so that
subsequent call to ip/ipv6_hdr() do not crash.


Please pull or let me know of any problem!

Thanks a lot David,
	Antonio


The following changes since commit 07a51cd3794960548627a27aae68c1446341db32:

  vxlan: fix fdb_dump index calculation (2015-08-10 21:15:18 -0700)

are available in the git repository at:

  git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem

for you to fetch changes up to 125300dd5850a0489b4aacf06e1424a1e34eaefa:

  batman-adv: Fix potentially broken skb network header access (2015-08-11 18:11:19 +0200)

----------------------------------------------------------------
Included changes:
- avoid integer overflow in GW selection routine
- prevent race condition by making capability bit changes atomic (use
  clear/set/test_bit)
- fix synchronization issue in mcast tvlv handler
- fix crash on double list removal of TT Request objects
- fix leak by puring packets enqueued for sending upon iface removal
- ensure network header pointer is set in skb

----------------------------------------------------------------
Linus Lüssing (6):
      batman-adv: Make DAT capability changes atomic
      batman-adv: Make NC capability changes atomic
      batman-adv: Make TT capability changes atomic
      batman-adv: Make MCAST capability changes atomic
      batman-adv: Fix potential synchronization issues in mcast tvlv handler
      batman-adv: Fix potentially broken skb network header access

Marek Lindner (1):
      batman-adv: protect tt request from double deletion

Ruben Wisniewski (1):
      batman-adv: Avoid u32 overflow during gateway select

Simon Wunderlich (1):
      batman-adv: remove broadcast packets scheduled for purged outgoing if

Sven Eckelmann (1):
      batman-adv: Replace gw_reselect divisor with simple shift

 net/batman-adv/distributed-arp-table.c |  7 +--
 net/batman-adv/gateway_client.c        |  8 +---
 net/batman-adv/multicast.c             | 81 +++++++++++++++++++++++++---------
 net/batman-adv/network-coding.c        |  7 +--
 net/batman-adv/originator.c            |  5 +++
 net/batman-adv/send.c                  |  3 +-
 net/batman-adv/soft-interface.c        |  7 ++-
 net/batman-adv/translation-table.c     | 17 ++++---
 net/batman-adv/types.h                 | 15 ++++---
 9 files changed, 102 insertions(+), 48 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 01/10] batman-adv: Replace gw_reselect divisor with simple shift
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 02/10] batman-adv: Avoid u32 overflow during gateway select Antonio Quartulli
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

The gw_factor is divided by BATADV_TQ_LOCAL_WINDOW_SIZE ** 2 * 64. But the
rest of the calculation has nothing to do with the tq window size and
therefore the calculation is just (tmp_gw_factor / (64 ** 3)).

Replace it with a simple shift to avoid a costly 64-bit divide when the
max_gw_factor is changed from u32 to u64. This type change is necessary
to avoid an overflow bug.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/gateway_client.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index bb015862..e1e1f31 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -154,14 +154,10 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 	struct batadv_neigh_ifinfo *router_ifinfo;
 	struct batadv_gw_node *gw_node, *curr_gw = NULL;
 	uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
-	uint32_t gw_divisor;
 	uint8_t max_tq = 0;
 	uint8_t tq_avg;
 	struct batadv_orig_node *orig_node;
 
-	gw_divisor = BATADV_TQ_LOCAL_WINDOW_SIZE * BATADV_TQ_LOCAL_WINDOW_SIZE;
-	gw_divisor *= 64;
-
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(gw_node, &bat_priv->gw.list, list) {
 		if (gw_node->deleted)
@@ -187,7 +183,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 			tmp_gw_factor = tq_avg * tq_avg;
 			tmp_gw_factor *= gw_node->bandwidth_down;
 			tmp_gw_factor *= 100 * 100;
-			tmp_gw_factor /= gw_divisor;
+			tmp_gw_factor >>= 18;
 
 			if ((tmp_gw_factor > max_gw_factor) ||
 			    ((tmp_gw_factor == max_gw_factor) &&
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 02/10] batman-adv: Avoid u32 overflow during gateway select
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 01/10] batman-adv: Replace gw_reselect divisor with simple shift Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic Antonio Quartulli
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem
  Cc: Ruben Wisniewsi, Marek Lindner, netdev, b.a.t.m.a.n, Antonio Quartulli

From: Ruben Wisniewski <ruben@freifunk-nrw.de>

The gateway selection based on fast connections is using a single value
calculated from the average tq (0-255) and the download bandwidth (in
100Kibit). The formula for the first step (tq ** 2 * 10000 * bandwidth)
tends to overflow a u32 with low bandwidth settings like 50 [100KiBit]
and a tq value of over 92.

Changing this to a 64 bit unsigned integer allows to support a
bandwidth_down with up to ~2.8e10 [100KiBit] and a perfect tq of 255. This
is ~6.6 times higher than the maximum possible value of the gateway
announcement TVLV.

This problem only affects the non-default gw_sel_class 1.

Signed-off-by: Ruben Wisniewsi <ruben@vfn-nrw.de>
[sven@narfation.org: rewritten commit message]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/gateway_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index e1e1f31..4ac24d8 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -153,7 +153,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 	struct batadv_neigh_node *router;
 	struct batadv_neigh_ifinfo *router_ifinfo;
 	struct batadv_gw_node *gw_node, *curr_gw = NULL;
-	uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
+	uint64_t max_gw_factor = 0, tmp_gw_factor = 0;
 	uint8_t max_tq = 0;
 	uint8_t tq_avg;
 	struct batadv_orig_node *orig_node;
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 01/10] batman-adv: Replace gw_reselect divisor with simple shift Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 02/10] batman-adv: Avoid u32 overflow during gateway select Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 19:36   ` Sergei Shtylyov
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 04/10] batman-adv: Make NC " Antonio Quartulli
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Linus Lüssing <linus.luessing@c0d3.blue>

Bitwise OR/AND assignments in C aren't guaranteed to be atomic. One
OGM handler might undo the set/clear of a specific bit from another
handler run in between.

Fix this by using the atomic set_bit()/clear_bit()/test_bit() functions.

Fixes: 17cf0ea455f1 ("batman-adv: tvlv - add distributed arp table container")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/distributed-arp-table.c | 7 ++++---
 net/batman-adv/types.h                 | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index fb54e6a..244dcf6 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -19,6 +19,7 @@
 #include "main.h"
 
 #include <linux/atomic.h>
+#include <linux/bitops.h>
 #include <linux/byteorder/generic.h>
 #include <linux/errno.h>
 #include <linux/etherdevice.h>
@@ -453,7 +454,7 @@ static bool batadv_is_orig_node_eligible(struct batadv_dat_candidate *res,
 	int j;
 
 	/* check if orig node candidate is running DAT */
-	if (!(candidate->capabilities & BATADV_ORIG_CAPA_HAS_DAT))
+	if (!(test_bit(BATADV_ORIG_CAPA_HAS_DAT, &candidate->capabilities)))
 		goto out;
 
 	/* Check if this node has already been selected... */
@@ -713,9 +714,9 @@ static void batadv_dat_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 					   uint16_t tvlv_value_len)
 {
 	if (flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND)
-		orig->capabilities &= ~BATADV_ORIG_CAPA_HAS_DAT;
+		clear_bit(BATADV_ORIG_CAPA_HAS_DAT, &orig->capabilities);
 	else
-		orig->capabilities |= BATADV_ORIG_CAPA_HAS_DAT;
+		set_bit(BATADV_ORIG_CAPA_HAS_DAT, &orig->capabilities);
 }
 
 /**
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 67d6348..29fd625 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -273,7 +273,7 @@ struct batadv_orig_node {
 	struct hlist_node mcast_want_all_ipv4_node;
 	struct hlist_node mcast_want_all_ipv6_node;
 #endif
-	uint8_t capabilities;
+	unsigned long capabilities;
 	uint8_t capa_initialized;
 	atomic_t last_ttvn;
 	unsigned char *tt_buff;
@@ -313,7 +313,7 @@ struct batadv_orig_node {
  *  (= orig node announces a tvlv of type BATADV_TVLV_MCAST)
  */
 enum batadv_orig_capabilities {
-	BATADV_ORIG_CAPA_HAS_DAT = BIT(0),
+	BATADV_ORIG_CAPA_HAS_DAT,
 	BATADV_ORIG_CAPA_HAS_NC = BIT(1),
 	BATADV_ORIG_CAPA_HAS_TT = BIT(2),
 	BATADV_ORIG_CAPA_HAS_MCAST = BIT(3),
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 04/10] batman-adv: Make NC capability changes atomic
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
                   ` (2 preceding siblings ...)
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 19:38   ` Sergei Shtylyov
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 05/10] batman-adv: Make TT " Antonio Quartulli
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Linus Lüssing <linus.luessing@c0d3.blue>

Bitwise OR/AND assignments in C aren't guaranteed to be atomic. One
OGM handler might undo the set/clear of a specific bit from another
handler run in between.

Fix this by using the atomic set_bit()/clear_bit()/test_bit() functions.

Fixes: 3f4841ffb336 ("batman-adv: tvlv - add network coding container")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/network-coding.c | 7 ++++---
 net/batman-adv/types.h          | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index f0a50f3..cfdc80d 100644
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -19,6 +19,7 @@
 #include "main.h"
 
 #include <linux/atomic.h>
+#include <linux/bitops.h>
 #include <linux/byteorder/generic.h>
 #include <linux/compiler.h>
 #include <linux/debugfs.h>
@@ -134,9 +135,9 @@ static void batadv_nc_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 					  uint16_t tvlv_value_len)
 {
 	if (flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND)
-		orig->capabilities &= ~BATADV_ORIG_CAPA_HAS_NC;
+		clear_bit(BATADV_ORIG_CAPA_HAS_NC, &orig->capabilities);
 	else
-		orig->capabilities |= BATADV_ORIG_CAPA_HAS_NC;
+		set_bit(BATADV_ORIG_CAPA_HAS_NC, &orig->capabilities);
 }
 
 /**
@@ -894,7 +895,7 @@ void batadv_nc_update_nc_node(struct batadv_priv *bat_priv,
 		goto out;
 
 	/* check if orig node is network coding enabled */
-	if (!(orig_node->capabilities & BATADV_ORIG_CAPA_HAS_NC))
+	if (!(test_bit(BATADV_ORIG_CAPA_HAS_NC, &orig_node->capabilities)))
 		goto out;
 
 	/* accept ogms from 'good' neighbors and single hop neighbors */
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 29fd625..ed4aec5 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -314,7 +314,7 @@ struct batadv_orig_node {
  */
 enum batadv_orig_capabilities {
 	BATADV_ORIG_CAPA_HAS_DAT,
-	BATADV_ORIG_CAPA_HAS_NC = BIT(1),
+	BATADV_ORIG_CAPA_HAS_NC,
 	BATADV_ORIG_CAPA_HAS_TT = BIT(2),
 	BATADV_ORIG_CAPA_HAS_MCAST = BIT(3),
 };
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 05/10] batman-adv: Make TT capability changes atomic
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
                   ` (3 preceding siblings ...)
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 04/10] batman-adv: Make NC " Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 06/10] batman-adv: Make MCAST " Antonio Quartulli
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Linus Lüssing <linus.luessing@c0d3.blue>

Bitwise OR/AND assignments in C aren't guaranteed to be atomic. One
OGM handler might undo the set/clear of a specific bit from another
handler run in between.

Fix this by using the atomic set_bit()/clear_bit()/test_bit() functions.

Fixes: e17931d1a61d ("batman-adv: introduce capability initialization bitfield")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/translation-table.c | 8 +++++---
 net/batman-adv/types.h             | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index b482495..1573489 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -19,6 +19,7 @@
 #include "main.h"
 
 #include <linux/atomic.h>
+#include <linux/bitops.h>
 #include <linux/bug.h>
 #include <linux/byteorder/generic.h>
 #include <linux/compiler.h>
@@ -1862,7 +1863,7 @@ void batadv_tt_global_del_orig(struct batadv_priv *bat_priv,
 		}
 		spin_unlock_bh(list_lock);
 	}
-	orig_node->capa_initialized &= ~BATADV_ORIG_CAPA_HAS_TT;
+	clear_bit(BATADV_ORIG_CAPA_HAS_TT, &orig_node->capa_initialized);
 }
 
 static bool batadv_tt_global_to_purge(struct batadv_tt_global_entry *tt_global,
@@ -2821,7 +2822,7 @@ static void _batadv_tt_update_changes(struct batadv_priv *bat_priv,
 				return;
 		}
 	}
-	orig_node->capa_initialized |= BATADV_ORIG_CAPA_HAS_TT;
+	set_bit(BATADV_ORIG_CAPA_HAS_TT, &orig_node->capa_initialized);
 }
 
 static void batadv_tt_fill_gtable(struct batadv_priv *bat_priv,
@@ -3321,7 +3322,8 @@ static void batadv_tt_update_orig(struct batadv_priv *bat_priv,
 	bool has_tt_init;
 
 	tt_vlan = (struct batadv_tvlv_tt_vlan_data *)tt_buff;
-	has_tt_init = orig_node->capa_initialized & BATADV_ORIG_CAPA_HAS_TT;
+	has_tt_init = test_bit(BATADV_ORIG_CAPA_HAS_TT,
+			       &orig_node->capa_initialized);
 
 	/* orig table not initialised AND first diff is in the OGM OR the ttvn
 	 * increased by one -> we can apply the attached changes
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index ed4aec5..6f801ef 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -274,7 +274,7 @@ struct batadv_orig_node {
 	struct hlist_node mcast_want_all_ipv6_node;
 #endif
 	unsigned long capabilities;
-	uint8_t capa_initialized;
+	unsigned long capa_initialized;
 	atomic_t last_ttvn;
 	unsigned char *tt_buff;
 	int16_t tt_buff_len;
@@ -315,7 +315,7 @@ struct batadv_orig_node {
 enum batadv_orig_capabilities {
 	BATADV_ORIG_CAPA_HAS_DAT,
 	BATADV_ORIG_CAPA_HAS_NC,
-	BATADV_ORIG_CAPA_HAS_TT = BIT(2),
+	BATADV_ORIG_CAPA_HAS_TT,
 	BATADV_ORIG_CAPA_HAS_MCAST = BIT(3),
 };
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 06/10] batman-adv: Make MCAST capability changes atomic
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
                   ` (4 preceding siblings ...)
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 05/10] batman-adv: Make TT " Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 07/10] batman-adv: Fix potential synchronization issues in mcast tvlv handler Antonio Quartulli
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Linus Lüssing <linus.luessing@c0d3.blue>

Bitwise OR/AND assignments in C aren't guaranteed to be atomic. One
OGM handler might undo the set/clear of a specific bit from another
handler run in between.

Fix this by using the atomic set_bit()/clear_bit()/test_bit() functions.

Fixes: 60432d756cf0 ("batman-adv: Announce new capability via multicast TVLV")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/multicast.c | 18 ++++++++++--------
 net/batman-adv/types.h     |  2 +-
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 7aa480b..f92473d 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -19,6 +19,7 @@
 #include "main.h"
 
 #include <linux/atomic.h>
+#include <linux/bitops.h>
 #include <linux/byteorder/generic.h>
 #include <linux/errno.h>
 #include <linux/etherdevice.h>
@@ -697,29 +698,30 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 	uint8_t mcast_flags = BATADV_NO_FLAGS;
 	bool orig_initialized;
 
-	orig_initialized = orig->capa_initialized & BATADV_ORIG_CAPA_HAS_MCAST;
+	orig_initialized = test_bit(BATADV_ORIG_CAPA_HAS_MCAST,
+				    &orig->capa_initialized);
 
 	/* If mcast support is turned on decrease the disabled mcast node
 	 * counter only if we had increased it for this node before. If this
 	 * is a completely new orig_node no need to decrease the counter.
 	 */
 	if (orig_mcast_enabled &&
-	    !(orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST)) {
+	    !(test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities))) {
 		if (orig_initialized)
 			atomic_dec(&bat_priv->mcast.num_disabled);
-		orig->capabilities |= BATADV_ORIG_CAPA_HAS_MCAST;
+		set_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities);
 	/* 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 ||
+		   (test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities) ||
 		    !orig_initialized)) {
 		atomic_inc(&bat_priv->mcast.num_disabled);
-		orig->capabilities &= ~BATADV_ORIG_CAPA_HAS_MCAST;
+		clear_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities);
 	}
 
-	orig->capa_initialized |= BATADV_ORIG_CAPA_HAS_MCAST;
+	set_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized);
 
 	if (orig_mcast_enabled && tvlv_value &&
 	    (tvlv_value_len >= sizeof(mcast_flags)))
@@ -763,8 +765,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) &&
-	    orig->capa_initialized & BATADV_ORIG_CAPA_HAS_MCAST)
+	if (!(test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities)) &&
+	    test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized))
 		atomic_dec(&bat_priv->mcast.num_disabled);
 
 	batadv_mcast_want_unsnoop_update(bat_priv, orig, BATADV_NO_FLAGS);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 6f801ef..1eeed18 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -316,7 +316,7 @@ enum batadv_orig_capabilities {
 	BATADV_ORIG_CAPA_HAS_DAT,
 	BATADV_ORIG_CAPA_HAS_NC,
 	BATADV_ORIG_CAPA_HAS_TT,
-	BATADV_ORIG_CAPA_HAS_MCAST = BIT(3),
+	BATADV_ORIG_CAPA_HAS_MCAST,
 };
 
 /**
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 07/10] batman-adv: Fix potential synchronization issues in mcast tvlv handler
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
                   ` (5 preceding siblings ...)
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 06/10] batman-adv: Make MCAST " Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 08/10] batman-adv: protect tt request from double deletion Antonio Quartulli
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Linus Lüssing <linus.luessing@c0d3.blue>

So far the mcast tvlv handler did not anticipate the processing of
multiple incoming OGMs from the same originator at the same time. This
can lead to various issues:

* Broken refcounting: For instance two mcast handlers might both assume
  that an originator just got multicast capabilities and will together
  wrongly decrease mcast.num_disabled by two, potentially leading to
  an integer underflow.

* Potential kernel panic on hlist_del_rcu(): Two mcast handlers might
  one after another try to do an
  hlist_del_rcu(&orig->mcast_want_all_*_node). The second one will
  cause memory corruption / crashes.
  (Reported by: Sven Eckelmann <sven@narfation.org>)

Right in the beginning the code path makes assumptions about the current
multicast related state of an originator and bases all updates on that. The
easiest and least error prune way to fix the issues in this case is to
serialize multiple mcast handler invocations with a spinlock.

Fixes: 60432d756cf0 ("batman-adv: Announce new capability via multicast TVLV")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/multicast.c  | 63 +++++++++++++++++++++++++++++++++++----------
 net/batman-adv/originator.c |  5 ++++
 net/batman-adv/types.h      |  3 +++
 3 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index f92473d..7d2b1f8 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -20,6 +20,7 @@
 
 #include <linux/atomic.h>
 #include <linux/bitops.h>
+#include <linux/bug.h>
 #include <linux/byteorder/generic.h>
 #include <linux/errno.h>
 #include <linux/etherdevice.h>
@@ -589,19 +590,26 @@ batadv_mcast_forw_mode(struct batadv_priv *bat_priv, struct sk_buff *skb,
  *
  * If the BATADV_MCAST_WANT_ALL_UNSNOOPABLES flag of this originator,
  * orig, has toggled then this method updates counter and list accordingly.
+ *
+ * Caller needs to hold orig->mcast_handler_lock.
  */
 static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
 					     struct batadv_orig_node *orig,
 					     uint8_t mcast_flags)
 {
+	struct hlist_node *node = &orig->mcast_want_all_unsnoopables_node;
+	struct hlist_head *head = &bat_priv->mcast.want_all_unsnoopables_list;
+
 	/* switched from flag unset to set */
 	if (mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
 	    !(orig->mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES)) {
 		atomic_inc(&bat_priv->mcast.num_want_all_unsnoopables);
 
 		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
-		hlist_add_head_rcu(&orig->mcast_want_all_unsnoopables_node,
-				   &bat_priv->mcast.want_all_unsnoopables_list);
+		/* flag checks above + mcast_handler_lock prevents this */
+		WARN_ON(!hlist_unhashed(node));
+
+		hlist_add_head_rcu(node, head);
 		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
 	/* switched from flag set to unset */
 	} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES) &&
@@ -609,7 +617,10 @@ static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
 		atomic_dec(&bat_priv->mcast.num_want_all_unsnoopables);
 
 		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
-		hlist_del_rcu(&orig->mcast_want_all_unsnoopables_node);
+		/* flag checks above + mcast_handler_lock prevents this */
+		WARN_ON(hlist_unhashed(node));
+
+		hlist_del_init_rcu(node);
 		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
 	}
 }
@@ -622,19 +633,26 @@ static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
  *
  * If the BATADV_MCAST_WANT_ALL_IPV4 flag of this originator, orig, has
  * toggled then this method updates counter and list accordingly.
+ *
+ * Caller needs to hold orig->mcast_handler_lock.
  */
 static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
 					  struct batadv_orig_node *orig,
 					  uint8_t mcast_flags)
 {
+	struct hlist_node *node = &orig->mcast_want_all_ipv4_node;
+	struct hlist_head *head = &bat_priv->mcast.want_all_ipv4_list;
+
 	/* switched from flag unset to set */
 	if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV4 &&
 	    !(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV4)) {
 		atomic_inc(&bat_priv->mcast.num_want_all_ipv4);
 
 		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
-		hlist_add_head_rcu(&orig->mcast_want_all_ipv4_node,
-				   &bat_priv->mcast.want_all_ipv4_list);
+		/* flag checks above + mcast_handler_lock prevents this */
+		WARN_ON(!hlist_unhashed(node));
+
+		hlist_add_head_rcu(node, head);
 		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
 	/* switched from flag set to unset */
 	} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV4) &&
@@ -642,7 +660,10 @@ static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
 		atomic_dec(&bat_priv->mcast.num_want_all_ipv4);
 
 		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
-		hlist_del_rcu(&orig->mcast_want_all_ipv4_node);
+		/* flag checks above + mcast_handler_lock prevents this */
+		WARN_ON(hlist_unhashed(node));
+
+		hlist_del_init_rcu(node);
 		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
 	}
 }
@@ -655,19 +676,26 @@ static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
  *
  * If the BATADV_MCAST_WANT_ALL_IPV6 flag of this originator, orig, has
  * toggled then this method updates counter and list accordingly.
+ *
+ * Caller needs to hold orig->mcast_handler_lock.
  */
 static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
 					  struct batadv_orig_node *orig,
 					  uint8_t mcast_flags)
 {
+	struct hlist_node *node = &orig->mcast_want_all_ipv6_node;
+	struct hlist_head *head = &bat_priv->mcast.want_all_ipv6_list;
+
 	/* switched from flag unset to set */
 	if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV6 &&
 	    !(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV6)) {
 		atomic_inc(&bat_priv->mcast.num_want_all_ipv6);
 
 		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
-		hlist_add_head_rcu(&orig->mcast_want_all_ipv6_node,
-				   &bat_priv->mcast.want_all_ipv6_list);
+		/* flag checks above + mcast_handler_lock prevents this */
+		WARN_ON(!hlist_unhashed(node));
+
+		hlist_add_head_rcu(node, head);
 		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
 	/* switched from flag set to unset */
 	} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV6) &&
@@ -675,7 +703,10 @@ static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
 		atomic_dec(&bat_priv->mcast.num_want_all_ipv6);
 
 		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
-		hlist_del_rcu(&orig->mcast_want_all_ipv6_node);
+		/* flag checks above + mcast_handler_lock prevents this */
+		WARN_ON(hlist_unhashed(node));
+
+		hlist_del_init_rcu(node);
 		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
 	}
 }
@@ -698,6 +729,11 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 	uint8_t mcast_flags = BATADV_NO_FLAGS;
 	bool orig_initialized;
 
+	if (orig_mcast_enabled && tvlv_value &&
+	    (tvlv_value_len >= sizeof(mcast_flags)))
+		mcast_flags = *(uint8_t *)tvlv_value;
+
+	spin_lock_bh(&orig->mcast_handler_lock);
 	orig_initialized = test_bit(BATADV_ORIG_CAPA_HAS_MCAST,
 				    &orig->capa_initialized);
 
@@ -723,15 +759,12 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 
 	set_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized);
 
-	if (orig_mcast_enabled && tvlv_value &&
-	    (tvlv_value_len >= sizeof(mcast_flags)))
-		mcast_flags = *(uint8_t *)tvlv_value;
-
 	batadv_mcast_want_unsnoop_update(bat_priv, orig, mcast_flags);
 	batadv_mcast_want_ipv4_update(bat_priv, orig, mcast_flags);
 	batadv_mcast_want_ipv6_update(bat_priv, orig, mcast_flags);
 
 	orig->mcast_flags = mcast_flags;
+	spin_unlock_bh(&orig->mcast_handler_lock);
 }
 
 /**
@@ -765,6 +798,8 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
 {
 	struct batadv_priv *bat_priv = orig->bat_priv;
 
+	spin_lock_bh(&orig->mcast_handler_lock);
+
 	if (!(test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities)) &&
 	    test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized))
 		atomic_dec(&bat_priv->mcast.num_disabled);
@@ -772,4 +807,6 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
 	batadv_mcast_want_unsnoop_update(bat_priv, orig, BATADV_NO_FLAGS);
 	batadv_mcast_want_ipv4_update(bat_priv, orig, BATADV_NO_FLAGS);
 	batadv_mcast_want_ipv6_update(bat_priv, orig, BATADV_NO_FLAGS);
+
+	spin_unlock_bh(&orig->mcast_handler_lock);
 }
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index 018b749..32a0fcf 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -696,8 +696,13 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
 	orig_node->last_seen = jiffies;
 	reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
 	orig_node->bcast_seqno_reset = reset_time;
+
 #ifdef CONFIG_BATMAN_ADV_MCAST
 	orig_node->mcast_flags = BATADV_NO_FLAGS;
+	INIT_HLIST_NODE(&orig_node->mcast_want_all_unsnoopables_node);
+	INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv4_node);
+	INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv6_node);
+	spin_lock_init(&orig_node->mcast_handler_lock);
 #endif
 
 	/* create a vlan object for the "untagged" LAN */
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 1eeed18..55610a8 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -221,6 +221,7 @@ struct batadv_orig_bat_iv {
  * @batadv_dat_addr_t:  address of the orig node in the distributed hash
  * @last_seen: time when last packet from this node was received
  * @bcast_seqno_reset: time when the broadcast seqno window was reset
+ * @mcast_handler_lock: synchronizes mcast-capability and -flag changes
  * @mcast_flags: multicast flags announced by the orig node
  * @mcast_want_all_unsnoop_node: a list node for the
  *  mcast.want_all_unsnoopables list
@@ -268,6 +269,8 @@ struct batadv_orig_node {
 	unsigned long last_seen;
 	unsigned long bcast_seqno_reset;
 #ifdef CONFIG_BATMAN_ADV_MCAST
+	/* synchronizes mcast tvlv specific orig changes */
+	spinlock_t mcast_handler_lock;
 	uint8_t mcast_flags;
 	struct hlist_node mcast_want_all_unsnoopables_node;
 	struct hlist_node mcast_want_all_ipv4_node;
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 08/10] batman-adv: protect tt request from double deletion
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
                   ` (6 preceding siblings ...)
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 07/10] batman-adv: Fix potential synchronization issues in mcast tvlv handler Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 09/10] batman-adv: remove broadcast packets scheduled for purged outgoing if Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 10/10] batman-adv: Fix potentially broken skb network header access Antonio Quartulli
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner, Antonio Quartulli

From: Marek Lindner <mareklindner@neomailbox.ch>

The list_del() calls were changed to list_del_init() to prevent
an accidental double deletion in batadv_tt_req_node_new().

Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/translation-table.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 1573489..cd35bb8 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -2196,7 +2196,7 @@ static void batadv_tt_req_list_free(struct batadv_priv *bat_priv)
 	spin_lock_bh(&bat_priv->tt.req_list_lock);
 
 	list_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) {
-		list_del(&node->list);
+		list_del_init(&node->list);
 		kfree(node);
 	}
 
@@ -2232,7 +2232,7 @@ static void batadv_tt_req_purge(struct batadv_priv *bat_priv)
 	list_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) {
 		if (batadv_has_timed_out(node->issued_at,
 					 BATADV_TT_REQUEST_TIMEOUT)) {
-			list_del(&node->list);
+			list_del_init(&node->list);
 			kfree(node);
 		}
 	}
@@ -2514,7 +2514,8 @@ out:
 		batadv_hardif_free_ref(primary_if);
 	if (ret && tt_req_node) {
 		spin_lock_bh(&bat_priv->tt.req_list_lock);
-		list_del(&tt_req_node->list);
+		/* list_del_init() verifies tt_req_node still is in the list */
+		list_del_init(&tt_req_node->list);
 		spin_unlock_bh(&bat_priv->tt.req_list_lock);
 		kfree(tt_req_node);
 	}
@@ -2951,7 +2952,7 @@ static void batadv_handle_tt_response(struct batadv_priv *bat_priv,
 	list_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) {
 		if (!batadv_compare_eth(node->addr, resp_src))
 			continue;
-		list_del(&node->list);
+		list_del_init(&node->list);
 		kfree(node);
 	}
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 09/10] batman-adv: remove broadcast packets scheduled for purged outgoing if
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
                   ` (7 preceding siblings ...)
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 08/10] batman-adv: protect tt request from double deletion Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 10/10] batman-adv: Fix potentially broken skb network header access Antonio Quartulli
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem
  Cc: netdev, Simon Wunderlich, b.a.t.m.a.n, Marek Lindner, Antonio Quartulli

From: Simon Wunderlich <simon@open-mesh.com>

When an interface is purged, the broadcast packets scheduled for this
interface should get purged as well.

Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/send.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index 0a01992..191076e 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -616,7 +616,8 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
 		 * we delete only packets belonging to the given interface
 		 */
 		if ((hard_iface) &&
-		    (forw_packet->if_incoming != hard_iface))
+		    (forw_packet->if_incoming != hard_iface) &&
+		    (forw_packet->if_outgoing != hard_iface))
 			continue;
 
 		spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 10/10] batman-adv: Fix potentially broken skb network header access
  2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
                   ` (8 preceding siblings ...)
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 09/10] batman-adv: remove broadcast packets scheduled for purged outgoing if Antonio Quartulli
@ 2015-08-11 16:35 ` Antonio Quartulli
  9 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-11 16:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Linus Lüssing <linus.luessing@c0d3.blue>

The two commits noted below added calls to ip_hdr() and ipv6_hdr(). They
need a correctly set skb network header.

Unfortunately we cannot rely on the device drivers to set it for us.
Therefore setting it in the beginning of the according ndo_start_xmit
handler.

Fixes: 1d8ab8d3c176 ("batman-adv: Modified forwarding behaviour for multicast packets")
Fixes: ab49886e3da7 ("batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/soft-interface.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index c002961..926292d 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -202,6 +202,7 @@ static int batadv_interface_tx(struct sk_buff *skb,
 	int gw_mode;
 	enum batadv_forw_mode forw_mode;
 	struct batadv_orig_node *mcast_single_orig = NULL;
+	int network_offset = ETH_HLEN;
 
 	if (atomic_read(&bat_priv->mesh_state) != BATADV_MESH_ACTIVE)
 		goto dropped;
@@ -214,14 +215,18 @@ static int batadv_interface_tx(struct sk_buff *skb,
 	case ETH_P_8021Q:
 		vhdr = vlan_eth_hdr(skb);
 
-		if (vhdr->h_vlan_encapsulated_proto != ethertype)
+		if (vhdr->h_vlan_encapsulated_proto != ethertype) {
+			network_offset += VLAN_HLEN;
 			break;
+		}
 
 		/* fall through */
 	case ETH_P_BATMAN:
 		goto dropped;
 	}
 
+	skb_set_network_header(skb, network_offset);
+
 	if (batadv_bla_tx(bat_priv, skb, vid))
 		goto dropped;
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic Antonio Quartulli
@ 2015-08-11 19:36   ` Sergei Shtylyov
  2015-08-14 20:49     ` Antonio Quartulli
  0 siblings, 1 reply; 15+ messages in thread
From: Sergei Shtylyov @ 2015-08-11 19:36 UTC (permalink / raw)
  To: Antonio Quartulli, davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

Hello.

On 08/11/2015 07:35 PM, Antonio Quartulli wrote:

> From: Linus Lüssing <linus.luessing@c0d3.blue>

> Bitwise OR/AND assignments in C aren't guaranteed to be atomic. One
> OGM handler might undo the set/clear of a specific bit from another
> handler run in between.

> Fix this by using the atomic set_bit()/clear_bit()/test_bit() functions.

> Fixes: 17cf0ea455f1 ("batman-adv: tvlv - add distributed arp table container")
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
> Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
> ---
>   net/batman-adv/distributed-arp-table.c | 7 ++++---
>   net/batman-adv/types.h                 | 4 ++--
>   2 files changed, 6 insertions(+), 5 deletions(-)

> diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
> index fb54e6a..244dcf6 100644
> --- a/net/batman-adv/distributed-arp-table.c
> +++ b/net/batman-adv/distributed-arp-table.c
[...]
> @@ -453,7 +454,7 @@ static bool batadv_is_orig_node_eligible(struct batadv_dat_candidate *res,
>   	int j;
>
>   	/* check if orig node candidate is running DAT */
> -	if (!(candidate->capabilities & BATADV_ORIG_CAPA_HAS_DAT))
> +	if (!(test_bit(BATADV_ORIG_CAPA_HAS_DAT, &candidate->capabilities)))

    () around the tst_bit() call not needed.

[...]

MBR, Sergei


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 04/10] batman-adv: Make NC capability changes atomic
  2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 04/10] batman-adv: Make NC " Antonio Quartulli
@ 2015-08-11 19:38   ` Sergei Shtylyov
  0 siblings, 0 replies; 15+ messages in thread
From: Sergei Shtylyov @ 2015-08-11 19:38 UTC (permalink / raw)
  To: Antonio Quartulli, davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

On 08/11/2015 07:35 PM, Antonio Quartulli wrote:

> From: Linus Lüssing <linus.luessing@c0d3.blue>
>
> Bitwise OR/AND assignments in C aren't guaranteed to be atomic. One
> OGM handler might undo the set/clear of a specific bit from another
> handler run in between.
>
> Fix this by using the atomic set_bit()/clear_bit()/test_bit() functions.
>
> Fixes: 3f4841ffb336 ("batman-adv: tvlv - add network coding container")
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
> Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
> ---
>   net/batman-adv/network-coding.c | 7 ++++---
>   net/batman-adv/types.h          | 2 +-
>   2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
> index f0a50f3..cfdc80d 100644
> --- a/net/batman-adv/network-coding.c
> +++ b/net/batman-adv/network-coding.c
[...]
> @@ -894,7 +895,7 @@ void batadv_nc_update_nc_node(struct batadv_priv *bat_priv,
>   		goto out;
>
>   	/* check if orig node is network coding enabled */
> -	if (!(orig_node->capabilities & BATADV_ORIG_CAPA_HAS_NC))
> +	if (!(test_bit(BATADV_ORIG_CAPA_HAS_NC, &orig_node->capabilities)))

    Likewise, () around the call no needed.

[...]

MBR, Sergei


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic
  2015-08-11 19:36   ` Sergei Shtylyov
@ 2015-08-14 20:49     ` Antonio Quartulli
  0 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-14 20:49 UTC (permalink / raw)
  To: sergei.shtylyov
  Cc: netdev,
	The list for a Better Approach To Mobile Ad-hoc Networking,
	davem, Marek Lindner

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



On 11/08/15 21:36, Sergei Shtylyov wrote:
>>       /* check if orig node candidate is running DAT */
>> -    if (!(candidate->capabilities & BATADV_ORIG_CAPA_HAS_DAT))
>> +    if (!(test_bit(BATADV_ORIG_CAPA_HAS_DAT, &candidate->capabilities)))
> 
>    () around the tst_bit() call not needed.

Thanks for the hint Sergei. Even if I don't remember having seen any
complaint from checkpatch.pl about this.

I'll resend the pull request with this fixed patches.

Cheers,


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic
  2015-08-14 20:57 [B.A.T.M.A.N.] pull request: batman-adv 20150814 Antonio Quartulli
@ 2015-08-14 20:57 ` Antonio Quartulli
  0 siblings, 0 replies; 15+ messages in thread
From: Antonio Quartulli @ 2015-08-14 20:57 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

From: Linus Lüssing <linus.luessing@c0d3.blue>

Bitwise OR/AND assignments in C aren't guaranteed to be atomic. One
OGM handler might undo the set/clear of a specific bit from another
handler run in between.

Fix this by using the atomic set_bit()/clear_bit()/test_bit() functions.

Fixes: 17cf0ea455f1 ("batman-adv: tvlv - add distributed arp table container")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/distributed-arp-table.c | 7 ++++---
 net/batman-adv/types.h                 | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index fb54e6a..1cfba20 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -19,6 +19,7 @@
 #include "main.h"
 
 #include <linux/atomic.h>
+#include <linux/bitops.h>
 #include <linux/byteorder/generic.h>
 #include <linux/errno.h>
 #include <linux/etherdevice.h>
@@ -453,7 +454,7 @@ static bool batadv_is_orig_node_eligible(struct batadv_dat_candidate *res,
 	int j;
 
 	/* check if orig node candidate is running DAT */
-	if (!(candidate->capabilities & BATADV_ORIG_CAPA_HAS_DAT))
+	if (!test_bit(BATADV_ORIG_CAPA_HAS_DAT, &candidate->capabilities))
 		goto out;
 
 	/* Check if this node has already been selected... */
@@ -713,9 +714,9 @@ static void batadv_dat_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 					   uint16_t tvlv_value_len)
 {
 	if (flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND)
-		orig->capabilities &= ~BATADV_ORIG_CAPA_HAS_DAT;
+		clear_bit(BATADV_ORIG_CAPA_HAS_DAT, &orig->capabilities);
 	else
-		orig->capabilities |= BATADV_ORIG_CAPA_HAS_DAT;
+		set_bit(BATADV_ORIG_CAPA_HAS_DAT, &orig->capabilities);
 }
 
 /**
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 67d6348..29fd625 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -273,7 +273,7 @@ struct batadv_orig_node {
 	struct hlist_node mcast_want_all_ipv4_node;
 	struct hlist_node mcast_want_all_ipv6_node;
 #endif
-	uint8_t capabilities;
+	unsigned long capabilities;
 	uint8_t capa_initialized;
 	atomic_t last_ttvn;
 	unsigned char *tt_buff;
@@ -313,7 +313,7 @@ struct batadv_orig_node {
  *  (= orig node announces a tvlv of type BATADV_TVLV_MCAST)
  */
 enum batadv_orig_capabilities {
-	BATADV_ORIG_CAPA_HAS_DAT = BIT(0),
+	BATADV_ORIG_CAPA_HAS_DAT,
 	BATADV_ORIG_CAPA_HAS_NC = BIT(1),
 	BATADV_ORIG_CAPA_HAS_TT = BIT(2),
 	BATADV_ORIG_CAPA_HAS_MCAST = BIT(3),
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-08-14 20:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-11 16:35 [B.A.T.M.A.N.] pull request: batman-adv 20150811 Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 01/10] batman-adv: Replace gw_reselect divisor with simple shift Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 02/10] batman-adv: Avoid u32 overflow during gateway select Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic Antonio Quartulli
2015-08-11 19:36   ` Sergei Shtylyov
2015-08-14 20:49     ` Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 04/10] batman-adv: Make NC " Antonio Quartulli
2015-08-11 19:38   ` Sergei Shtylyov
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 05/10] batman-adv: Make TT " Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 06/10] batman-adv: Make MCAST " Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 07/10] batman-adv: Fix potential synchronization issues in mcast tvlv handler Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 08/10] batman-adv: protect tt request from double deletion Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 09/10] batman-adv: remove broadcast packets scheduled for purged outgoing if Antonio Quartulli
2015-08-11 16:35 ` [B.A.T.M.A.N.] [PATCH 10/10] batman-adv: Fix potentially broken skb network header access Antonio Quartulli
2015-08-14 20:57 [B.A.T.M.A.N.] pull request: batman-adv 20150814 Antonio Quartulli
2015-08-14 20:57 ` [B.A.T.M.A.N.] [PATCH 03/10] batman-adv: Make DAT capability changes atomic Antonio Quartulli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).