batman-adv/patches/0005-batman-adv-Fix-non-atomic-bla_claim-backbone_gw-acce.patch

   1 From: Sven Eckelmann <sven@narfation.org>
   2 Date: Fri, 1 Jul 2016 15:49:43 +0200
   3 Subject: [PATCH] batman-adv: Fix non-atomic bla_claim::backbone_gw access
   4
   5 The pointer batadv_bla_claim::backbone_gw can be changed at any time.
   6 Therefore, access to it must be protected to ensure that two function
   7 accessing the same backbone_gw are actually accessing the same. This is
   8 especially important when the crc_lock is used or when the backbone_gw of a
   9 claim is exchanged.
  10
  11 Not doing so leads to invalid memory access and/or reference leaks.
  12
  13 Fixes: a9ce0dc43e2c ("batman-adv: add basic bridge loop avoidance code")
  14 Fixes: b307e72d119f ("batman-adv: lock crc access in bridge loop avoidance")
  15 Signed-off-by: Sven Eckelmann <sven@narfation.org>
  16 Acked-by: Simon Wunderlich <sw@simonwunderlich.de>
  17 Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
  18
  19 Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/e401297e3a393896e9b07bef8d6e2df203b60d43
  20 ---
  21  net/batman-adv/bridge_loop_avoidance.c | 111 ++++++++++++++++++++++++++-------
  22  net/batman-adv/types.h                 |   2 +
  23  2 files changed, 90 insertions(+), 23 deletions(-)
  24
  25 diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
  26 index 7129780..825a5cd 100644
  27 --- a/net/batman-adv/bridge_loop_avoidance.c
  28 +++ b/net/batman-adv/bridge_loop_avoidance.c
  29 @@ -177,10 +177,21 @@ static void batadv_backbone_gw_put(struct batadv_bla_backbone_gw *backbone_gw)
  30  static void batadv_claim_release(struct kref *ref)
  31  {
  32         struct batadv_bla_claim *claim;
  33 +       struct batadv_bla_backbone_gw *old_backbone_gw;
  34
  35         claim = container_of(ref, struct batadv_bla_claim, refcount);
  36
  37 -       batadv_backbone_gw_put(claim->backbone_gw);
  38 +       spin_lock_bh(&claim->backbone_lock);
  39 +       old_backbone_gw = claim->backbone_gw;
  40 +       claim->backbone_gw = NULL;
  41 +       spin_unlock_bh(&claim->backbone_lock);
  42 +
  43 +       spin_lock_bh(&old_backbone_gw->crc_lock);
  44 +       old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
  45 +       spin_unlock_bh(&old_backbone_gw->crc_lock);
  46 +
  47 +       batadv_backbone_gw_put(old_backbone_gw);
  48 +
  49         kfree_rcu(claim, rcu);
  50  }
  51
  52 @@ -677,8 +688,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
  53                                  const u8 *mac, const unsigned short vid,
  54                                  struct batadv_bla_backbone_gw *backbone_gw)
  55  {
  56 +       struct batadv_bla_backbone_gw *old_backbone_gw;
  57         struct batadv_bla_claim *claim;
  58         struct batadv_bla_claim search_claim;
  59 +       bool remove_crc = false;
  60         int hash_added;
  61
  62         ether_addr_copy(search_claim.addr, mac);
  63 @@ -692,8 +705,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
  64                         return;
  65
  66                 ether_addr_copy(claim->addr, mac);
  67 +               spin_lock_init(&claim->backbone_lock);
  68                 claim->vid = vid;
  69                 claim->lasttime = jiffies;
  70 +               kref_get(&backbone_gw->refcount);
  71                 claim->backbone_gw = backbone_gw;
  72
  73                 kref_init(&claim->refcount);
  74 @@ -721,15 +736,26 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
  75                            "bla_add_claim(): changing ownership for %pM, vid %d\n",
  76                            mac, BATADV_PRINT_VID(vid));
  77
  78 -               spin_lock_bh(&claim->backbone_gw->crc_lock);
  79 -               claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
  80 -               spin_unlock_bh(&claim->backbone_gw->crc_lock);
  81 -               batadv_backbone_gw_put(claim->backbone_gw);
  82 +               remove_crc = true;
  83         }
  84 -       /* set (new) backbone gw */
  85 +
  86 +       /* replace backbone_gw atomically and adjust reference counters */
  87 +       spin_lock_bh(&claim->backbone_lock);
  88 +       old_backbone_gw = claim->backbone_gw;
  89         kref_get(&backbone_gw->refcount);
  90         claim->backbone_gw = backbone_gw;
  91 +       spin_unlock_bh(&claim->backbone_lock);
  92
  93 +       if (remove_crc) {
  94 +               /* remove claim address from old backbone_gw */
  95 +               spin_lock_bh(&old_backbone_gw->crc_lock);
  96 +               old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
  97 +               spin_unlock_bh(&old_backbone_gw->crc_lock);
  98 +       }
  99 +
 100 +       batadv_backbone_gw_put(old_backbone_gw);
 101 +
 102 +       /* add claim address to new backbone_gw */
 103         spin_lock_bh(&backbone_gw->crc_lock);
 104         backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
 105         spin_unlock_bh(&backbone_gw->crc_lock);
 106 @@ -740,6 +766,26 @@ claim_free_ref:
 107  }
 108
 109  /**
 110 + * batadv_bla_claim_get_backbone_gw - Get valid reference for backbone_gw of
 111 + *  claim
 112 + * @claim: claim whose backbone_gw should be returned
 113 + *
 114 + * Return: valid reference to claim::backbone_gw
 115 + */
 116 +static struct batadv_bla_backbone_gw *
 117 +batadv_bla_claim_get_backbone_gw(struct batadv_bla_claim *claim)
 118 +{
 119 +       struct batadv_bla_backbone_gw *backbone_gw;
 120 +
 121 +       spin_lock_bh(&claim->backbone_lock);
 122 +       backbone_gw = claim->backbone_gw;
 123 +       kref_get(&backbone_gw->refcount);
 124 +       spin_unlock_bh(&claim->backbone_lock);
 125 +
 126 +       return backbone_gw;
 127 +}
 128 +
 129 +/**
 130   * batadv_bla_del_claim - delete a claim from the claim hash
 131   * @bat_priv: the bat priv with all the soft interface information
 132   * @mac: mac address of the claim to be removed
 133 @@ -763,10 +809,6 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv,
 134                            batadv_choose_claim, claim);
 135         batadv_claim_put(claim); /* reference from the hash is gone */
 136
 137 -       spin_lock_bh(&claim->backbone_gw->crc_lock);
 138 -       claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
 139 -       spin_unlock_bh(&claim->backbone_gw->crc_lock);
 140 -
 141         /* don't need the reference from hash_find() anymore */
 142         batadv_claim_put(claim);
 143  }
 144 @@ -1219,6 +1261,7 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
 145                                     struct batadv_hard_iface *primary_if,
 146                                     int now)
 147  {
 148 +       struct batadv_bla_backbone_gw *backbone_gw;
 149         struct batadv_bla_claim *claim;
 150         struct hlist_head *head;
 151         struct batadv_hashtable *hash;
 152 @@ -1233,14 +1276,17 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
 153
 154                 rcu_read_lock();
 155                 hlist_for_each_entry_rcu(claim, head, hash_entry) {
 156 +                       backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
 157                         if (now)
 158                                 goto purge_now;
 159 -                       if (!batadv_compare_eth(claim->backbone_gw->orig,
 160 +
 161 +                       if (!batadv_compare_eth(backbone_gw->orig,
 162                                                 primary_if->net_dev->dev_addr))
 163 -                               continue;
 164 +                               goto skip;
 165 +
 166                         if (!batadv_has_timed_out(claim->lasttime,
 167                                                   BATADV_BLA_CLAIM_TIMEOUT))
 168 -                               continue;
 169 +                               goto skip;
 170
 171                         batadv_dbg(BATADV_DBG_BLA, bat_priv,
 172                                    "bla_purge_claims(): %pM, vid %d, time out\n",
 173 @@ -1248,8 +1294,10 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
 174
 175  purge_now:
 176                         batadv_handle_unclaim(bat_priv, primary_if,
 177 -                                             claim->backbone_gw->orig,
 178 +                                             backbone_gw->orig,
 179                                               claim->addr, claim->vid);
 180 +skip:
 181 +                       batadv_backbone_gw_put(backbone_gw);
 182                 }
 183                 rcu_read_unlock();
 184         }
 185 @@ -1760,9 +1808,11 @@ batadv_bla_loopdetect_check(struct batadv_priv *bat_priv, struct sk_buff *skb,
 186  bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 187                    unsigned short vid, bool is_bcast)
 188  {
 189 +       struct batadv_bla_backbone_gw *backbone_gw;
 190         struct ethhdr *ethhdr;
 191         struct batadv_bla_claim search_claim, *claim = NULL;
 192         struct batadv_hard_iface *primary_if;
 193 +       bool own_claim;
 194         bool ret;
 195
 196         ethhdr = eth_hdr(skb);
 197 @@ -1797,8 +1847,12 @@ bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 198         }
 199
 200         /* if it is our own claim ... */
 201 -       if (batadv_compare_eth(claim->backbone_gw->orig,
 202 -                              primary_if->net_dev->dev_addr)) {
 203 +       backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
 204 +       own_claim = batadv_compare_eth(backbone_gw->orig,
 205 +                                      primary_if->net_dev->dev_addr);
 206 +       batadv_backbone_gw_put(backbone_gw);
 207 +
 208 +       if (own_claim) {
 209                 /* ... allow it in any case */
 210                 claim->lasttime = jiffies;
 211                 goto allow;
 212 @@ -1862,7 +1916,9 @@ bool batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 213  {
 214         struct ethhdr *ethhdr;
 215         struct batadv_bla_claim search_claim, *claim = NULL;
 216 +       struct batadv_bla_backbone_gw *backbone_gw;
 217         struct batadv_hard_iface *primary_if;
 218 +       bool client_roamed;
 219         bool ret = false;
 220
 221         primary_if = batadv_primary_if_get_selected(bat_priv);
 222 @@ -1892,8 +1948,12 @@ bool batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 223                 goto allow;
 224
 225         /* check if we are responsible. */
 226 -       if (batadv_compare_eth(claim->backbone_gw->orig,
 227 -                              primary_if->net_dev->dev_addr)) {
 228 +       backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
 229 +       client_roamed = batadv_compare_eth(backbone_gw->orig,
 230 +                                          primary_if->net_dev->dev_addr);
 231 +       batadv_backbone_gw_put(backbone_gw);
 232 +
 233 +       if (client_roamed) {
 234                 /* if yes, the client has roamed and we have
 235                  * to unclaim it.
 236                  */
 237 @@ -1941,6 +2001,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
 238         struct net_device *net_dev = (struct net_device *)seq->private;
 239         struct batadv_priv *bat_priv = netdev_priv(net_dev);
 240         struct batadv_hashtable *hash = bat_priv->bla.claim_hash;
 241 +       struct batadv_bla_backbone_gw *backbone_gw;
 242         struct batadv_bla_claim *claim;
 243         struct batadv_hard_iface *primary_if;
 244         struct hlist_head *head;
 245 @@ -1965,17 +2026,21 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
 246
 247                 rcu_read_lock();
 248                 hlist_for_each_entry_rcu(claim, head, hash_entry) {
 249 -                       is_own = batadv_compare_eth(claim->backbone_gw->orig,
 250 +                       backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
 251 +
 252 +                       is_own = batadv_compare_eth(backbone_gw->orig,
 253                                                     primary_addr);
 254
 255 -                       spin_lock_bh(&claim->backbone_gw->crc_lock);
 256 -                       backbone_crc = claim->backbone_gw->crc;
 257 -                       spin_unlock_bh(&claim->backbone_gw->crc_lock);
 258 +                       spin_lock_bh(&backbone_gw->crc_lock);
 259 +                       backbone_crc = backbone_gw->crc;
 260 +                       spin_unlock_bh(&backbone_gw->crc_lock);
 261                         seq_printf(seq, " * %pM on %5d by %pM [%c] (%#.4x)\n",
 262                                    claim->addr, BATADV_PRINT_VID(claim->vid),
 263 -                                  claim->backbone_gw->orig,
 264 +                                  backbone_gw->orig,
 265                                    (is_own ? 'x' : ' '),
 266                                    backbone_crc);
 267 +
 268 +                       batadv_backbone_gw_put(backbone_gw);
 269                 }
 270                 rcu_read_unlock();
 271         }
 272 diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
 273 index ba846b0..0051222 100644
 274 --- a/net/batman-adv/types.h
 275 +++ b/net/batman-adv/types.h
 276 @@ -1042,6 +1042,7 @@ struct batadv_bla_backbone_gw {
 277   * @addr: mac address of claimed non-mesh client
 278   * @vid: vlan id this client was detected on
 279   * @backbone_gw: pointer to backbone gw claiming this client
 280 + * @backbone_lock: lock protecting backbone_gw pointer
 281   * @lasttime: last time we heard of claim (locals only)
 282   * @hash_entry: hlist node for batadv_priv_bla::claim_hash
 283   * @refcount: number of contexts the object is used
 284 @@ -1051,6 +1052,7 @@ struct batadv_bla_claim {
 285         u8 addr[ETH_ALEN];
 286         unsigned short vid;
 287         struct batadv_bla_backbone_gw *backbone_gw;
 288 +       spinlock_t backbone_lock; /* protects backbone_gw */
 289         unsigned long lasttime;
 290         struct hlist_node hash_entry;
 291         struct rcu_head rcu;