diff mbox

[maint,v2] batman-adv: Reduce refcnt of removed router when updating route

Message ID 1458473273-8769-1-git-send-email-sven@narfation.org (mailing list archive)
State Accepted, archived
Delegated to: Marek Lindner
Headers

Commit Message

Sven Eckelmann March 20, 2016, 11:27 a.m. UTC 
  _batadv_update_route rcu_derefences orig_ifinfo->router outside of a
spinlock protected region to print some information messages to the debug
log. But this pointer is not checked again when the new pointer is assigned
in the spinlock protected region. Thus is can happen that the value of
orig_ifinfo->router changed in the meantime and thus the reference counter
of the wrong router gets reduced after the spinlock protected region.

Just rcu_dereferencing the value of orig_ifinfo->router inside the spinlock
protected region (which also set the new pointer) is enough to get the
correct old router object.

Fixes: d90ddb94423f ("batman-adv: Make orig_node->router an rcu protected pointer")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
--
v2:
 - add comment explaining the idea behind the extra rcu_dereference_protected
---
 net/batman-adv/routing.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Marek Lindner March 27, 2016, 7:51 a.m. UTC | #1 
On Sunday, March 20, 2016 12:27:53 Sven Eckelmann wrote:
> _batadv_update_route rcu_derefences orig_ifinfo->router outside of a
> spinlock protected region to print some information messages to the debug
> log. But this pointer is not checked again when the new pointer is assigned
> in the spinlock protected region. Thus is can happen that the value of
> orig_ifinfo->router changed in the meantime and thus the reference counter
> of the wrong router gets reduced after the spinlock protected region.
> 
> Just rcu_dereferencing the value of orig_ifinfo->router inside the spinlock
> protected region (which also set the new pointer) is enough to get the
> correct old router object.
> 
> Fixes: d90ddb94423f ("batman-adv: Make orig_node->router an rcu protected
> pointer") Signed-off-by: Sven Eckelmann <sven@narfation.org>
> --
> v2:
>  - add comment explaining the idea behind the extra
> rcu_dereference_protected ---
>  net/batman-adv/routing.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Applied with minor modifications in revision 08ba64d.

Thanks,
Marek
diff mbox

Patch

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 1fb1be3..18fc4db 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -104,6 +104,13 @@  static void _batadv_update_route(struct batadv_priv *bat_priv,
 		neigh_node = NULL;
 
 	spin_lock_bh(&orig_node->neigh_list_lock);
+	/* get previous best router to decrease the reference counter later.
+	 * curr_router used earlier may not be the current orig_ifinfo->router
+	 * because it was dereferenced outside of the neigh_list_lock protected
+	 * region.
+	 */
+	curr_router = rcu_dereference_protected(orig_ifinfo->router, true);
+
 	rcu_assign_pointer(orig_ifinfo->router, neigh_node);
 	spin_unlock_bh(&orig_node->neigh_list_lock);
 	batadv_orig_ifinfo_free_ref(orig_ifinfo);