Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-11-26 14:12:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

nl80211: fix dumpit error path RTNL deadlocks

Browse Source 
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.

Cc: stable@vger.kernel.org
Reported-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
This commit is contained in:
Johannes Berg 2017-03-15 14:26:04 +01:00
parent 22a0e18eac
commit ea90e0dc8c
1 changed files with 56 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

127
net/wireless/nl80211.c
View File

@ -545,22 +545,18 @@ 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,
genl_family_attrbuf(&nl80211_fam), genl_family_attrbuf(&nl80211_fam),
nl80211_fam.maxattr, nl80211_policy); nl80211_fam.maxattr, nl80211_policy);
if (err) if (err)
goto out_unlock; return err;
*wdev = __cfg80211_wdev_from_attrs( *wdev = __cfg80211_wdev_from_attrs(
sock_net(skb->sk), sock_net(skb->sk),
genl_family_attrbuf(&nl80211_fam)); genl_family_attrbuf(&nl80211_fam));
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;
@ -570,10 +566,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;
@ -584,21 +578,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 */
@ -2608,17 +2592,17 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback *
int filter_wiphy = -1; int filter_wiphy = -1;
struct cfg80211_registered_device *rdev; struct cfg80211_registered_device *rdev;
struct wireless_dev *wdev; struct wireless_dev *wdev;
int ret;
rtnl_lock(); rtnl_lock();
if (!cb->args[2]) { if (!cb->args[2]) {
struct nl80211_dump_wiphy_state state = { struct nl80211_dump_wiphy_state state = {
.filter_wiphy = -1, .filter_wiphy = -1,
}; };
int ret;
ret = nl80211_dump_wiphy_parse(skb, cb, &state); ret = nl80211_dump_wiphy_parse(skb, cb, &state);
if (ret) if (ret)
return ret; goto out_unlock;
filter_wiphy = state.filter_wiphy; filter_wiphy = state.filter_wiphy;
@ -2663,12 +2647,14 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback *
wp_idx++; wp_idx++;
} }
out: out:
rtnl_unlock();
cb->args[0] = wp_idx; cb->args[0] = wp_idx;
cb->args[1] = if_idx; cb->args[1] = if_idx;
return skb->len; ret = skb->len;
out_unlock:
rtnl_unlock();
return ret;
} }
static int nl80211_get_interface(struct sk_buff *skb, struct genl_info *info) static int nl80211_get_interface(struct sk_buff *skb, struct genl_info *info)
@ -4452,9 +4438,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;
@ -4489,7 +4476,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;
} }
@ -5275,9 +5262,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;
@ -5310,7 +5298,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;
} }
@ -5470,9 +5458,10 @@ static int nl80211_dump_mpp(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_mpp) { if (!rdev->ops->dump_mpp) {
err = -EOPNOTSUPP; err = -EOPNOTSUPP;
@ -5505,7 +5494,7 @@ static int nl80211_dump_mpp(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;
} }
@ -7674,9 +7663,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);
@ -7699,7 +7691,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;
} }
@ -7784,9 +7776,10 @@ static int nl80211_dump_survey(struct sk_buff *skb, struct netlink_callback *cb)
int res; int res;
bool radio_stats; bool radio_stats;
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;
/* prepare_wdev_dump parsed the attributes */ /* prepare_wdev_dump parsed the attributes */
radio_stats = attrbuf[NL80211_ATTR_SURVEY_RADIO_STATS]; radio_stats = attrbuf[NL80211_ATTR_SURVEY_RADIO_STATS];
@ -7827,7 +7820,7 @@ static int nl80211_dump_survey(struct sk_buff *skb, struct netlink_callback *cb)
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;
} }
@ -11508,17 +11501,13 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb,
void *data = NULL; void *data = NULL;
unsigned int data_len = 0; unsigned int data_len = 0;
rtnl_lock();
if (cb->args[0]) { if (cb->args[0]) {
/* subtract the 1 again here */ /* subtract the 1 again here */
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;
@ -11538,23 +11527,19 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb,
err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize, err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
attrbuf, nl80211_fam.maxattr, nl80211_policy); attrbuf, nl80211_fam.maxattr, nl80211_policy);
if (err) if (err)
goto out_unlock; return err;
if (!attrbuf[NL80211_ATTR_VENDOR_ID] || if (!attrbuf[NL80211_ATTR_VENDOR_ID] ||
!attrbuf[NL80211_ATTR_VENDOR_SUBCMD]) { !attrbuf[NL80211_ATTR_VENDOR_SUBCMD])
err = -EINVAL; return -EINVAL;
goto out_unlock;
}
*wdev = __cfg80211_wdev_from_attrs(sock_net(skb->sk), attrbuf); *wdev = __cfg80211_wdev_from_attrs(sock_net(skb->sk), attrbuf);
if (IS_ERR(*wdev)) if (IS_ERR(*wdev))
*wdev = NULL; *wdev = NULL;
*rdev = __cfg80211_rdev_from_attrs(sock_net(skb->sk), attrbuf); *rdev = __cfg80211_rdev_from_attrs(sock_net(skb->sk), attrbuf);
if (IS_ERR(*rdev)) { if (IS_ERR(*rdev))
err = PTR_ERR(*rdev); return PTR_ERR(*rdev);
goto out_unlock;
}
vid = nla_get_u32(attrbuf[NL80211_ATTR_VENDOR_ID]); vid = nla_get_u32(attrbuf[NL80211_ATTR_VENDOR_ID]);
subcmd = nla_get_u32(attrbuf[NL80211_ATTR_VENDOR_SUBCMD]); subcmd = nla_get_u32(attrbuf[NL80211_ATTR_VENDOR_SUBCMD]);
@ -11567,19 +11552,15 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb,
if (vcmd->info.vendor_id != vid || vcmd->info.subcmd != subcmd) if (vcmd->info.vendor_id != vid || vcmd->info.subcmd != subcmd)
continue; continue;
if (!vcmd->dumpit) { if (!vcmd->dumpit)
err = -EOPNOTSUPP; return -EOPNOTSUPP;
goto out_unlock;
}
vcmd_idx = i; vcmd_idx = i;
break; break;
} }
if (vcmd_idx < 0) { if (vcmd_idx < 0)
err = -EOPNOTSUPP; return -EOPNOTSUPP;
goto out_unlock;
}
if (attrbuf[NL80211_ATTR_VENDOR_DATA]) { if (attrbuf[NL80211_ATTR_VENDOR_DATA]) {
data = nla_data(attrbuf[NL80211_ATTR_VENDOR_DATA]); data = nla_data(attrbuf[NL80211_ATTR_VENDOR_DATA]);
@ -11596,9 +11577,6 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb,
/* keep rtnl locked in successful case */ /* keep rtnl locked in successful case */
return 0; return 0;
out_unlock:
rtnl_unlock();
return err;
} }
static int nl80211_vendor_cmd_dump(struct sk_buff *skb, static int nl80211_vendor_cmd_dump(struct sk_buff *skb,
@ -11613,9 +11591,10 @@ static int nl80211_vendor_cmd_dump(struct sk_buff *skb,
int err; int err;
struct nlattr *vendor_data; struct nlattr *vendor_data;
rtnl_lock();
err = nl80211_prepare_vendor_dump(skb, cb, &rdev, &wdev); err = nl80211_prepare_vendor_dump(skb, cb, &rdev, &wdev);
if (err) if (err)
return err; goto out;
vcmd_idx = cb->args[2]; vcmd_idx = cb->args[2];
data = (void *)cb->args[3]; data = (void *)cb->args[3];
@ -11624,15 +11603,21 @@ static int nl80211_vendor_cmd_dump(struct sk_buff *skb,
if (vcmd->flags & (WIPHY_VENDOR_CMD_NEED_WDEV | if (vcmd->flags & (WIPHY_VENDOR_CMD_NEED_WDEV |
WIPHY_VENDOR_CMD_NEED_NETDEV)) { WIPHY_VENDOR_CMD_NEED_NETDEV)) {
if (!wdev) if (!wdev) {
return -EINVAL; err = -EINVAL;
goto out;
}
if (vcmd->flags & WIPHY_VENDOR_CMD_NEED_NETDEV && if (vcmd->flags & WIPHY_VENDOR_CMD_NEED_NETDEV &&
!wdev->netdev) !wdev->netdev) {
return -EINVAL; err = -EINVAL;
goto out;
}
if (vcmd->flags & WIPHY_VENDOR_CMD_NEED_RUNNING) { if (vcmd->flags & WIPHY_VENDOR_CMD_NEED_RUNNING) {
if (!wdev_running(wdev)) if (!wdev_running(wdev)) {
return -ENETDOWN; err = -ENETDOWN;
goto out;
}
} }
} }