aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohannes Berg <johannes@sipsolutions.net>2009-03-24 09:35:46 +0100
committerJohn W. Linville <linville@tuxdriver.com>2009-03-27 20:13:20 -0400
commit4bbf4d56583dd52c429d88f43cb614bdbe5deea6 (patch)
tree7a3f902a08820342254e0d67607fe870b02620b3
parent3832c287f11ba001bbe48e9be8c59cb9f71f6b43 (diff)
cfg80211: fix locking in nl80211_set_wiphy
Luis reports that there's a circular locking dependency; this is because cfg80211_dev_rename() will acquire the cfg80211_mutex while the device mutex is held, while this normally is done the other way around. The solution is to open-code the device-getting in nl80211_set_wiphy and require holding the mutex around cfg80211_dev_rename rather than acquiring it within. Also fix a bug -- rtnl locking is expected by drivers so we need to provide it. Reported-by: Luis R. Rodriguez <lrodriguez@atheros.com> Signed-off-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: John W. Linville <linville@tuxdriver.com>
-rw-r--r--net/wireless/core.c30
-rw-r--r--net/wireless/core.h3
-rw-r--r--net/wireless/nl80211.c28
3 files changed, 33 insertions, 28 deletions
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 17fe3904974..d1f556535f6 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -87,7 +87,7 @@ struct wiphy *wiphy_idx_to_wiphy(int wiphy_idx)
}
/* requires cfg80211_mutex to be held! */
-static struct cfg80211_registered_device *
+struct cfg80211_registered_device *
__cfg80211_drv_from_info(struct genl_info *info)
{
int ifindex;
@@ -176,13 +176,14 @@ void cfg80211_put_dev(struct cfg80211_registered_device *drv)
mutex_unlock(&drv->mtx);
}
+/* requires cfg80211_mutex to be held */
int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
char *newname)
{
struct cfg80211_registered_device *drv;
int wiphy_idx, taken = -1, result, digits;
- mutex_lock(&cfg80211_mutex);
+ assert_cfg80211_lock();
/* prohibit calling the thing phy%d when %d is not its number */
sscanf(newname, PHY_NAME "%d%n", &wiphy_idx, &taken);
@@ -195,30 +196,23 @@ int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
* deny the name if it is phy<idx> where <idx> is printed
* without leading zeroes. taken == strlen(newname) here
*/
- result = -EINVAL;
if (taken == strlen(PHY_NAME) + digits)
- goto out_unlock;
+ return -EINVAL;
}
/* Ignore nop renames */
- result = 0;
if (strcmp(newname, dev_name(&rdev->wiphy.dev)) == 0)
- goto out_unlock;
+ return 0;
/* Ensure another device does not already have this name. */
- list_for_each_entry(drv, &cfg80211_drv_list, list) {
- result = -EINVAL;
+ list_for_each_entry(drv, &cfg80211_drv_list, list)
if (strcmp(newname, dev_name(&drv->wiphy.dev)) == 0)
- goto out_unlock;
- }
+ return -EINVAL;
- /* this will only check for collisions in sysfs
- * which is not even always compiled in.
- */
result = device_rename(&rdev->wiphy.dev, newname);
if (result)
- goto out_unlock;
+ return result;
if (rdev->wiphy.debugfsdir &&
!debugfs_rename(rdev->wiphy.debugfsdir->d_parent,
@@ -228,13 +222,9 @@ int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
printk(KERN_ERR "cfg80211: failed to rename debugfs dir to %s!\n",
newname);
- result = 0;
-out_unlock:
- mutex_unlock(&cfg80211_mutex);
- if (result == 0)
- nl80211_notify_dev_rename(rdev);
+ nl80211_notify_dev_rename(rdev);
- return result;
+ return 0;
}
/* exported functions */
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 97a6fd8b2b0..d43daa236ef 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -99,6 +99,9 @@ struct cfg80211_internal_bss {
struct cfg80211_registered_device *cfg80211_drv_by_wiphy_idx(int wiphy_idx);
int get_wiphy_idx(struct wiphy *wiphy);
+struct cfg80211_registered_device *
+__cfg80211_drv_from_info(struct genl_info *info);
+
/*
* This function returns a pointer to the driver
* that the genl_info item that is passed refers to.
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8808431bd58..353e1a4ece8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -366,16 +366,26 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
int result = 0, rem_txq_params = 0;
struct nlattr *nl_txq_params;
- rdev = cfg80211_get_dev_from_info(info);
- if (IS_ERR(rdev))
- return PTR_ERR(rdev);
+ rtnl_lock();
+
+ mutex_lock(&cfg80211_mutex);
+
+ rdev = __cfg80211_drv_from_info(info);
+ if (IS_ERR(rdev)) {
+ result = PTR_ERR(rdev);
+ goto unlock;
+ }
- if (info->attrs[NL80211_ATTR_WIPHY_NAME]) {
+ mutex_lock(&rdev->mtx);
+
+ if (info->attrs[NL80211_ATTR_WIPHY_NAME])
result = cfg80211_dev_rename(
rdev, nla_data(info->attrs[NL80211_ATTR_WIPHY_NAME]));
- if (result)
- goto bad_res;
- }
+
+ mutex_unlock(&cfg80211_mutex);
+
+ if (result)
+ goto bad_res;
if (info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS]) {
struct ieee80211_txq_params txq_params;
@@ -471,7 +481,9 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
bad_res:
- cfg80211_put_dev(rdev);
+ mutex_unlock(&rdev->mtx);
+ unlock:
+ rtnl_unlock();
return result;
}