Commit 8e6a95d2 authored by Johannes Berg's avatar Johannes Berg Committed by Ben Hutchings

nl80211: fix dumpit error path RTNL deadlocks

commit ea90e0dc 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: default avatarSowmini Varadhan <sowmini.varadhan@oracle.com>
Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarJohannes 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: default avatarBen Hutchings <ben@decadent.org.uk>
parent b6695dba
...@@ -471,21 +471,17 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb, ...@@ -471,21 +471,17 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
{ {
int err; int err;
rtnl_lock();
if (!cb->args[0]) { if (!cb->args[0]) {
err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize, err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
nl80211_fam.attrbuf, nl80211_fam.maxattr, nl80211_fam.attrbuf, nl80211_fam.maxattr,
nl80211_policy); nl80211_policy);
if (err) if (err)
goto out_unlock; return err;
*wdev = __cfg80211_wdev_from_attrs(sock_net(skb->sk), *wdev = __cfg80211_wdev_from_attrs(sock_net(skb->sk),
nl80211_fam.attrbuf); nl80211_fam.attrbuf);
if (IS_ERR(*wdev)) { if (IS_ERR(*wdev))
err = PTR_ERR(*wdev); return PTR_ERR(*wdev);
goto out_unlock;
}
*rdev = wiphy_to_rdev((*wdev)->wiphy); *rdev = wiphy_to_rdev((*wdev)->wiphy);
/* 0 is the first index - add 1 to parse only once */ /* 0 is the first index - add 1 to parse only once */
cb->args[0] = (*rdev)->wiphy_idx + 1; cb->args[0] = (*rdev)->wiphy_idx + 1;
...@@ -495,10 +491,8 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb, ...@@ -495,10 +491,8 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
struct wiphy *wiphy = wiphy_idx_to_wiphy(cb->args[0] - 1); struct wiphy *wiphy = wiphy_idx_to_wiphy(cb->args[0] - 1);
struct wireless_dev *tmp; struct wireless_dev *tmp;
if (!wiphy) { if (!wiphy)
err = -ENODEV; return -ENODEV;
goto out_unlock;
}
*rdev = wiphy_to_rdev(wiphy); *rdev = wiphy_to_rdev(wiphy);
*wdev = NULL; *wdev = NULL;
...@@ -509,21 +503,11 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb, ...@@ -509,21 +503,11 @@ static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
} }
} }
if (!*wdev) { if (!*wdev)
err = -ENODEV; return -ENODEV;
goto out_unlock;
}
} }
return 0; return 0;
out_unlock:
rtnl_unlock();
return err;
}
static void nl80211_finish_wdev_dump(struct cfg80211_registered_device *rdev)
{
rtnl_unlock();
} }
/* IE validation */ /* IE validation */
...@@ -3727,9 +3711,10 @@ static int nl80211_dump_station(struct sk_buff *skb, ...@@ -3727,9 +3711,10 @@ static int nl80211_dump_station(struct sk_buff *skb,
int sta_idx = cb->args[2]; int sta_idx = cb->args[2];
int err; int err;
rtnl_lock();
err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev); err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
if (err) if (err)
return err; goto out_err;
if (!wdev->netdev) { if (!wdev->netdev) {
err = -EINVAL; err = -EINVAL;
...@@ -3765,7 +3750,7 @@ static int nl80211_dump_station(struct sk_buff *skb, ...@@ -3765,7 +3750,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
cb->args[2] = sta_idx; cb->args[2] = sta_idx;
err = skb->len; err = skb->len;
out_err: out_err:
nl80211_finish_wdev_dump(rdev); rtnl_unlock();
return err; return err;
} }
...@@ -4443,9 +4428,10 @@ static int nl80211_dump_mpath(struct sk_buff *skb, ...@@ -4443,9 +4428,10 @@ static int nl80211_dump_mpath(struct sk_buff *skb,
int path_idx = cb->args[2]; int path_idx = cb->args[2];
int err; int err;
rtnl_lock();
err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev); err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
if (err) if (err)
return err; goto out_err;
if (!rdev->ops->dump_mpath) { if (!rdev->ops->dump_mpath) {
err = -EOPNOTSUPP; err = -EOPNOTSUPP;
...@@ -4479,7 +4465,7 @@ static int nl80211_dump_mpath(struct sk_buff *skb, ...@@ -4479,7 +4465,7 @@ static int nl80211_dump_mpath(struct sk_buff *skb,
cb->args[2] = path_idx; cb->args[2] = path_idx;
err = skb->len; err = skb->len;
out_err: out_err:
nl80211_finish_wdev_dump(rdev); rtnl_unlock();
return err; return err;
} }
...@@ -6157,9 +6143,12 @@ static int nl80211_dump_scan(struct sk_buff *skb, struct netlink_callback *cb) ...@@ -6157,9 +6143,12 @@ static int nl80211_dump_scan(struct sk_buff *skb, struct netlink_callback *cb)
int start = cb->args[2], idx = 0; int start = cb->args[2], idx = 0;
int err; int err;
rtnl_lock();
err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev); err = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
if (err) if (err) {
rtnl_unlock();
return err; return err;
}
wdev_lock(wdev); wdev_lock(wdev);
spin_lock_bh(&rdev->bss_lock); spin_lock_bh(&rdev->bss_lock);
...@@ -6182,7 +6171,7 @@ static int nl80211_dump_scan(struct sk_buff *skb, struct netlink_callback *cb) ...@@ -6182,7 +6171,7 @@ static int nl80211_dump_scan(struct sk_buff *skb, struct netlink_callback *cb)
wdev_unlock(wdev); wdev_unlock(wdev);
cb->args[2] = idx; cb->args[2] = idx;
nl80211_finish_wdev_dump(rdev); rtnl_unlock();
return skb->len; return skb->len;
} }
...@@ -6255,9 +6244,10 @@ static int nl80211_dump_survey(struct sk_buff *skb, ...@@ -6255,9 +6244,10 @@ static int nl80211_dump_survey(struct sk_buff *skb,
int survey_idx = cb->args[2]; int survey_idx = cb->args[2];
int res; int res;
rtnl_lock();
res = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev); res = nl80211_prepare_wdev_dump(skb, cb, &rdev, &wdev);
if (res) if (res)
return res; goto out_err;
if (!wdev->netdev) { if (!wdev->netdev) {
res = -EINVAL; res = -EINVAL;
...@@ -6303,7 +6293,7 @@ static int nl80211_dump_survey(struct sk_buff *skb, ...@@ -6303,7 +6293,7 @@ static int nl80211_dump_survey(struct sk_buff *skb,
cb->args[2] = survey_idx; cb->args[2] = survey_idx;
res = skb->len; res = skb->len;
out_err: out_err:
nl80211_finish_wdev_dump(rdev); rtnl_unlock();
return res; return res;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment