From: Sven Eckelmann <sven.eckelmann@gmx.de>
To: b.a.t.m.a.n@lists.open-mesh.org
Subject: [B.A.T.M.A.N.] [PATCH 9/9] batman-adv: count batman_if list queries as reference
Date: Fri, 17 Sep 2010 17:41:05 +0200 [thread overview]
Message-ID: <1284738065-8715-10-git-send-email-sven.eckelmann@gmx.de> (raw)
In-Reply-To: <1284738065-8715-1-git-send-email-sven.eckelmann@gmx.de>
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>
---
batman-adv/bat_sysfs.c | 37 +++++++++++++++++++++++++++----------
batman-adv/hard-interface.c | 29 ++++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 15 deletions(-)
diff --git a/batman-adv/bat_sysfs.c b/batman-adv/bat_sysfs.c
index 8e180ba..79090c9 100644
--- a/batman-adv/bat_sysfs.c
+++ b/batman-adv/bat_sysfs.c
@@ -453,13 +453,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,
@@ -469,6 +473,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;
@@ -479,6 +484,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;
}
@@ -488,13 +494,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;
}
@@ -505,7 +514,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,
@@ -514,23 +526,28 @@ 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");
case IF_INACTIVE:
- return sprintf(buff, "inactive\n");
+ length = sprintf(buff, "inactive\n");
case IF_ACTIVE:
- return sprintf(buff, "active\n");
+ length = sprintf(buff, "active\n");
case IF_TO_BE_ACTIVATED:
- return sprintf(buff, "enabling\n");
+ length = sprintf(buff, "enabling\n");
case IF_NOT_IN_USE:
default:
- return sprintf(buff, "not in use\n");
+ length = sprintf(buff, "not in use\n");
}
+
+ hardif_put(batman_if);
+
+ return length;
}
static BAT_ATTR(mesh_iface, S_IRUGO | S_IWUSR,
diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c
index 445498c..f519b4b 100644
--- a/batman-adv/hard-interface.c
+++ b/batman-adv/hard-interface.c
@@ -51,6 +51,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;
}
@@ -98,6 +101,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;
}
@@ -294,6 +300,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);
@@ -352,13 +359,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;
@@ -412,6 +426,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:
@@ -461,7 +477,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;
@@ -484,8 +500,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);
@@ -497,6 +515,7 @@ static int hard_if_event(struct notifier_block *this,
default:
break;
};
+ hardif_put(batman_if);
out:
return NOTIFY_DONE;
--
1.7.2.3
next prev parent reply other threads:[~2010-09-17 15:41 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-17 15:40 [B.A.T.M.A.N.] [PATCHv2 0/9] RCU locking patches Sven Eckelmann
2010-09-17 15:40 ` [B.A.T.M.A.N.] [PATCH 1/9] batman-adv: Introduce if_list_lock to protect if_list Sven Eckelmann
2010-09-17 15:40 ` [B.A.T.M.A.N.] [PATCH 2/9] batman-adv: Protect update side of gw_list Sven Eckelmann
2010-09-17 15:40 ` [B.A.T.M.A.N.] [PATCH 3/9] batman-adv: Always protect list_for_each_entry_rcu with RCU Sven Eckelmann
2010-09-17 15:41 ` [B.A.T.M.A.N.] [PATCH 4/9] batman-adv: Remove unneeded rcu_read_lock Sven Eckelmann
2010-09-17 15:41 ` [B.A.T.M.A.N.] [PATCH 5/9] batman-adv: Use synchronize_rcu instead of call_rcu Sven Eckelmann
2010-09-17 15:41 ` [B.A.T.M.A.N.] [PATCH 6/9] batman-adv: Use refcnt to track usage count of gw_node Sven Eckelmann
2010-09-17 15:41 ` [B.A.T.M.A.N.] [PATCH 7/9] batman-adv: Use refcnt to track usage count of batman_if Sven Eckelmann
2010-09-17 15:41 ` [B.A.T.M.A.N.] [PATCH 8/9] batman-adv: count election of gateway as reference Sven Eckelmann
2010-09-17 15:41 ` Sven Eckelmann [this message]
2010-09-17 17:27 ` [B.A.T.M.A.N.] [PATCHv2 9/9] batman-adv: count batman_if list queries " Sven Eckelmann
2010-09-18 14:42 ` Marek Lindner
2010-09-18 14:42 ` [B.A.T.M.A.N.] [PATCH] " Marek Lindner
2010-09-18 15:21 ` [B.A.T.M.A.N.] [PATCHv2 9/9] " Sven Eckelmann
2010-09-18 15:23 ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Track references of batman_if in set_primary_if Sven Eckelmann
2010-09-18 15:23 ` [B.A.T.M.A.N.] [PATCH 2/2] Introduce update_primary_addr to update mac address Sven Eckelmann
2010-09-18 15:41 ` [B.A.T.M.A.N.] [PATCHv2 9/9] batman-adv: count batman_if list queries as reference Marek Lindner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1284738065-8715-10-git-send-email-sven.eckelmann@gmx.de \
--to=sven.eckelmann@gmx.de \
--cc=b.a.t.m.a.n@lists.open-mesh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).