pull request [net]: batman-adv 20160114

pull request [net]: batman-adv 20160114

[Antonio Quartulli]

Hello David,

these are two small fixes intended for your net tree.

Patch 1 prevents a lockdep splat from appearing along the
shutdown path due to a missing lock in mcast_free(), by Simon Wunderlich.

Patch 2 fixes a new feature introduced in 4.5 by ensuring that a
batadv_hardif_neigh_node object is free'd only when it is no
longer used in any context, by Sven Eckelmann.

Please pull or let me know of any issue.
Thanks a lot,
	Antonio

The following changes since commit f1640c3ddeec12804bc9a21feee85fc15aca95f6:

  bgmac: fix a missing check for build_skb (2016-01-13 00:24:14 -0500)

are available in the git repository at:

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

for you to fetch changes up to bab7c6c3deac70966a3000402c0ea6d0c20edd15:

  batman-adv: Fix list removal of batadv_hardif_neigh_node (2016-01-13 19:28:27 +0800)

----------------------------------------------------------------
Included bugfixes:
- avoid freeing batadv_hardif_neigh_node when still in use in other contexts
- prevent lockdep splat in mcast_free during shutdown

----------------------------------------------------------------
Simon Wunderlich (1):
      batman-adv: fix lockdep splat when doing mcast_free

Sven Eckelmann (1):
      batman-adv: Fix list removal of batadv_hardif_neigh_node

 net/batman-adv/multicast.c  |  2 ++
 net/batman-adv/originator.c | 18 ++++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

[PATCH 1/2] batman-adv: fix lockdep splat when doing mcast_free

[Antonio Quartulli]

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

While testing, we got something like this:

WARNING: CPU: 0 PID: 238 at net/batman-adv/multicast.c:142 batadv_mcast_mla_tt_retract+0x94/0x205 [batman_adv]()
[...]
Call Trace:
[<ffffffff815fc597>] dump_stack+0x4b/0x64
[<ffffffff810b34dc>] warn_slowpath_common+0xbc/0x120
[<ffffffffa0024ec5>] ? batadv_mcast_mla_tt_retract+0x94/0x205 [batman_adv]
[<ffffffff810b3705>] warn_slowpath_null+0x15/0x20
[<ffffffffa0024ec5>] batadv_mcast_mla_tt_retract+0x94/0x205 [batman_adv]
[<ffffffffa00273fe>] batadv_mcast_free+0x36/0x39 [batman_adv]
[<ffffffffa0020c77>] batadv_mesh_free+0x7d/0x13f [batman_adv]
[<ffffffffa0036a6b>] batadv_softif_free+0x15/0x25 [batman_adv]
[...]

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

diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index eb76386f8d4b..75fa5013af72 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -802,7 +802,9 @@ void batadv_mcast_free(struct batadv_priv *bat_priv)
 	batadv_tvlv_container_unregister(bat_priv, BATADV_TVLV_MCAST, 1);
 	batadv_tvlv_handler_unregister(bat_priv, BATADV_TVLV_MCAST, 1);
 
+	spin_lock_bh(&bat_priv->tt.commit_lock);
 	batadv_mcast_mla_tt_retract(bat_priv, NULL);
+	spin_unlock_bh(&bat_priv->tt.commit_lock);
 }
 
 /**
-- 
2.7.0

[PATCH 2/2] batman-adv: Fix list removal of batadv_hardif_neigh_node

[Antonio Quartulli]

From: Sven Eckelmann <sven@narfation.org>

The neigh_list with batadv_hardif_neigh_node objects is accessed with only
rcu_read_lock in batadv_hardif_neigh_get and batadv_iv_neigh_print. Thus it
is not allowed to kfree the object before the rcu grace period ends (which
may still protects context accessing this object). Therefore the object has
first to be removed from the neigh_list and then it has either wait with
synchronize_rcu or call_rcu till the grace period ends before it can be
freed.

Fixes: cef63419f7db ("batman-adv: add list of unique single hop neighbors per hard-interface")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 net/batman-adv/originator.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index 3c782a33bdac..ae6d18cafc5a 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -211,10 +211,6 @@ static void batadv_hardif_neigh_free_rcu(struct rcu_head *rcu)
 
 	hardif_neigh = container_of(rcu, struct batadv_hardif_neigh_node, rcu);
 
-	spin_lock_bh(&hardif_neigh->if_incoming->neigh_list_lock);
-	hlist_del_init_rcu(&hardif_neigh->list);
-	spin_unlock_bh(&hardif_neigh->if_incoming->neigh_list_lock);
-
 	batadv_hardif_free_ref_now(hardif_neigh->if_incoming);
 	kfree(hardif_neigh);
 }
@@ -227,8 +223,13 @@ static void batadv_hardif_neigh_free_rcu(struct rcu_head *rcu)
 static void
 batadv_hardif_neigh_free_now(struct batadv_hardif_neigh_node *hardif_neigh)
 {
-	if (atomic_dec_and_test(&hardif_neigh->refcount))
+	if (atomic_dec_and_test(&hardif_neigh->refcount)) {
+		spin_lock_bh(&hardif_neigh->if_incoming->neigh_list_lock);
+		hlist_del_init_rcu(&hardif_neigh->list);
+		spin_unlock_bh(&hardif_neigh->if_incoming->neigh_list_lock);
+
 		batadv_hardif_neigh_free_rcu(&hardif_neigh->rcu);
+	}
 }
 
 /**
@@ -238,8 +239,13 @@ batadv_hardif_neigh_free_now(struct batadv_hardif_neigh_node *hardif_neigh)
  */
 void batadv_hardif_neigh_free_ref(struct batadv_hardif_neigh_node *hardif_neigh)
 {
-	if (atomic_dec_and_test(&hardif_neigh->refcount))
+	if (atomic_dec_and_test(&hardif_neigh->refcount)) {
+		spin_lock_bh(&hardif_neigh->if_incoming->neigh_list_lock);
+		hlist_del_init_rcu(&hardif_neigh->list);
+		spin_unlock_bh(&hardif_neigh->if_incoming->neigh_list_lock);
+
 		call_rcu(&hardif_neigh->rcu, batadv_hardif_neigh_free_rcu);
+	}
 }
 
 /**
-- 
2.7.0

Re: pull request [net]: batman-adv 20160114

[David Miller]

From: Antonio Quartulli <a@unstable.cc>
Date: Thu, 14 Jan 2016 01:51:52 +0800

> these are two small fixes intended for your net tree.
> 
> Patch 1 prevents a lockdep splat from appearing along the
> shutdown path due to a missing lock in mcast_free(), by Simon Wunderlich.
> 
> Patch 2 fixes a new feature introduced in 4.5 by ensuring that a
> batadv_hardif_neigh_node object is free'd only when it is no
> longer used in any context, by Sven Eckelmann.
> 
> Please pull or let me know of any issue.

Pulled, thanks!