lkml.org 
[lkml]   [2017]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
Subject[PATCH 3.16 053/178] nl80211: fix dumpit error path RTNL deadlocks
3.16.46-rc1 review patch.  If anyone has any objections, please let me know.

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

From: Johannes Berg <johannes.berg@intel.com>

commit ea90e0dc8cecba6359b481e24d9c37160f6f524f upstream.

Sowmini pointed out Dmitry's RTNL deadlock report to me, and it turns out
to be perfectly accurate - there are various error paths that miss unlock
of the RTNL.

To fix those, change the locking a bit to not be conditional in all those
nl80211_prepare_*_dump() functions, but make those require the RTNL to
start with, and fix the buggy error paths. This also let me use sparse
(by appropriately overriding the rtnl_lock/rtnl_unlock functions) to
validate the changes.

Reported-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
[bwh: Backported to 3.16:
- Drop changes to nl80211_dump_interface(), nl80211_dump_mpp(),
nl80211_prepare_vendor_dump(), nl80211_vendor_cmd_dump()
- Adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/wireless/nl80211.c | 127 ++++++++++++++++++++++---------------------------
1 file changed, 56 insertions(+), 71 deletions(-)

--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -471,21 +471,17 @@ static int nl80211_prepare_wdev_dump(str
{
int err;

- rtnl_lock();
-
if (!cb->args[0]) {
err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
nl80211_fam.attrbuf, nl80211_fam.maxattr,
nl80211_policy);
if (err)
- goto out_unlock;
+ return err;

*wdev = __cfg80211_wdev_from_attrs(sock_net(skb->sk),
nl80211_fam.attrbuf);
- if (IS_ERR(*wdev)) {
- err = PTR_ERR(*wdev);
- goto out_unlock;
- }
+ if (IS_ERR(*wdev))
+ return PTR_ERR(*wdev);
*rdev = wiphy_to_rdev((*wdev)->wiphy);
/* 0 is the first index - add 1 to parse only once */
cb->args[0] = (*rdev)->wiphy_idx + 1;
@@ -495,10 +491,8 @@ static int nl80211_prepare_wdev_dump(str
struct wiphy *wiphy = wiphy_idx_to_wiphy(cb->args[0] - 1);
struct wireless_dev *tmp;

- if (!wiphy) {
- err = -ENODEV;
- goto out_unlock;
- }
+ if (!wiphy)
+ return -ENODEV;
*rdev = wiphy_to_rdev(wiphy);
*wdev = NULL;

@@ -509,21 +503,11 @@ static int nl80211_prepare_wdev_dump(str
}
}

- if (!*wdev) {
- err = -ENODEV;
- goto out_unlock;
- }
+ if (!*wdev)
+ return -ENODEV;
}

return 0;
- out_unlock:
- rtnl_unlock();
- return err;
-}
-
-static void nl80211_finish_wdev_dump(struct cfg80211_registered_device *rdev)
-{
- rtnl_unlock();
}

/* IE validation */
@@ -3727,9 +3711,10 @@ static int nl80211_dump_station(struct s
int sta_idx = cb->args[2];
int err;

+ rtnl_lock();
err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
if (err)
- return err;
+ goto out_err;

if (!wdev->netdev) {
err = -EINVAL;
@@ -3765,7 +3750,7 @@ static int nl80211_dump_station(struct s
cb->args[2] = sta_idx;
err = skb->len;
out_err:
- nl80211_finish_wdev_dump(rdev);
+ rtnl_unlock();

return err;
}
@@ -4443,9 +4428,10 @@ static int nl80211_dump_mpath(struct sk_
int path_idx = cb->args[2];
int err;

+ rtnl_lock();
err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
if (err)
- return err;
+ goto out_err;

if (!rdev->ops->dump_mpath) {
err = -EOPNOTSUPP;
@@ -4479,7 +4465,7 @@ static int nl80211_dump_mpath(struct sk_
cb->args[2] = path_idx;
err = skb->len;
out_err:
- nl80211_finish_wdev_dump(rdev);
+ rtnl_unlock();
return err;
}

@@ -6157,9 +6143,12 @@ static int nl80211_dump_scan(struct sk_b
int start = cb->args[2], idx = 0;
int err;

+ rtnl_lock();
err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
- if (err)
+ if (err) {
+ rtnl_unlock();
return err;
+ }

wdev_lock(wdev);
spin_lock_bh(&rdev->bss_lock);
@@ -6182,7 +6171,7 @@ static int nl80211_dump_scan(struct sk_b
wdev_unlock(wdev);

cb->args[2] = idx;
- nl80211_finish_wdev_dump(rdev);
+ rtnl_unlock();

return skb->len;
}
@@ -6255,9 +6244,10 @@ static int nl80211_dump_survey(struct sk
int survey_idx = cb->args[2];
int res;

+ rtnl_lock();
res = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
if (res)
- return res;
+ goto out_err;

if (!wdev->netdev) {
res = -EINVAL;
@@ -6303,7 +6293,7 @@ static int nl80211_dump_survey(struct sk
cb->args[2] = survey_idx;
res = skb->len;
out_err:
- nl80211_finish_wdev_dump(rdev);
+ rtnl_unlock();
return res;
}

\
 
 \ /
  Last update: 2017-07-16 15:58    [W:0.753 / U:19.876 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site