batman-adv/patches/0020-batman-adv-Fix-reference-leak-in-batadv_find_router.patch

   1 From: Sven Eckelmann <sven@narfation.org>
   2 Date: Thu, 30 Jun 2016 20:11:34 +0200
   3 Subject: [PATCH] batman-adv: Fix reference leak in batadv_find_router
   4
   5 The replacement of last_bonding_candidate in batadv_orig_node has to be an
   6 atomic operation. Otherwise it is possible that the reference counter of a
   7 batadv_orig_ifinfo is reduced which was no longer the
   8 last_bonding_candidate when the new candidate is added. This can either
   9 lead to an invalid memory access or to reference leaks which make it
  10 impossible to an interface which was added to batman-adv.
  11
  12 Fixes: 797edd9e87ac ("batman-adv: add bonding again")
  13 Signed-off-by: Sven Eckelmann <sven@narfation.org>
  14 Acked-by: Simon Wunderlich <sw@simonwunderlich.de>
  15 Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
  16
  17 Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/6ecc711374afd93ee0c2216b38ae52d3ce680c3f
  18 ---
  19  net/batman-adv/routing.c | 52 ++++++++++++++++++++++++++++++++++++------------
  20  net/batman-adv/types.h   |  4 +++-
  21  2 files changed, 42 insertions(+), 14 deletions(-)
  22
  23 diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
  24 index 27e07dd..694dc74 100644
  25 --- a/net/batman-adv/routing.c
  26 +++ b/net/batman-adv/routing.c
  27 @@ -456,6 +456,29 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv,
  28  }
  29
  30  /**
  31 + * batadv_last_bonding_replace - Replace last_bonding_candidate of orig_node
  32 + * @orig_node: originator node whose bonding candidates should be replaced
  33 + * @new_candidate: new bonding candidate or NULL
  34 + */
  35 +static void
  36 +batadv_last_bonding_replace(struct batadv_orig_node *orig_node,
  37 +                           struct batadv_orig_ifinfo *new_candidate)
  38 +{
  39 +       struct batadv_orig_ifinfo *old_candidate;
  40 +
  41 +       spin_lock_bh(&orig_node->neigh_list_lock);
  42 +       old_candidate = orig_node->last_bonding_candidate;
  43 +
  44 +       if (new_candidate)
  45 +               kref_get(&new_candidate->refcount);
  46 +       orig_node->last_bonding_candidate = new_candidate;
  47 +       spin_unlock_bh(&orig_node->neigh_list_lock);
  48 +
  49 +       if (old_candidate)
  50 +               batadv_orig_ifinfo_put(old_candidate);
  51 +}
  52 +
  53 +/**
  54   * batadv_find_router - find a suitable router for this originator
  55   * @bat_priv: the bat priv with all the soft interface information
  56   * @orig_node: the destination node
  57 @@ -562,10 +585,6 @@ next:
  58         }
  59         rcu_read_unlock();
  60
  61 -       /* last_bonding_candidate is reset below, remove the old reference. */
  62 -       if (orig_node->last_bonding_candidate)
  63 -               batadv_orig_ifinfo_put(orig_node->last_bonding_candidate);
  64 -
  65         /* After finding candidates, handle the three cases:
  66          * 1) there is a next candidate, use that
  67          * 2) there is no next candidate, use the first of the list
  68 @@ -574,21 +593,28 @@ next:
  69         if (next_candidate) {
  70                 batadv_neigh_node_put(router);
  71
  72 -               /* remove references to first candidate, we don't need it. */
  73 -               if (first_candidate) {
  74 -                       batadv_neigh_node_put(first_candidate_router);
  75 -                       batadv_orig_ifinfo_put(first_candidate);
  76 -               }
  77 +               kref_get(&next_candidate_router->refcount);
  78                 router = next_candidate_router;
  79 -               orig_node->last_bonding_candidate = next_candidate;
  80 +               batadv_last_bonding_replace(orig_node, next_candidate);
  81         } else if (first_candidate) {
  82                 batadv_neigh_node_put(router);
  83
  84 -               /* refcounting has already been done in the loop above. */
  85 +               kref_get(&first_candidate_router->refcount);
  86                 router = first_candidate_router;
  87 -               orig_node->last_bonding_candidate = first_candidate;
  88 +               batadv_last_bonding_replace(orig_node, first_candidate);
  89         } else {
  90 -               orig_node->last_bonding_candidate = NULL;
  91 +               batadv_last_bonding_replace(orig_node, NULL);
  92 +       }
  93 +
  94 +       /* cleanup of candidates */
  95 +       if (first_candidate) {
  96 +               batadv_neigh_node_put(first_candidate_router);
  97 +               batadv_orig_ifinfo_put(first_candidate);
  98 +       }
  99 +
 100 +       if (next_candidate) {
 101 +               batadv_neigh_node_put(next_candidate_router);
 102 +               batadv_orig_ifinfo_put(next_candidate);
 103         }
 104
 105         return router;
 106 diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
 107 index 41a85b5..c143649 100644
 108 --- a/net/batman-adv/types.h
 109 +++ b/net/batman-adv/types.h
 110 @@ -330,7 +330,9 @@ struct batadv_orig_node {
 111         DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
 112         u32 last_bcast_seqno;
 113         struct hlist_head neigh_list;
 114 -       /* neigh_list_lock protects: neigh_list and router */
 115 +       /* neigh_list_lock protects: neigh_list, ifinfo_list,
 116 +        * last_bonding_candidate and router
 117 +        */
 118         spinlock_t neigh_list_lock;
 119         struct hlist_node hash_entry;
 120         struct batadv_priv *bat_priv;