Staging: batman-adv: count batman_if list queries as reference

The return of get_batman_if_by_netdev and get_active_batman_if leaks a
pointer from the rcu protected list of interfaces. We must protect it to
prevent a too early release of the memory. Those functions must increase
the reference counter before rcu_read_unlock or it may be to late to
prevent a free.

hardif_add_interface must also increase the reference count for the
returned batman_if to make the behaviour consistent.

Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/staging/batman-adv/TODO b/drivers/staging/batman-adv/TODO
index cb6b026..5913731 100644 (file)
--- a/drivers/staging/batman-adv/TODO
+++ b/drivers/staging/batman-adv/TODO
@@ -1,7 +1,3 @@
- * Rework usage of RCU
-    - don't leak pointers from rcu out of rcu critical area which may
-      get freed
-    - go through Documentation/RCU/checklist.txt
  * Request a new review
  * Process the comments from the review
  * Move into mainline proper

diff --git a/drivers/staging/batman-adv/bat_sysfs.c b/drivers/staging/batman-adv/bat_sysfs.c
index 0610169..bc17fb8 100644 (file)
--- a/drivers/staging/batman-adv/bat_sysfs.c
+++ b/drivers/staging/batman-adv/bat_sysfs.c
@@ -405,13 +405,17 @@ static ssize_t show_mesh_iface(struct kobject *kobj, struct attribute *attr,
        struct device *dev = to_dev(kobj->parent);
        struct net_device *net_dev = to_net_dev(dev);
        struct batman_if *batman_if = get_batman_if_by_netdev(net_dev);
+       ssize_t length;
 
        if (!batman_if)
                return 0;
 
-       return sprintf(buff, "%s\n",
-                      batman_if->if_status == IF_NOT_IN_USE ?
-                                       "none" : batman_if->soft_iface->name);
+       length = sprintf(buff, "%s\n", batman_if->if_status == IF_NOT_IN_USE ?
+                        "none" : batman_if->soft_iface->name);
+
+       hardif_put(batman_if);
+
+       return length;
 }
 
 static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr,
@@ -421,6 +425,7 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr,
        struct net_device *net_dev = to_net_dev(dev);
        struct batman_if *batman_if = get_batman_if_by_netdev(net_dev);
        int status_tmp = -1;
+       int ret;
 
        if (!batman_if)
                return count;
@@ -431,6 +436,7 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr,
        if (strlen(buff) >= IFNAMSIZ) {
                pr_err("Invalid parameter for 'mesh_iface' setting received: "
                       "interface name too long '%s'\n", buff);
+               hardif_put(batman_if);
                return -EINVAL;
        }
 
@@ -440,13 +446,16 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr,
                status_tmp = IF_I_WANT_YOU;
 
        if ((batman_if->if_status == status_tmp) || ((batman_if->soft_iface) &&
-           (strncmp(batman_if->soft_iface->name, buff, IFNAMSIZ) == 0)))
+           (strncmp(batman_if->soft_iface->name, buff, IFNAMSIZ) == 0))) {
+               hardif_put(batman_if);
                return count;
+       }
 
        if (status_tmp == IF_NOT_IN_USE) {
                rtnl_lock();
                hardif_disable_interface(batman_if);
                rtnl_unlock();
+               hardif_put(batman_if);
                return count;
        }
 
@@ -457,7 +466,10 @@ static ssize_t store_mesh_iface(struct kobject *kobj, struct attribute *attr,
                rtnl_unlock();
        }
 
-       return hardif_enable_interface(batman_if, buff);
+       ret = hardif_enable_interface(batman_if, buff);
+       hardif_put(batman_if);
+
+       return ret;
 }
 
 static ssize_t show_iface_status(struct kobject *kobj, struct attribute *attr,
@@ -466,23 +478,33 @@ static ssize_t show_iface_status(struct kobject *kobj, struct attribute *attr,
        struct device *dev = to_dev(kobj->parent);
        struct net_device *net_dev = to_net_dev(dev);
        struct batman_if *batman_if = get_batman_if_by_netdev(net_dev);
+       ssize_t length;
 
        if (!batman_if)
                return 0;
 
        switch (batman_if->if_status) {
        case IF_TO_BE_REMOVED:
-               return sprintf(buff, "disabling\n");
+               length = sprintf(buff, "disabling\n");
+               break;
        case IF_INACTIVE:
-               return sprintf(buff, "inactive\n");
+               length = sprintf(buff, "inactive\n");
+               break;
        case IF_ACTIVE:
-               return sprintf(buff, "active\n");
+               length = sprintf(buff, "active\n");
+               break;
        case IF_TO_BE_ACTIVATED:
-               return sprintf(buff, "enabling\n");
+               length = sprintf(buff, "enabling\n");
+               break;
        case IF_NOT_IN_USE:
        default:
-               return sprintf(buff, "not in use\n");
+               length = sprintf(buff, "not in use\n");
+               break;
        }
+
+       hardif_put(batman_if);
+
+       return length;
 }
 
 static BAT_ATTR(mesh_iface, S_IRUGO | S_IWUSR,

diff --git a/drivers/staging/batman-adv/hard-interface.c b/drivers/staging/batman-adv/hard-interface.c
index df6e5bd..eef5631 100644 (file)
--- a/drivers/staging/batman-adv/hard-interface.c
+++ b/drivers/staging/batman-adv/hard-interface.c
@@ -49,6 +49,9 @@ struct batman_if *get_batman_if_by_netdev(struct net_device *net_dev)
        batman_if = NULL;
 
 out:
+       if (batman_if)
+               hardif_hold(batman_if);
+
        rcu_read_unlock();
        return batman_if;
 }
@@ -96,6 +99,9 @@ static struct batman_if *get_active_batman_if(struct net_device *soft_iface)
        batman_if = NULL;
 
 out:
+       if (batman_if)
+               hardif_hold(batman_if);
+
        rcu_read_unlock();
        return batman_if;
 }
@@ -292,6 +298,7 @@ int hardif_enable_interface(struct batman_if *batman_if, char *iface_name)
        batman_if->batman_adv_ptype.type = __constant_htons(ETH_P_BATMAN);
        batman_if->batman_adv_ptype.func = batman_skb_recv;
        batman_if->batman_adv_ptype.dev = batman_if->net_dev;
+       hardif_hold(batman_if);
        dev_add_pack(&batman_if->batman_adv_ptype);
 
        atomic_set(&batman_if->seqno, 1);
@@ -350,13 +357,20 @@ void hardif_disable_interface(struct batman_if *batman_if)
        bat_info(batman_if->soft_iface, "Removing interface: %s\n",
                 batman_if->net_dev->name);
        dev_remove_pack(&batman_if->batman_adv_ptype);
+       hardif_put(batman_if);
 
        bat_priv->num_ifaces--;
        orig_hash_del_if(batman_if, bat_priv->num_ifaces);
 
-       if (batman_if == bat_priv->primary_if)
-               set_primary_if(bat_priv,
-                              get_active_batman_if(batman_if->soft_iface));
+       if (batman_if == bat_priv->primary_if) {
+               struct batman_if *new_if;
+
+               new_if = get_active_batman_if(batman_if->soft_iface);
+               set_primary_if(bat_priv, new_if);
+
+               if (new_if)
+                       hardif_put(new_if);
+       }
 
        kfree(batman_if->packet_buff);
        batman_if->packet_buff = NULL;
@@ -410,6 +424,8 @@ static struct batman_if *hardif_add_interface(struct net_device *net_dev)
        list_add_tail_rcu(&batman_if->list, &if_list);
        spin_unlock(&if_list_lock);
 
+       /* extra reference for return */
+       hardif_hold(batman_if);
        return batman_if;
 
 free_if:
@@ -459,7 +475,7 @@ static int hard_if_event(struct notifier_block *this,
        struct bat_priv *bat_priv;
 
        if (!batman_if && event == NETDEV_REGISTER)
-                       batman_if = hardif_add_interface(net_dev);
+               batman_if = hardif_add_interface(net_dev);
 
        if (!batman_if)
                goto out;
@@ -482,8 +498,10 @@ static int hard_if_event(struct notifier_block *this,
                        update_min_mtu(batman_if->soft_iface);
                break;
        case NETDEV_CHANGEADDR:
-               if (batman_if->if_status == IF_NOT_IN_USE)
+               if (batman_if->if_status == IF_NOT_IN_USE) {
+                       hardif_put(batman_if);
                        goto out;
+               }
 
                check_known_mac_addr(batman_if->net_dev->dev_addr);
                update_mac_addresses(batman_if);
@@ -495,6 +513,7 @@ static int hard_if_event(struct notifier_block *this,
        default:
                break;
        };
+       hardif_put(batman_if);
 
 out:
        return NOTIFY_DONE;