3.16.36-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Sven Eckelmann <s...@narfation.org>

commit a33d970d0b54b09746d5540af8271fad4eb10229 upstream.

The batadv_tt_local_entry was specific to a batadv_softif_vlan and held an
implicit reference to it. But this reference was never stored in form of a
pointer in the tt_local_entry itself. Instead batadv_tt_local_remove,
batadv_tt_local_table_free and batadv_tt_local_purge_pending_clients depend
on a consistent state of bat_priv->softif_vlan_list and that
batadv_softif_vlan_get always returns the batadv_softif_vlan object which
it has a reference for. But batadv_softif_vlan_get cannot guarantee that
because it is working only with rcu_read_lock on this list. It can
therefore happen that an vid is in this list twice or that
batadv_softif_vlan_get cannot find the batadv_softif_vlan for an vid due to
some other list operations taking place at the same time.

Instead add a batadv_softif_vlan pointer directly in batadv_tt_local_entry
which will be used for the reference counter decremented on release of
batadv_tt_local_entry.

Fixes: 35df3b298fc8 ("batman-adv: fix TT VLAN inconsistency on VLAN re-add")
Signed-off-by: Sven Eckelmann <s...@narfation.org>
Acked-by: Antonio Quartulli <a...@unstable.cc>
Signed-off-by: Marek Lindner <mareklind...@neomailbox.ch>
Signed-off-by: Antonio Quartulli <a...@unstable.cc>
[bwh: Backported to 3.16:
 - s/_put/_free_ref/ in various function names
 - Adjust context]
Signed-off-by: Ben Hutchings <b...@decadent.org.uk>
---
 net/batman-adv/translation-table.c | 42 ++++----------------------------------
 net/batman-adv/types.h             |  2 ++
 2 files changed, 6 insertions(+), 38 deletions(-)

--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -176,8 +176,10 @@ batadv_tt_global_hash_find(struct batadv
 static void
 batadv_tt_local_entry_free_ref(struct batadv_tt_local_entry *tt_local_entry)
 {
-       if (atomic_dec_and_test(&tt_local_entry->common.refcount))
+       if (atomic_dec_and_test(&tt_local_entry->common.refcount)) {
+               batadv_softif_vlan_free_ref(tt_local_entry->vlan);
                kfree_rcu(tt_local_entry, common.rcu);
+       }
 }
 
 /**
@@ -595,6 +597,7 @@ bool batadv_tt_local_add(struct net_devi
        atomic_set(&tt_local->common.refcount, 2);
        tt_local->last_seen = jiffies;
        tt_local->common.added_at = tt_local->last_seen;
+       tt_local->vlan = vlan;
 
        /* the batman interface mac and multicast addresses should never be
         * purged
@@ -908,7 +911,6 @@ int batadv_tt_local_seq_print_text(struc
        struct batadv_tt_common_entry *tt_common_entry;
        struct batadv_tt_local_entry *tt_local;
        struct batadv_hard_iface *primary_if;
-       struct batadv_softif_vlan *vlan;
        struct hlist_head *head;
        unsigned short vid;
        uint32_t i;
@@ -944,14 +946,6 @@ int batadv_tt_local_seq_print_text(struc
                        last_seen_msecs = last_seen_msecs % 1000;
 
                        no_purge = tt_common_entry->flags & np_flag;
-
-                       vlan = batadv_softif_vlan_get(bat_priv, vid);
-                       if (!vlan) {
-                               seq_printf(seq, "Cannot retrieve VLAN %d\n",
-                                          BATADV_PRINT_VID(vid));
-                               continue;
-                       }
-
                        seq_printf(seq,
                                   " * %pM %4i [%c%c%c%c%c%c] %3u.%03u   
(%#.8x)\n",
                                   tt_common_entry->addr,
@@ -969,9 +963,7 @@ int batadv_tt_local_seq_print_text(struc
                                    BATADV_TT_CLIENT_ISOLA ? 'I' : '.'),
                                   no_purge ? 0 : last_seen_secs,
                                   no_purge ? 0 : last_seen_msecs,
-                                  vlan->tt.crc);
-
-                       batadv_softif_vlan_free_ref(vlan);
+                                  tt_local->vlan->tt.crc);
                }
                rcu_read_unlock();
        }
@@ -1016,7 +1008,6 @@ uint16_t batadv_tt_local_remove(struct b
 {
        struct batadv_tt_local_entry *tt_local_entry;
        uint16_t flags, curr_flags = BATADV_NO_FLAGS;
-       struct batadv_softif_vlan *vlan;
        void *tt_entry_exists;
 
        tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
@@ -1056,14 +1047,6 @@ uint16_t batadv_tt_local_remove(struct b
        /* extra call to free the local tt entry */
        batadv_tt_local_entry_free_ref(tt_local_entry);
 
-       /* decrease the reference held for this vlan */
-       vlan = batadv_softif_vlan_get(bat_priv, vid);
-       if (!vlan)
-               goto out;
-
-       batadv_softif_vlan_free_ref(vlan);
-       batadv_softif_vlan_free_ref(vlan);
-
 out:
        if (tt_local_entry)
                batadv_tt_local_entry_free_ref(tt_local_entry);
@@ -1136,7 +1119,6 @@ static void batadv_tt_local_table_free(s
        spinlock_t *list_lock; /* protects write access to the hash lists */
        struct batadv_tt_common_entry *tt_common_entry;
        struct batadv_tt_local_entry *tt_local;
-       struct batadv_softif_vlan *vlan;
        struct hlist_node *node_tmp;
        struct hlist_head *head;
        uint32_t i;
@@ -1158,14 +1140,6 @@ static void batadv_tt_local_table_free(s
                                                struct batadv_tt_local_entry,
                                                common);
 
-                       /* decrease the reference held for this vlan */
-                       vlan = batadv_softif_vlan_get(bat_priv,
-                                                     tt_common_entry->vid);
-                       if (vlan) {
-                               batadv_softif_vlan_free_ref(vlan);
-                               batadv_softif_vlan_free_ref(vlan);
-                       }
-
                        batadv_tt_local_entry_free_ref(tt_local);
                }
                spin_unlock_bh(list_lock);
@@ -3174,7 +3148,6 @@ static void batadv_tt_local_purge_pendin
        struct batadv_hashtable *hash = bat_priv->tt.local_hash;
        struct batadv_tt_common_entry *tt_common;
        struct batadv_tt_local_entry *tt_local;
-       struct batadv_softif_vlan *vlan;
        struct hlist_node *node_tmp;
        struct hlist_head *head;
        spinlock_t *list_lock; /* protects write access to the hash lists */
@@ -3204,13 +3177,6 @@ static void batadv_tt_local_purge_pendin
                                                struct batadv_tt_local_entry,
                                                common);
 
-                       /* decrease the reference held for this vlan */
-                       vlan = batadv_softif_vlan_get(bat_priv, tt_common->vid);
-                       if (vlan) {
-                               batadv_softif_vlan_free_ref(vlan);
-                               batadv_softif_vlan_free_ref(vlan);
-                       }
-
                        batadv_tt_local_entry_free_ref(tt_local);
                }
                spin_unlock_bh(list_lock);
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -934,10 +934,12 @@ struct batadv_tt_common_entry {
  * struct batadv_tt_local_entry - translation table local entry data
  * @common: general translation table data
  * @last_seen: timestamp used for purging stale tt local entries
+ * @vlan: soft-interface vlan of the entry
  */
 struct batadv_tt_local_entry {
        struct batadv_tt_common_entry common;
        unsigned long last_seen;
+       struct batadv_softif_vlan *vlan;
 };
 
 /**

Reply via email to